From 546f1588fdf867544d7f7353963445d3d6d78997 Mon Sep 17 00:00:00 2001 From: WalterWoshid Date: Tue, 16 Jan 2024 15:35:30 +0100 Subject: [PATCH] Fix #5230 --- src/Framework/TestBuilder.php | 19 +----- src/Framework/TestCase.php | 14 ---- src/Framework/TestRunner.php | 67 +++++++++++++------ src/Framework/TestSuite.php | 67 ++++++++++++++++--- src/Util/PHP/AbstractPhpProcess.php | 61 ++++++++++------- src/Util/PHP/Template/TestCaseClass.tpl | 47 ++++++++----- src/Util/PHP/Template/TestCaseMethod.tpl | 4 +- ...SeparateClassRunMethodInNewProcessTest.php | 5 +- 8 files changed, 178 insertions(+), 106 deletions(-) diff --git a/src/Framework/TestBuilder.php b/src/Framework/TestBuilder.php index 8e5e019dfe..b7db75e0d5 100644 --- a/src/Framework/TestBuilder.php +++ b/src/Framework/TestBuilder.php @@ -47,7 +47,6 @@ public function build(ReflectionClass $theClass, string $methodName): Test $data, $this->shouldTestMethodBeRunInSeparateProcess($className, $methodName), $this->shouldGlobalStateBePreserved($className, $methodName), - $this->shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess($className), $this->backupSettings($className, $methodName), ); } @@ -60,7 +59,6 @@ public function build(ReflectionClass $theClass, string $methodName): Test $test, $this->shouldTestMethodBeRunInSeparateProcess($className, $methodName), $this->shouldGlobalStateBePreserved($className, $methodName), - $this->shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess($className), $this->backupSettings($className, $methodName), ); @@ -72,7 +70,7 @@ public function build(ReflectionClass $theClass, string $methodName): Test * @psalm-param non-empty-string $methodName * @psalm-param array{backupGlobals: ?bool, backupGlobalsExcludeList: list, backupStaticProperties: ?bool, backupStaticPropertiesExcludeList: array>} $backupSettings */ - private function buildDataProviderTestSuite(string $methodName, string $className, array $data, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, array $backupSettings): DataProviderTestSuite + private function buildDataProviderTestSuite(string $methodName, string $className, array $data, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, array $backupSettings): DataProviderTestSuite { $dataProviderTestSuite = DataProviderTestSuite::empty( $className . '::' . $methodName, @@ -91,7 +89,6 @@ private function buildDataProviderTestSuite(string $methodName, string $classNam $_test, $runTestInSeparateProcess, $preserveGlobalState, - $runClassInSeparateProcess, $backupSettings, ); @@ -104,16 +101,12 @@ private function buildDataProviderTestSuite(string $methodName, string $classNam /** * @psalm-param array{backupGlobals: ?bool, backupGlobalsExcludeList: list, backupStaticProperties: ?bool, backupStaticPropertiesExcludeList: array>} $backupSettings */ - private function configureTestCase(TestCase $test, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, array $backupSettings): void + private function configureTestCase(TestCase $test, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, array $backupSettings): void { if ($runTestInSeparateProcess) { $test->setRunTestInSeparateProcess(true); } - if ($runClassInSeparateProcess) { - $test->setRunClassInSeparateProcess(true); - } - if ($preserveGlobalState !== null) { $test->setPreserveGlobalState($preserveGlobalState); } @@ -258,12 +251,4 @@ private function shouldTestMethodBeRunInSeparateProcess(string $className, strin return false; } - - /** - * @psalm-param class-string $className - */ - private function shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess(string $className): bool - { - return MetadataRegistry::parser()->forClass($className)->isRunClassInSeparateProcess()->isNotEmpty(); - } } diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index 021336f305..f706acc985 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -126,7 +126,6 @@ abstract class TestCase extends Assert implements Reorderable, SelfDescribing, T */ private array $backupStaticPropertiesExcludeList = []; private ?Snapshot $snapshot = null; - private ?bool $runClassInSeparateProcess = null; private ?bool $runTestInSeparateProcess = null; private bool $preserveGlobalState = false; private bool $inIsolation = false; @@ -489,7 +488,6 @@ final public function run(): void } else { (new TestRunner)->runInSeparateProcess( $this, - $this->runClassInSeparateProcess && !$this->runTestInSeparateProcess, $this->preserveGlobalState, ); } @@ -885,14 +883,6 @@ final public function setRunTestInSeparateProcess(bool $runTestInSeparateProcess } } - /** - * @internal This method is not covered by the backward compatibility promise for PHPUnit - */ - final public function setRunClassInSeparateProcess(bool $runClassInSeparateProcess): void - { - $this->runClassInSeparateProcess = $runClassInSeparateProcess; - } - /** * @internal This method is not covered by the backward compatibility promise for PHPUnit */ @@ -1942,10 +1932,6 @@ private function shouldRunInSeparateProcess(): bool return true; } - if ($this->runClassInSeparateProcess) { - return true; - } - return ConfigurationRegistry::get()->processIsolation(); } diff --git a/src/Framework/TestRunner.php b/src/Framework/TestRunner.php index c7f31d7c1d..8f05dd2e94 100644 --- a/src/Framework/TestRunner.php +++ b/src/Framework/TestRunner.php @@ -10,6 +10,7 @@ namespace PHPUnit\Framework; use const PHP_EOL; +use function array_merge; use function assert; use function class_exists; use function defined; @@ -253,11 +254,13 @@ public function run(TestCase $test): void * @throws ProcessIsolationException * @throws StaticAnalysisCacheNotConfiguredException */ - public function runInSeparateProcess(TestCase $test, bool $runEntireClass, bool $preserveGlobalState): void + public function runInSeparateProcess(TestCase|TestSuite $test, bool $preserveGlobalState): void { $class = new ReflectionClass($test); - if ($runEntireClass) { + $isTestSuite = $test instanceof TestSuite; + + if ($isTestSuite) { $template = new Template( __DIR__ . '/../Util/PHP/Template/TestCaseClass.tpl', ); @@ -300,21 +303,49 @@ public function runInSeparateProcess(TestCase $test, bool $runEntireClass, bool $phar = '\'\''; } - $data = var_export(serialize($test->providedData()), true); - $dataName = var_export($test->dataName(), true); - $dependencyInput = var_export(serialize($test->dependencyInput()), true); - $includePath = var_export(get_include_path(), true); - // must do these fixes because TestCaseMethod.tpl has unserialize('{data}') in it, and we can't break BC - // the lines above used to use addcslashes() rather than var_export(), which breaks null byte escape sequences - $data = "'." . $data . ".'"; - $dataName = "'.(" . $dataName . ").'"; - $dependencyInput = "'." . $dependencyInput . ".'"; - $includePath = "'." . $includePath . ".'"; + $tests = $test instanceof TestSuite ? $test->tests() : [$test]; + + $var = [ + 'testData' => [], + ]; + + $testData = []; + + foreach ($tests as $t) { + assert($t instanceof TestCase); + + $data = serialize($t->providedData()); + $dataName = serialize($t->dataName()); + $dependencyInput = serialize($t->dependencyInput()); + + $methodName = $t->name(); + $className = $t::class; + + $testData[] = [ + 'data' => $data, + 'dataName' => $dataName, + 'dependencyInput' => $dependencyInput, + 'methodName' => $methodName, + 'className' => $className, + ]; + } + + if ($isTestSuite) { + foreach ($testData as $data) { + $var['testData'][] = $data; + } + $var['testData'] = serialize($var['testData']); + } else { + $var = $testData[0]; + } + + $includePath = serialize(get_include_path()); + $offset = hrtime(); $serializedConfiguration = $this->saveConfigurationForChildProcess(); $processResultFile = tempnam(sys_get_temp_dir(), 'phpunit_'); - $var = [ + $var = array_merge($var, [ 'bootstrap' => $bootstrap, 'composerAutoload' => $composerAutoload, 'phar' => $phar, @@ -322,25 +353,17 @@ public function runInSeparateProcess(TestCase $test, bool $runEntireClass, bool 'className' => $class->getName(), 'collectCodeCoverageInformation' => $coverage, 'linesToBeIgnored' => $linesToBeIgnored, - 'data' => $data, - 'dataName' => $dataName, - 'dependencyInput' => $dependencyInput, 'constants' => $constants, 'globals' => $globals, 'include_path' => $includePath, 'included_files' => $includedFiles, 'iniSettings' => $iniSettings, - 'name' => $test->name(), 'offsetSeconds' => $offset[0], 'offsetNanoseconds' => $offset[1], 'serializedConfiguration' => $serializedConfiguration, 'processResultFile' => $processResultFile, 'exportObjects' => $exportObjects, - ]; - - if (!$runEntireClass) { - $var['methodName'] = $test->name(); - } + ]); $template->setVar($var); diff --git a/src/Framework/TestSuite.php b/src/Framework/TestSuite.php index 15a144cfb2..414bd93e1a 100644 --- a/src/Framework/TestSuite.php +++ b/src/Framework/TestSuite.php @@ -29,21 +29,25 @@ use PHPUnit\Event; use PHPUnit\Event\Code\TestMethod; use PHPUnit\Event\NoPreviousThrowableException; +use PHPUnit\Event\TestData\MoreThanOneDataSetFromDataProviderException; use PHPUnit\Metadata\Api\Dependencies; use PHPUnit\Metadata\Api\Groups; use PHPUnit\Metadata\Api\HookMethods; use PHPUnit\Metadata\Api\Requirements; use PHPUnit\Metadata\MetadataCollection; +use PHPUnit\Metadata\Parser\Registry as MetadataRegistry; use PHPUnit\Runner\Exception as RunnerException; use PHPUnit\Runner\Filter\Factory; use PHPUnit\Runner\PhptTestCase; use PHPUnit\Runner\TestSuiteLoader; use PHPUnit\TestRunner\TestResult\Facade as TestResultFacade; +use PHPUnit\Util\Exception as UtilException; use PHPUnit\Util\Filter; use PHPUnit\Util\Reflection; use PHPUnit\Util\Test as TestUtil; use ReflectionClass; use ReflectionMethod; +use SebastianBergmann\CodeCoverage\StaticAnalysisCacheNotConfiguredException; use SebastianBergmann\CodeCoverage\UnintentionallyCoveredCodeException; use Throwable; @@ -68,9 +72,10 @@ class TestSuite implements IteratorAggregate, Reorderable, SelfDescribing, Test /** * @psalm-var list */ - private array $tests = []; - private ?array $providedTests = null; - private ?Factory $iteratorFilter = null; + private array $tests = []; + private ?array $providedTests = null; + private ?Factory $iteratorFilter = null; + private ?bool $runClassInSeparateProcess = null; /** * @psalm-param non-empty-string $name @@ -134,6 +139,10 @@ public static function fromClassReflector(ReflectionClass $class): static ); } + if ($testSuite->shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess($class->getName())) { + $testSuite->setRunClassInSeparateProcess(true); + } + return $testSuite; } @@ -309,11 +318,17 @@ public function groupDetails(): array /** * @throws \SebastianBergmann\CodeCoverage\InvalidArgumentException + * @throws \SebastianBergmann\Template\InvalidArgumentException * @throws CodeCoverageException * @throws Event\RuntimeException * @throws Exception + * @throws MoreThanOneDataSetFromDataProviderException * @throws NoPreviousThrowableException + * @throws ProcessIsolationException + * @throws RunnerException + * @throws StaticAnalysisCacheNotConfiguredException * @throws UnintentionallyCoveredCodeException + * @throws UtilException */ public function run(): void { @@ -330,14 +345,22 @@ public function run(): void return; } - foreach ($this as $test) { - if (TestResultFacade::shouldStop()) { - $emitter->testRunnerExecutionAborted(); + if ($this->shouldRunInSeparateProcess()) { + (new TestRunner)->runInSeparateProcess($this, false); + } else { + foreach ($this as $test) { + if (TestResultFacade::shouldStop()) { + $emitter->testRunnerExecutionAborted(); - break; - } + break; + } - $test->run(); + if ($test instanceof self && $test->shouldRunInSeparateProcess()) { + (new TestRunner)->runInSeparateProcess($test, false); + } else { + $test->run(); + } + } } $this->invokeMethodsAfterLastTest($emitter); @@ -462,9 +485,18 @@ public function isForTestClass(): bool return class_exists($this->name, false) && is_subclass_of($this->name, TestCase::class); } + public function shouldRunInSeparateProcess(): bool + { + if ($this->runClassInSeparateProcess) { + return true; + } + + return false; + } + /** - * @throws \PHPUnit\Event\TestData\MoreThanOneDataSetFromDataProviderException * @throws Exception + * @throws MoreThanOneDataSetFromDataProviderException */ protected function addTestMethod(ReflectionClass $class, ReflectionMethod $method): void { @@ -679,4 +711,19 @@ private function invokeMethodsAfterLastTest(Event\Emitter $emitter): void ); } } + + private function setRunClassInSeparateProcess(bool $runClassInSeparateProcess): void + { + if ($this->runClassInSeparateProcess === null) { + $this->runClassInSeparateProcess = $runClassInSeparateProcess; + } + } + + /** + * @psalm-param class-string $className + */ + private function shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess(string $className): bool + { + return MetadataRegistry::parser()->forClass($className)->isRunClassInSeparateProcess()->isNotEmpty(); + } } diff --git a/src/Util/PHP/AbstractPhpProcess.php b/src/Util/PHP/AbstractPhpProcess.php index 7223d69563..1dd697c396 100644 --- a/src/Util/PHP/AbstractPhpProcess.php +++ b/src/Util/PHP/AbstractPhpProcess.php @@ -32,6 +32,7 @@ use PHPUnit\Framework\Exception; use PHPUnit\Framework\Test; use PHPUnit\Framework\TestCase; +use PHPUnit\Framework\TestSuite; use PHPUnit\Runner\CodeCoverage; use PHPUnit\TestRunner\TestResult\PassedTests; use SebastianBergmann\Environment\Runtime; @@ -147,8 +148,10 @@ public function runTestJob(string $job, Test $test, string $processResultFile): @unlink($processResultFile); } + $tests = $test instanceof TestSuite ? $test->tests() : [$test]; + $this->processChildResult( - $test, + $tests, $processResult, $_result['stderr'], ); @@ -224,22 +227,26 @@ protected function settingsToParameters(array $settings): string } /** + * @psalm-param array $tests + * * @throws \PHPUnit\Runner\Exception * @throws Exception * @throws MoreThanOneDataSetFromDataProviderException * @throws NoPreviousThrowableException */ - private function processChildResult(Test $test, string $stdout, string $stderr): void + private function processChildResult(array $tests, string $stdout, string $stderr): void { if (!empty($stderr)) { $exception = new Exception(trim($stderr)); - assert($test instanceof TestCase); + foreach ($tests as $test) { + assert($test instanceof TestCase); - Facade::emitter()->testErrored( - TestMethodBuilder::fromTestCase($test), - ThrowableBuilder::from($exception), - ); + Facade::emitter()->testErrored( + TestMethodBuilder::fromTestCase($test), + ThrowableBuilder::from($exception), + ); + } return; } @@ -262,17 +269,19 @@ static function (int $errno, string $errstr, string $errfile, int $errline): nev if ($childResult === false) { $exception = new AssertionFailedError('Test was run in child process and ended unexpectedly'); - assert($test instanceof TestCase); + foreach ($tests as $test) { + assert($test instanceof TestCase); - Facade::emitter()->testErrored( - TestMethodBuilder::fromTestCase($test), - ThrowableBuilder::from($exception), - ); + Facade::emitter()->testErrored( + TestMethodBuilder::fromTestCase($test), + ThrowableBuilder::from($exception), + ); - Facade::emitter()->testFinished( - TestMethodBuilder::fromTestCase($test), - 0, - ); + Facade::emitter()->testFinished( + TestMethodBuilder::fromTestCase($test), + 0, + ); + } } } catch (ErrorException $e) { restore_error_handler(); @@ -281,12 +290,14 @@ static function (int $errno, string $errstr, string $errfile, int $errline): nev $exception = new Exception(trim($stdout), 0, $e); - assert($test instanceof TestCase); + foreach ($tests as $test) { + assert($test instanceof TestCase); - Facade::emitter()->testErrored( - TestMethodBuilder::fromTestCase($test), - ThrowableBuilder::from($exception), - ); + Facade::emitter()->testErrored( + TestMethodBuilder::fromTestCase($test), + ThrowableBuilder::from($exception), + ); + } } if ($childResult !== false) { @@ -297,10 +308,12 @@ static function (int $errno, string $errstr, string $errfile, int $errline): nev Facade::instance()->forward($childResult['events']); PassedTests::instance()->import($childResult['passedTests']); - assert($test instanceof TestCase); + foreach ($tests as $test) { + assert($test instanceof TestCase); - $test->setResult($childResult['testResult']); - $test->addToAssertionCount($childResult['numAssertions']); + $test->setResult($childResult['testResult']); + $test->addToAssertionCount($childResult['numAssertions']); + } if (CodeCoverage::instance()->isActive() && $childResult['codeCoverage'] instanceof \SebastianBergmann\CodeCoverage\CodeCoverage) { CodeCoverage::instance()->codeCoverage()->merge( diff --git a/src/Util/PHP/Template/TestCaseClass.tpl b/src/Util/PHP/Template/TestCaseClass.tpl index f6d06695e0..23acb843c0 100644 --- a/src/Util/PHP/Template/TestCaseClass.tpl +++ b/src/Util/PHP/Template/TestCaseClass.tpl @@ -1,9 +1,9 @@ ignoreLines({linesToBeIgnored}); } - $test = new {className}('{name}'); + $output = ''; - $test->setData('{dataName}', unserialize('{data}')); - $test->setDependencyInput(unserialize('{dependencyInput}')); - $test->setInIsolation(true); + $tests = unserialize('{testData}'); + foreach ($tests as &$test) { + $className = $test['className']; + $methodName = $test['methodName']; + $dataName = unserialize($test['dataName']); + $data = unserialize($test['data']); + $dependencyInput = unserialize($test['dependencyInput']); - ob_end_clean(); + $test = new $className($methodName); - $test->run(); + assert($test instanceof TestCase); - $output = ''; + $test->setData($dataName, $data); + $test->setDependencyInput($dependencyInput); + $test->setInIsolation(true); + + @ob_end_clean(); - if (!$test->expectsOutput()) { - $output = $test->output(); + $test->run(); + + if (!$test->expectsOutput()) { + $output .= $test->output(); + } } ini_set('xdebug.scream', '0'); @@ -79,9 +90,10 @@ function __phpunit_run_isolated_test() } } - file_put_contents( - '{processResultFile}', - serialize( + $fileContents = ''; + + foreach ($tests as $test) { + $fileContents .= serialize( [ 'testResult' => $test->result(), 'codeCoverage' => {collectCodeCoverageInformation} ? CodeCoverage::instance()->codeCoverage() : null, @@ -90,7 +102,12 @@ function __phpunit_run_isolated_test() 'events' => $dispatcher->flush(), 'passedTests' => PassedTests::instance() ] - ) + ); + } + + file_put_contents( + '{processResultFile}', + $fileContents, ); } diff --git a/src/Util/PHP/Template/TestCaseMethod.tpl b/src/Util/PHP/Template/TestCaseMethod.tpl index b9eaef7362..b3c9b22586 100644 --- a/src/Util/PHP/Template/TestCaseMethod.tpl +++ b/src/Util/PHP/Template/TestCaseMethod.tpl @@ -16,7 +16,7 @@ if (!defined('STDOUT')) { {iniSettings} ini_set('display_errors', 'stderr'); -set_include_path('{include_path}'); +set_include_path(unserialize('{include_path}')); $composerAutoload = {composerAutoload}; $phar = {phar}; @@ -50,7 +50,7 @@ function __phpunit_run_isolated_test() $test = new {className}('{methodName}'); - $test->setData('{dataName}', unserialize('{data}')); + $test->setData(unserialize('{dataName}'), unserialize('{data}')); $test->setDependencyInput(unserialize('{dependencyInput}')); $test->setInIsolation(true); diff --git a/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php b/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php index 936d86a516..549bdb2483 100644 --- a/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php +++ b/tests/end-to-end/regression/2724/SeparateClassRunMethodInNewProcessTest.php @@ -24,8 +24,9 @@ final class SeparateClassRunMethodInNewProcessTest extends TestCase public const PROCESS_ID_FILE_PATH = __DIR__ . '/parent_process_id.txt'; public const INITIAL_PARENT_PROCESS_ID = 0; public const INITIAL_PROCESS_ID = 1; - public static $parentProcessId = self::INITIAL_PARENT_PROCESS_ID; - public static $processId = self::INITIAL_PROCESS_ID; + public static int $parentProcessId = self::INITIAL_PARENT_PROCESS_ID; + public static int $processId = self::INITIAL_PROCESS_ID; + public int $number = 1; public static function setUpBeforeClass(): void {