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

@runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses #3258

Closed
bugreportuser opened this issue Aug 19, 2018 · 18 comments
Assignees
Labels
feature/process-isolation Issues related to running tests in separate PHP processes type/bug Something is broken

Comments

@bugreportuser
Copy link

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...
@bugreportuser bugreportuser changed the title @runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses @runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses Aug 19, 2018
@hwmaier
Copy link

hwmaier commented Oct 7, 2018

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
Copy link
Author

@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
Copy link
Contributor

@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
Copy link

hwmaier commented Dec 4, 2018

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

@bugreportuser
Copy link
Author

@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
Copy link
Contributor

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:

if ($this->runInSeparateProcess()) {
$runEntireClass = $this->runClassInSeparateProcess && !$this->runTestInSeparateProcess;
$class = new ReflectionClass($this);
if ($runEntireClass) {
print "## @runClassInSeperateProcess\n";
$template = new Text_Template(
__DIR__ . '/../Util/PHP/Template/TestCaseClass.tpl'
);
} else {
print "## @runInSeperateProcess\n";
$template = new Text_Template(
__DIR__ . '/../Util/PHP/Template/TestCaseMethod.tpl'
);
}

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.

@stale stale bot added the stale label Feb 4, 2019
@bugreportuser
Copy link
Author

@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
Copy link

hwmaier commented Feb 4, 2019

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

@stale stale bot closed this as completed Feb 11, 2019
@mundschenk-at
Copy link

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

@mundschenk-at
Copy link

@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.

@stale stale bot removed the stale label Jun 13, 2019
Repository owner deleted a comment from stale bot Jul 2, 2019
Repository owner deleted a comment from stale bot Jul 2, 2019
Repository owner deleted a comment from stale bot Jul 2, 2019
Repository owner deleted a comment from epdenouden Jul 2, 2019
Repository owner deleted a comment from stale bot Jul 2, 2019
@sebastianbergmann sebastianbergmann added feature/process-isolation Issues related to running tests in separate PHP processes type/bug Something is broken labels Feb 11, 2020
@n-peugnet
Copy link

@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
Copy link

Please fix this, still not working correctly

@WalterWoshid
Copy link

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
Copy link
Owner

sebastianbergmann commented Feb 21, 2023

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

@sebastianbergmann
Copy link
Owner

Superseded by #5230

@WalterWoshid
Copy link

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

@sebastianbergmann
Copy link
Owner

@WalterWoshid Great! Thank you for looking into this.

@WalterWoshid
Copy link

#5233

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/process-isolation Issues related to running tests in separate PHP processes type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

7 participants