mvorisek
commented
1 year ago I confirm this problem is still present in the latest phpunit 9.x (9.6.8, 10.x probably too, untested).
The @requires must be evaluated before a new process is started.
Open GlennM opened 4 years ago
mvorisek
commented
1 year ago I confirm this problem is still present in the latest phpunit 9.x (9.6.8, 10.x probably too, untested).
The @requires must be evaluated before a new process is started.
kubawerlos
commented
1 week ago I confirm this problem is still present in the latest phpunit 9.x (9.6.8, 10.x probably too, untested).
@mvorisek I can't replicate it, what am I doing wrong here?
kub@:~/code/sebastianbergmann/phpunit(10.5)$ git remote -v
upstram git@github.com:sebastianbergmann/phpunit.git (fetch)
upstram git@github.com:sebastianbergmann/phpunit.git (push)
kub@:~/code/sebastianbergmann/phpunit(10.5)$ git checkout 9.6.8
// (...)
HEAD is now at 17d621b3a Prepare release
kub@:~/code/sebastianbergmann/phpunit((HEAD detached at 9.6.8))$ composer install -q
kub@:~/code/sebastianbergmann/phpunit((HEAD detached at 9.6.8))$ cat test.php
<?php
class Test extends \PHPUnit\Framework\TestCase
{
/**
* @runInSeparateProcess
*
* @requires PHP < 7.4
*/
public function testSomething(): void
{
self::assertTrue(false);
}
}
kub@:~/code/sebastianbergmann/phpunit((HEAD detached at 9.6.8))$ cat test.php
<?php
class Test extends \PHPUnit\Framework\TestCase
{
/**
* @runInSeparateProcess
*
* @requires PHP < 7.4
*/
public function testSomething(): void
{
self::assertTrue(false);
}
}
kub@:~/code/sebastianbergmann/phpunit((HEAD detached at 9.6.8))$ php phpunit test.php
PHPUnit 9.6.8 by Sebastian Bergmann and contributors.
Runtime: PHP 8.3.11
Configuration: /home/kuba/code/sebastianbergmann/phpunit/phpunit.xml
S 1 / 1 (100%)
Time: 00:00.037, Memory: 6.00 MB
There was 1 skipped test:
1) Test::testSomething
PHP < 7.4 is required.
/home/kuba/code/sebastianbergmann/phpunit/test.php:7
/home/kuba/code/sebastianbergmann/phpunit/test.php:7
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestCase.php:2128
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestCase.php:1195
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestResult.php:728
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestCase.php:964
OK, but incomplete, skipped, or risky tests!
Tests: 1, Assertions: 0, Skipped: 1.@kubawerlos try with phpunit started using php -d disable_functions=exec, passthru, proc_open, shell_exec, system, popen
You should observe that the tests in a separate process still start the process.
kubawerlos
commented
1 week ago How exactly?
kub@:~/code/sebastianbergmann/phpunit((HEAD detached at 9.6.8))$ php -d disable_functions=exec,passthru,proc_open,shell_exec,system,popen phpuni
t test.php
Call to undefined function proc_open()
kub@:~/code/sebastianbergmann/phpunit((HEAD detached at 9.6.8))$ php -d disable_functions=exec,passthru,shell_exec,system,popen phpunit test.php
PHPUnit 9.6.8 by Sebastian Bergmann and contributors.
Runtime: PHP 8.3.11
Configuration: /home/kuba/code/sebastianbergmann/phpunit/phpunit.xml
S 1 / 1 (100%)
Time: 00:00.047, Memory: 6.00 MB
There was 1 skipped test:
1) Test::testSomething
PHP < 7.4 is required.
/home/kuba/code/sebastianbergmann/phpunit/test.php:7
/home/kuba/code/sebastianbergmann/phpunit/test.php:7
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestCase.php:2128
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestCase.php:1195
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestResult.php:728
/home/kuba/code/sebastianbergmann/phpunit/src/Framework/TestCase.php:964
OK, but incomplete, skipped, or risky tests!
Tests: 1, Assertions: 0, Skipped: 1.Based on your post it seems proc_open is used by phpunit to open a new process. And it seems you was able to replicate the issue - if all tests that requires a new process would be skipped, no proc_open would be needed.
kubawerlos
commented
1 week ago if all tests that requires a new process would be skipped, no
proc_openwould be needed.
not necessary, proc_open can still be used and the test is skipped, as in the php -d disable_functions=exec,passthru,shell_exec,system,popen phpunit test.php attempt.
My understanding of the report is that when @runInSeparateProcess and @requires are used together, the latter is ignored, test is executed and fails on self::assertTrue(false);, which is not happening.
mvorisek
commented
1 week ago See https://github.com/sebastianbergmann/phpunit/issues/4391#issuecomment-1547311304 on my opinion on this, I belive the @requires must be evaluated before the process is started, as in edge case, no process could be even started and if so, it is slow, as a new process start takes some milliseconds which might sum up when there are many tests to be skipped.
kubawerlos
commented
1 week ago Test should not be executed when condition is false.
This is the original report and it behaves correctly.
The
@requiresmust be evaluated before a new process is started.
Why? Is it some good practice that I am not aware of? @sebastianbergmann do you agree on 👆🏼 and this is actually a bug?
Summary
@runInSeparateProcessdoesn't respect@requiresCurrent behavior
Test with
@runInSeparateProcessgets executed while the condition for@requiresis falseHow to reproduce
Expected behavior
Test should not be executed when condition is false.