sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.71k stars 2.2k forks source link

@runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses #3258

Closed bugreportuser closed 2 years ago

bugreportuser commented 6 years ago
Q A
PHPUnit version 7.4-dev
PHP version 7.2.7
Installation Method Composer

@runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses and makes test SeparateClassPreserveTest.php fail when it's run by itself. My project has the same problem so I think the problem is with the annotation.

The older versions I tested have the same problem.

$ ./phpunit tests__Regression__GitHub__2591__SeparateClassPreserveTest
PHPUnit 7.4-dev by Sebastian Bergmann and contributors.

Runtime:       PHP 7.2.7-0ubuntu0.18.04.2
Configuration: /home/.../phpunit-master/phpunit.xml

E.E                                                                 3 / 3 (100%)

Time: 120 ms, Memory: 4.00MB

There were 2 errors:

1) Issue2591_SeparateClassPreserveTest::testOriginalGlobalString
Undefined index: globalString

/home/ubuntu/Downloads/phpunit-master/tests/Regression/GitHub/2591/SeparateClassPreserveTest.php:20
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestCase.php:1150
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestCase.php:844
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestResult.php:665
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestCase.php:798

2) Issue2591_SeparateClassPreserveTest::testGlobalString
Undefined index: globalString

/home/ubuntu/Downloads/phpunit-master/tests/Regression/GitHub/2591/SeparateClassPreserveTest.php:33
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestCase.php:1150
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestCase.php:844
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestResult.php:665
/home/ubuntu/Downloads/phpunit-master/src/Framework/TestCase.php:798

ERRORS!
Tests: 3, Assertions: 1, Errors: 2.

It makes sense for the first test to fail but the third shouldn't.

All tests pass when run together.

$ composer info | sort
doctrine/instantiator              1.1.0 A small, lightweight utility to ...
myclabs/deep-copy                  1.8.1 Create deep copies (clones) of y...
phar-io/manifest                   1.0.3 Component for reading phar.io ma...
phar-io/version                    2.0.1 Library for handling version inf...
phpdocumentor/reflection-common    1.0.1 Common reflection classes used b...
phpdocumentor/reflection-docblock  4.3.0 With this component, a library c...
phpdocumentor/type-resolver        0.4.0
phpspec/prophecy                   1.8.0 Highly opinionated mocking frame...
phpunit/php-code-coverage          6.0.7 Library that provides collection...
phpunit/php-file-iterator          2.0.1 FilterIterator implementation th...
phpunit/php-text-template          1.2.1 Simple template engine.
phpunit/php-timer                  2.0.0 Utility class for timing
phpunit/php-token-stream           3.0.0 Wrapper around PHP's tokenizer e...
sebastian/code-unit-reverse-lookup 1.0.1 Looks up which function or metho...
sebastian/comparator               3.0.2 Provides the functionality to co...
sebastian/diff                     3.0.1 Diff implementation
sebastian/environment              3.1.0 Provides functionality to handle...
sebastian/exporter                 3.1.0 Provides the functionality to ex...
sebastian/global-state             2.0.0 Snapshotting of global state
sebastian/object-enumerator        3.0.3 Traverses array structures and o...
sebastian/object-reflector         1.1.1 Allows reflection of object attr...
sebastian/recursion-context        3.0.0 Provides functionality to recurs...
sebastian/resource-operations      1.0.0 Provides a list of PHP built-in ...
sebastian/version                  2.0.1 Library that helps with managing...
theseer/tokenizer                  1.1.0 A small library for converting t...
webmozart/assert                   1.3.0 Assertions to validate method in...
hwmaier commented 6 years ago

I am seeing the same issue with phpUnit 7.4.0. Despite @runClassInSeparateProcesss annotation the individual tests in a class are all run in isolation to each other rather run in the same process but isolated to others classes. So there is no visible difference between @runClassInSeparateProcesss and @runTestsInSeparateProcesses and the expected speed improvement for setting up a process only once for a class does eventuate.

bugreportuser commented 5 years ago

@sebastianbergmann Can you see if this can be fixed? It makes tests run very slowly. I can create a pull request if it's something simple.

