Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behaviour when $GLOBALS is deleted #4033

Closed
Pointer666 opened this issue Feb 6, 2020 · 8 comments
Closed

Unexpected behaviour when $GLOBALS is deleted #4033

Pointer666 opened this issue Feb 6, 2020 · 8 comments
Assignees
Labels
type/bug Something is broken

Comments

@Pointer666
Copy link

Q A
PHPUnit version 8.5.2
PHP version 7.2.27
Installation Method Composer

doctrine/instantiator 1.3.0 A small, lightweight utility to instantiate objects in PHP without invoking their constructors
myclabs/deep-copy 1.9.5 Create deep copies (clones) of your objects
phar-io/manifest 1.0.3 Component for reading phar.io manifest information from a PHP Archive (PHAR)
phar-io/version 2.0.1 Library for handling version information and constraints
phpdocumentor/reflection-common 2.0.0 Common reflection classes used by phpdocumentor to reflect the code structure
phpdocumentor/reflection-docblock 4.3.4 With this component, a library can provide support for annotations via DocBlocks or otherwise retrieve information that is ...
phpdocumentor/type-resolver 1.0.1 A PSR-5 based resolver of Class names, Types and Structural Element Names
phpspec/prophecy v1.10.2 Highly opinionated mocking framework for PHP 5.3+
phpunit/php-code-coverage 7.0.10 Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator 2.0.2 FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-text-template 1.2.1 Simple template engine.
phpunit/php-timer 2.1.2 Utility class for timing
phpunit/php-token-stream 3.1.1 Wrapper around PHP's tokenizer extension.
phpunit/phpunit 8.5.2 The PHP Unit Testing framework.
sebastian/code-unit-reverse-lookup 1.0.1 Looks up which function or method a line of code belongs to
sebastian/comparator 3.0.2 Provides the functionality to compare PHP values for equality
sebastian/diff 3.0.2 Diff implementation
sebastian/environment 4.2.3 Provides functionality to handle HHVM/PHP environments
sebastian/exporter 3.1.2 Provides the functionality to export PHP variables for visualization
sebastian/global-state 3.0.0 Snapshotting of global state
sebastian/object-enumerator 3.0.3 Traverses array structures and object graphs to enumerate all referenced objects
sebastian/object-reflector 1.1.1 Allows reflection of object attributes, including inherited and non-public ones
sebastian/recursion-context 3.0.0 Provides functionality to recursively process PHP variables
sebastian/resource-operations 2.0.1 Provides a list of PHP built-in functions that operate on resources
sebastian/type 1.1.3 Collection of value objects that represent the types of the PHP type system
sebastian/version 2.0.1 Library that helps with managing the version number of Git-hosted PHP projects
symfony/polyfill-ctype v1.13.1 Symfony polyfill for ctype functions
theseer/tokenizer 1.1.3 A small library for converting tokenized PHP source code into XML and potentially other formats
webmozart/assert 1.6.0 Assertions to validate method input/output with nice error messages.

Summary

If the $GLOBALS array is cleared and an error occured in the test a warning is triggered inside
of phpunit and the causing error is never shown.

How to reproduce

class test extends \PHPUnit\Framework\TestCase
{
	public function SetUp(): void
	{
		$GLOBALS            = [];
	}

	public function testError()
	{
		$this->doesNotExist(); 
	}
}

result:

PHPUnit 8.5.2 by Sebastian Bergmann and contributors.

E                                                                   1 / 1 (100%)

Time: 22 ms, Memory: 4.00 MB

There was 1 error:

