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 #5230

Open
sebastianbergmann opened this issue Feb 21, 2023 · 4 comments
Labels
feature/process-isolation Issues related to running tests in separate PHP processes feature/test-runner CLI test runner type/bug Something is broken version/10 Something affects PHPUnit 10 version/11 Something affects PHPUnit 11

Comments

@sebastianbergmann
Copy link
Owner

As previously reported in #3258, @runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses.

Unless I am mistaken (which I may well be), this can be proven by applying ...

diff --git a/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php b/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php
index c62c030be..4bbaf9c7d 100644
--- a/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php
+++ b/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php
@@ -46,4 +46,12 @@ public function testTestMethodIsRunInSeparateProcess(): void
         $this->assertNotSame(self::INITIAL_PARENT_PROCESS_ID, self::$parentProcessId);
         $this->assertNotSame(self::$processId, self::$parentProcessId);
     }
+
+    /**
+     * @depends testTestMethodIsRunInSeparateProcess
+     */
+    public function testTestMethodIsRunInSameProcessAsOtherTestMethodsOfThisTestClass(): void
+    {
+        $this->assertSame(self::$processId, \getmypid());
+    }
 }

... to augment an existing test:

./phpunit tests/end-to-end/regression/2724-diff-pid-from-parent-process.phpt                                         
PHPUnit 9.6.3-21-g9da10e7e66 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.3
Configuration: /usr/local/src/phpunit/phpunit.xml

F                                                                   1 / 1 (100%)

Time: 00:00.262, Memory: 4.00 MB

There was 1 failure:

1) /usr/local/src/phpunit/tests/end-to-end/regression/2724-diff-pid-from-parent-process.phpt
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 PHPUnit 9.6.3-21-g9da10e7e66 by Sebastian Bergmann and contributors.
 
-.                                                                   1 / 1 (100%)
+.F                                                                  2 / 2 (100%)
 
 Time: 00:00.156, Memory: 4.00 MB
 
-OK (1 test, 3 assertions)
+There was 1 failure:
+
+1) SeparateClassRunMethodInNewProcessTest::testTestMethodIsRunInSameProcessAsOtherTestMethodsOfThisTestClass
+Failed asserting that 260001 is identical to 1.
+
+/usr/local/src/phpunit/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php:55
+
+FAILURES!
+Tests: 2, Assertions: 4, Failures: 1.

/usr/local/src/phpunit/tests/end-to-end/regression/2724-diff-pid-from-parent-process.phpt:17
/usr/local/src/phpunit/src/Framework/TestSuite.php:684
/usr/local/src/phpunit/src/TextUI/TestRunner.php:653
/usr/local/src/phpunit/src/TextUI/Command.php:144
/usr/local/src/phpunit/src/TextUI/Command.php:97

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

If I understand the above correctly, then, indeed, testTestMethodIsRunInSeparateProcess() and testTestMethodIsRunInSameProcessAsOtherTestMethodsOfThisTestClass() are run in two separate processes and not in a single separate process.

@sebastianbergmann sebastianbergmann added type/bug Something is broken feature/test-runner CLI test runner feature/process-isolation Issues related to running tests in separate PHP processes version/9 Something affects PHPUnit 9 version/10 Something affects PHPUnit 10 labels Feb 21, 2023
@sebastianbergmann
Copy link
Owner Author

@ArturGoldyn IIRC, you were the original author of this functionality. May I bother you to have a look? Thanks!

@sebastianbergmann
Copy link
Owner Author

I fail to see how this could have ever worked and wonder whether it actually ever did. I will stop looking into this issue now. I am open to review a pull request that fixes this issue, but at this point I am inclined to deprecate the functionality controlled by the @runClassInSeparateProcess annotation and the #[RunClassInSeparateProcess] attribute in PHPUnit 10.1 and remove it in PHPUnit 11.

@WalterWoshid
Copy link

Hi @sebastianbergmann

I have looked into the code and it's quite complicated. I did some quick changes and wanted to ask if this is the right way to do it.

  1. I have removed the $runClassInSeparateProcess property and all it's methods from from the TestCase class, because I think a test should not decide if the class is run in a separate process. This will always have the same effect as $runTestInSeparateProcess
  2. I also removed all shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess methods from the TestBuilder for the same reason
  3. I modified the run method in the TestSuite class to check if the test is a TestSuite and check $test->shouldRunInSeparateProcess()
    image
    (I'll refactor the spaghetti later)
  4. I had to modify the TestRunner::runInSeparateProcess method to support TestSuite
  5. The test ran successfully, but 19 more tests are failing than before lol

@WalterWoshid
Copy link

It seems that some of the tests fail, because the option --process-isolation was passed to argv and my new logic also checks for this option. Should this option isolate only all methods or the whole class? Should we create a new option --process-isolation-class or --class-isolation?

WalterWoshid added a commit to WalterWoshid/phpunit that referenced this issue Feb 22, 2023
WalterWoshid added a commit to WalterWoshid/phpunit that referenced this issue Feb 22, 2023
@sebastianbergmann sebastianbergmann changed the title @runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses @runClassInSeparateProcess has the same effect as @runTestsInSeparateProcesses Mar 6, 2023
WalterWoshid added a commit to WalterWoshid/phpunit that referenced this issue Jan 16, 2024
WalterWoshid added a commit to WalterWoshid/phpunit that referenced this issue Jan 16, 2024
WalterWoshid added a commit to WalterWoshid/phpunit that referenced this issue Jan 16, 2024
WalterWoshid added a commit to WalterWoshid/phpunit that referenced this issue Jan 16, 2024
@sebastianbergmann sebastianbergmann added version/11 Something affects PHPUnit 11 and removed version/9 Something affects PHPUnit 9 labels Feb 2, 2024
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 feature/test-runner CLI test runner type/bug Something is broken version/10 Something affects PHPUnit 10 version/11 Something affects PHPUnit 11
Projects
None yet
Development

No branches or pull requests

2 participants