epdenouden commented 5 years ago

@hwmaier @bugreportuser I am working on this right now. Will take me a bit of time to read up on the context. #2591 has multiple end-to-end tests and moving parts, however none of these is failing. I'll dig into this and report back here.

hwmaier commented 5 years ago

@epdenouden I am happy to assist and run a possible fix against our test cases.

bugreportuser commented 5 years ago

@epdenouden Thanks. It's good to see this being worked on.

I checked why the test fails, and it's because it doesn't run the bootstrap file. It passes when the bootstrap file runs, however the test is wrong. It should be:

     public function testGlobalString(): void
     {
-        $this->assertEquals('Hello', $GLOBALS['globalString']);
+        $this->assertEquals('Hello! I am changed from inside!', $GLOBALS['globalString']);
     }
 }

The test does fail when that line is changed.

epdenouden commented 5 years ago

TL;DR Great to get such quick feedback! The @runClassInSeperateProcess annotation doesn't work as advertised and I suspect the code isn't ever hit.

@hwmaier thanks, I surely will take you up on the offer! @bugreportuser thanks for pointing out the test. This whole group of tests could use a good second look. @sebastianbergmann I will fix the support for this feature for the 7.x branch

image

Background In all honesty I need to do more exploring of this piece of the PHPUnit code as I am not very familiar with it yet. After as first reading I was also wondering why the current end-to-end tests didn't notice the failing support for @runClassInSeparateProcess. As a first step for fixing this thing I'll make a list of the configurations that are tested by the #2591 regression tests.

It appears as though the 'in seperate process' piece of logic in TestSuite and TestCase cannot handle running complete classes. The logic for @runClassInSeperateProcess is buried in the TestCase and not in the parent TestSuite where one would expect it: https://github.com/sebastianbergmann/phpunit/blob/34114e897a9e743c4e77bcd172b46d105a345c1e/src/Framework/TestCase.php#L690-L705

There are two slightly different code templates to run either a complete class or an individual test in a seperate PHPUnit process. These templates look sensible. However, there doesn't seem to be any code at the TestSuite level to configure, start and parse the output of an external test run for a complete class. Logic like \PHPUnit\Util\PHP\AbstractPhpProcess::runTestJob and AbstractPhpProcess::processChildResult, for example, will need to be checked.

bugreportuser commented 5 years ago

@epdenouden I recently moved to another test framework so this issue isn't a priority for me anymore. The slow tests were affecting productivity so I decided it was time to switch. Thanks for your work on the issue.

@hwmaier Is this still important to you? I don't want to suggest delaying a fix if you're still waiting.

hwmaier commented 5 years ago

@bugreportuser The issue is not critical and I have workarounds at present, so from my point this can wait.

mundschenk-at commented 5 years ago

So the comments above don‘t count as activity? "Not super urgent“ does not mean it is not a valid issue.

mundschenk-at commented 5 years ago

@epdenouden Can I help in any way? I'm not very familiar with PHPUnit's internals, but I think this would be an important bug to fix.

n-peugnet commented 1 year ago

@sebastianbergmann could you please add a link to the pull request/commit that fixed this issue? Or maybe list the version(s) since which the fix is included? I can't seem to find this change in any of the changelog files.

WalterWoshid commented 1 year ago

Please fix this, still not working correctly

WalterWoshid commented 1 year ago

I have found out that the templates are identical, either methodName or name is chosen, but this comes from the same test. I think you have to implement the runClassInSeparateProcess for the TestSuite itself. I have not looked further into this and don't know if there is any additional logic that is run between the methods @sebastianbergmann

sebastianbergmann commented 1 year ago

Please open a new issue and provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.

sebastianbergmann commented 1 year ago

Superseded by #5230

WalterWoshid commented 1 year ago

@sebastianbergmann Thanks for the information, I'd love to create a pull request once I figure out how the process isolation works :)

sebastianbergmann commented 1 year ago

@WalterWoshid Great! Thank you for looking into this.

WalterWoshid commented 1 year ago

https://github.com/sebastianbergmann/phpunit/pull/5233