1) test::testError
PHP Notice:  Undefined index: _SERVER in /home/maik/test/vendor/phpunit/phpunit/src/Util/Filter.php on line 76
PHP Stack trace:
PHP   1. {main}() /home/maik/test/vendor/phpunit/phpunit/phpunit:0
PHP   2. PHPUnit\TextUI\Command::main() /home/maik/test/vendor/phpunit/phpunit/phpunit:61
PHP   3. PHPUnit\TextUI\Command->run() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/Command.php:159
PHP   4. PHPUnit\TextUI\TestRunner->doRun() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/Command.php:200
PHP   5. PHPUnit\TextUI\ResultPrinter->printResult() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:632
PHP   6. PHPUnit\TextUI\ResultPrinter->printErrors() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:164
PHP   7. PHPUnit\TextUI\ResultPrinter->printDefects() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:360
PHP   8. PHPUnit\TextUI\ResultPrinter->printDefect() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:325
PHP   9. PHPUnit\TextUI\ResultPrinter->printDefectTrace() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:334
PHP  10. PHPUnit\Framework\ExceptionWrapper->__toString() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:351
PHP  11. PHPUnit\Util\Filter::getFilteredStacktrace() /home/maik/test/vendor/phpunit/phpunit/src/Framework/ExceptionWrapper.php:50
PHP  12. PHPUnit\Util\Filter::shouldPrintFrame() /home/maik/test/vendor/phpunit/phpunit/src/Util/Filter.php:56
PHP Fatal error:  Method PHPUnit\Framework\ExceptionWrapper::__toString() must not throw an exception, caught TypeError: realpath() expects parameter 1 to be a valid path, null given in /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php on line 0
PHP Stack trace:
PHP   1. {main}() /home/maik/test/vendor/phpunit/phpunit/phpunit:0
PHP   2. PHPUnit\TextUI\Command::main() /home/maik/test/vendor/phpunit/phpunit/phpunit:61
PHP   3. PHPUnit\TextUI\Command->run() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/Command.php:159
PHP   4. PHPUnit\TextUI\TestRunner->doRun() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/Command.php:200
PHP   5. PHPUnit\TextUI\ResultPrinter->printResult() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:632
PHP   6. PHPUnit\TextUI\ResultPrinter->printErrors() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:164
PHP   7. PHPUnit\TextUI\ResultPrinter->printDefects() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:360
PHP   8. PHPUnit\TextUI\ResultPrinter->printDefect() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:325
PHP   9. PHPUnit\TextUI\ResultPrinter->printDefectTrace() /home/maik/test/vendor/phpunit/phpunit/src/TextUI/ResultPrinter.php:334

Expected behavior

I would expect the following output.

PHPUnit 8.5.2 by Sebastian Bergmann and contributors.

E                                                                   1 / 1 (100%)

Time: 22 ms, Memory: 4.00 MB

There was 1 error:

1) test::testError
Error: Call to undefined method test::doesNotExist()

/home/maik/test/test.php:14
/home/maik/test/vendor/phpunit/phpunit/phpunit:61

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

@Pointer666 Pointer666 added the type/bug Something is broken label Feb 6, 2020
@epdenouden
Copy link
Contributor

I will stitch this one up, I have unfinished business with global vars

@williamdes
Copy link

Same, I could not remove globals

@williamdes
Copy link

@sebastianbergmann
Copy link
Owner

I think that deleting $GLOBALS is a terrible idea. Anyway, I'll adapt PHPUnit to $GLOBALS being unavailable.

@williamdes
Copy link

I think that deleting $GLOBALS is a terrible idea. Anyway, I'll adapt PHPUnit to $GLOBALS being unavailable.

Could you explain why ?

For me keeping $GLOBALS between tests is also a terrible idea as they make them potentially depending on another test.

@sebastianbergmann
Copy link
Owner

For me keeping $GLOBALS between tests is also a terrible idea as they make them potentially depending on another test.

I totally agree with you.

PHPUnit can make a backup of $GLOBALS before a test and restore that backup to $GLOBALS after a test. This can be configured globally for all tests (using --globals-backup or backupGlobals="false") or on a test-by-test basis using the @backupGlobals enabled annotation.

This backup/restore functionality is expensive and is not required for clean code that does not modify $GLOBALS (and ideally not even looks at $GLOBALS). This is why this functionality is disabled by default. If your tests or your tested code interact(s) with $GLOBALS then you should enable this functionality.

By the way: the --strict-global-state option can be used to find tests that modify $GLOBALS. Careful, though, this is twice as expensive as --globals-backup because it makes two snapshots, one before the test and one after, of $GLOBALS and then compares them.

@williamdes
Copy link

Thank you for the feedback, I appreciate it !
backupGlobals="false" phpmyadmin/phpmyadmin@5bc53cc
Maybe this is the reason why I gained half time in performance (phpmyadmin/phpmyadmin#16131)

strict-global-state would be a good option for us but only for investigating.

I think the code I did in the setup/teardown will be good for us as we continue to remove $GLOBALS

By the way could you push a patch for a missing REQUEST_TIME_FLOAT it crashes the time output at the end ?

@sebastianbergmann
Copy link
Owner

strict-global-state would be a good option for us but only for investigating.

This is exactly what it is intended for. Do not always use this.

By the way could you push a patch for a missing REQUEST_TIME_FLOAT it crashes the time output at the end ?

This is taken care of in PHPUnit 9.2. I will not backport this to PHPUnit 8.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

4 participants