From 406eed2997bae21c8c18b08bf3c1ca3af536ea1e Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 14 Jun 2023 14:57:49 +0200 Subject: [PATCH] Implement dataset parallelization with `--functional` flag (#770) --- README.md | 10 +--- bin/phpunit-wrapper.php | 2 +- phpstan-baseline.neon | 5 -- src/Options.php | 12 +++++ .../ApplicationForWrapperWorker.php | 30 ++++++++++- src/WrapperRunner/SuiteLoader.php | 47 ++++++++++++---- src/WrapperRunner/WrapperRunner.php | 2 +- src/WrapperRunner/WrapperWorker.php | 4 ++ test/Unit/WrapperRunner/SuiteLoaderTest.php | 6 +-- test/Unit/WrapperRunner/WrapperRunnerTest.php | 53 +++++++++++++------ .../FunctionalParallelizationTest.php | 43 +++++++++++++++ .../Regression.phpt | 7 +++ test/fixtures/phpt/my_test.phpt | 2 +- 13 files changed, 175 insertions(+), 48 deletions(-) create mode 100644 test/fixtures/function_parallelization_tests/FunctionalParallelizationTest.php create mode 100644 test/fixtures/function_parallelization_tests/Regression.phpt diff --git a/README.md b/README.md index aaa7ac25..59bcb109 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ start using it with no additional bootstrap or configurations! Benefits: -* Zero configuration. After the installation, run with `vendor/bin/paratest`. That's it! +* Zero configuration. After the installation, run with `vendor/bin/paratest` to parallelize by TestCase or `vendor/bin/paratest --functional` to parallelize by Test. That's it! * Code Coverage report combining. Run your tests in N parallel processes and all the code coverage output will be combined into one report. # Installation @@ -72,14 +72,6 @@ If you have `xDebug` installed, activating it by the environment variable is eno XDEBUG_MODE=coverage vendor/bin/paratest ``` -### PHPDBG - -`PHPDBG` is automatically detected and used in the subprocesses if it's the running binary of the main process: - -``` -phpdbg vendor/bin/paratest -``` - ## Initial setup for all tests Because ParaTest runs multiple processes in parallel, each with their own instance of the PHP interpreter, diff --git a/bin/phpunit-wrapper.php b/bin/phpunit-wrapper.php index 6cd44e1f..73e47ea8 100644 --- a/bin/phpunit-wrapper.php +++ b/bin/phpunit-wrapper.php @@ -69,7 +69,7 @@ } // It must be a 1 byte string to ensure filesize() is equal to the number of tests executed - $exitCode = $application->runTest(trim($testPath)); + $exitCode = $application->runTest(trim($testPath, "\n")); fwrite($statusFile, (string) $exitCode); fflush($statusFile); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 506942d9..17e6d6a5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -20,11 +20,6 @@ parameters: count: 1 path: src/WrapperRunner/ResultPrinter.php - - - message: "#^Property ParaTest\\\\WrapperRunner\\\\SuiteLoader\\:\\:\\$files \\(array\\\\) does not accept array\\\\.$#" - count: 1 - path: src/WrapperRunner/SuiteLoader.php - - message: "#^Unreachable statement \\- code above always terminates\\.$#" count: 1 diff --git a/src/Options.php b/src/Options.php index 76ff4672..d88679ed 100644 --- a/src/Options.php +++ b/src/Options.php @@ -99,6 +99,7 @@ private function __construct( public readonly string $runner, public readonly string $tmpDir, public readonly bool $verbose, + public readonly bool $functional, ) { $this->needsTeamcity = $configuration->outputIsTeamCity() || $configuration->hasLogfileTeamcity(); } @@ -137,6 +138,10 @@ public static function fromConsoleInput(InputInterface $input, string $cwd): sel $verbose = $options['verbose']; unset($options['verbose']); + assert(is_bool($options['functional'])); + $functional = $options['functional']; + unset($options['functional']); + assert(array_key_exists('colors', $options)); if ($options['colors'] === Configuration::COLOR_DEFAULT) { unset($options['colors']); @@ -192,6 +197,7 @@ public static function fromConsoleInput(InputInterface $input, string $cwd): sel $runner, $tmpDir, $verbose, + $functional, ); } @@ -206,6 +212,12 @@ public static function setInputDefinition(InputDefinition $inputDefinition): voi ), // ParaTest options + new InputOption( + 'functional', + null, + InputOption::VALUE_NONE, + 'Whether to enable functional testing, for unit and dataset parallelization', + ), new InputOption( 'max-batch-size', 'm', diff --git a/src/WrapperRunner/ApplicationForWrapperWorker.php b/src/WrapperRunner/ApplicationForWrapperWorker.php index 1ce46ce2..665df8d7 100644 --- a/src/WrapperRunner/ApplicationForWrapperWorker.php +++ b/src/WrapperRunner/ApplicationForWrapperWorker.php @@ -16,6 +16,7 @@ use PHPUnit\Runner\Extension\ExtensionBootstrapper; use PHPUnit\Runner\Extension\Facade as ExtensionFacade; use PHPUnit\Runner\Extension\PharLoader; +use PHPUnit\Runner\Filter\Factory; use PHPUnit\Runner\TestSuiteLoader; use PHPUnit\Runner\TestSuiteSorter; use PHPUnit\TestRunner\TestResult\Facade as TestResultFacade; @@ -32,8 +33,12 @@ use function assert; use function file_put_contents; +use function is_file; use function mt_srand; use function serialize; +use function str_ends_with; +use function strpos; +use function substr; /** * @internal @@ -60,10 +65,23 @@ public function __construct( public function runTest(string $testPath): int { + $null = strpos($testPath, "\0"); + $filter = null; + if ($null !== false) { + $filter = new Factory(); + $filter->addNameFilter(substr($testPath, $null + 1)); + $testPath = substr($testPath, 0, $null); + } + $this->bootstrap(); - $testSuiteRefl = (new TestSuiteLoader())->load($testPath); - $testSuite = TestSuite::fromClassReflector($testSuiteRefl); + if (is_file($testPath) && str_ends_with($testPath, '.phpt')) { + $testSuite = TestSuite::empty($testPath); + $testSuite->addTestFile($testPath); + } else { + $testSuiteRefl = (new TestSuiteLoader())->load($testPath); + $testSuite = TestSuite::fromClassReflector($testSuiteRefl); + } (new TestSuiteFilterProcessor())->process($this->configuration, $testSuite); @@ -73,6 +91,14 @@ public function runTest(string $testPath): int ); } + if ($filter !== null) { + $testSuite->injectFilter($filter); + + EventFacade::emitter()->testSuiteFiltered( + TestSuiteBuilder::from($testSuite), + ); + } + EventFacade::emitter()->testRunnerExecutionStarted( TestSuiteBuilder::from($testSuite), ); diff --git a/src/WrapperRunner/SuiteLoader.php b/src/WrapperRunner/SuiteLoader.php index aad7c316..e1636a0a 100644 --- a/src/WrapperRunner/SuiteLoader.php +++ b/src/WrapperRunner/SuiteLoader.php @@ -4,6 +4,7 @@ namespace ParaTest\WrapperRunner; +use Generator; use ParaTest\Options; use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestSuite; @@ -23,6 +24,7 @@ use function array_keys; use function assert; use function count; +use function is_int; use function is_string; use function mt_srand; use function ob_get_clean; @@ -36,7 +38,7 @@ final class SuiteLoader { public readonly int $testCount; /** @var list */ - public readonly array $files; + public readonly array $tests; public function __construct( private readonly Options $options, @@ -73,8 +75,30 @@ public function __construct( $this->testCount = count($testSuite); $files = []; - $this->loadFiles($testSuite, $files); - $this->files = array_keys($files); + $tests = []; + foreach ($this->loadFiles($testSuite) as $file => $test) { + $files[$file] = null; + + if ($test instanceof PhptTestCase) { + $tests[] = $file; + } else { + $name = $test->name(); + if ($test->providedData() !== []) { + $dataName = $test->dataName(); + if (is_int($dataName)) { + $name .= '#' . $dataName; + } else { + $name .= '@' . $dataName; + } + } + + $tests[] = "$file\0$name"; + } + } + + $this->tests = $this->options->functional + ? $tests + : array_keys($files); if (! $this->options->configuration->hasCoverageReport()) { return; @@ -94,12 +118,13 @@ public function __construct( } } - /** @param array $files */ - private function loadFiles(TestSuite $testSuite, array &$files): void + /** @return Generator */ + private function loadFiles(TestSuite $testSuite): Generator { foreach ($testSuite as $test) { if ($test instanceof TestSuite) { - $this->loadFiles($test, $files); + yield from $this->loadFiles($test); + continue; } @@ -107,8 +132,9 @@ private function loadFiles(TestSuite $testSuite, array &$files): void $refProperty = new ReflectionProperty(PhptTestCase::class, 'filename'); $filename = $refProperty->getValue($test); assert(is_string($filename) && $filename !== ''); - $filename = $this->stripCwd($filename); - $files[$filename] = true; + $filename = $this->stripCwd($filename); + + yield $filename => $test; continue; } @@ -117,8 +143,9 @@ private function loadFiles(TestSuite $testSuite, array &$files): void $refClass = new ReflectionClass($test); $filename = $refClass->getFileName(); assert(is_string($filename) && $filename !== ''); - $filename = $this->stripCwd($filename); - $files[$filename] = true; + $filename = $this->stripCwd($filename); + + yield $filename => $test; continue; } diff --git a/src/WrapperRunner/WrapperRunner.php b/src/WrapperRunner/WrapperRunner.php index 37dea913..3e550d83 100644 --- a/src/WrapperRunner/WrapperRunner.php +++ b/src/WrapperRunner/WrapperRunner.php @@ -109,7 +109,7 @@ public function run(): int ); $result = TestResultFacade::result(); - $this->pending = $suiteLoader->files; + $this->pending = $suiteLoader->tests; $this->printer->setTestCount($suiteLoader->testCount); $this->printer->start(); $this->startWorkers(); diff --git a/src/WrapperRunner/WrapperWorker.php b/src/WrapperRunner/WrapperWorker.php index 2b58dc34..46e10789 100644 --- a/src/WrapperRunner/WrapperWorker.php +++ b/src/WrapperRunner/WrapperWorker.php @@ -105,6 +105,10 @@ public function __construct( $phpunitArguments = [$options->phpunit]; foreach ($options->phpunitOptions as $key => $value) { + if ($options->functional && $key === 'filter') { + continue; + } + $phpunitArguments[] = "--{$key}"; if ($value === true) { continue; diff --git a/test/Unit/WrapperRunner/SuiteLoaderTest.php b/test/Unit/WrapperRunner/SuiteLoaderTest.php index f1bd3102..7dcaea18 100644 --- a/test/Unit/WrapperRunner/SuiteLoaderTest.php +++ b/test/Unit/WrapperRunner/SuiteLoaderTest.php @@ -33,14 +33,14 @@ public function testLoadTestsuiteFileFromConfig(): void $loader = $this->loadSuite(); self::assertSame(7, $loader->testCount); - self::assertCount(7, $loader->files); + self::assertCount(7, $loader->tests); } public function testLoadFileGetsPathOfFile(): void { $path = $this->fixture('common_results' . DIRECTORY_SEPARATOR . 'SuccessTest.php'); $this->bareOptions['path'] = $path; - $files = $this->loadSuite()->files; + $files = $this->loadSuite()->tests; $file = array_shift($files); self::assertNotNull($file); @@ -61,7 +61,7 @@ public function testCacheIsWarmedWhenSpecified(): void public function testLoadsPhptFiles(): void { $this->bareOptions['path'] = $this->fixture('phpt'); - $files = $this->loadSuite()->files; + $files = $this->loadSuite()->tests; $file = array_shift($files); self::assertNotNull($file); diff --git a/test/Unit/WrapperRunner/WrapperRunnerTest.php b/test/Unit/WrapperRunner/WrapperRunnerTest.php index f39a8b46..174473fd 100644 --- a/test/Unit/WrapperRunner/WrapperRunnerTest.php +++ b/test/Unit/WrapperRunner/WrapperRunnerTest.php @@ -14,6 +14,7 @@ use ParaTest\WrapperRunner\WrapperWorker; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversNothing; +use PHPUnit\Framework\Attributes\RequiresPhp; use SebastianBergmann\CodeCoverage\CodeCoverage; use Symfony\Component\Process\Process; @@ -119,7 +120,7 @@ public function testReadPhpunitConfigPhpSectionBeforeLoadingTheSuite(): void { $this->bareOptions['--configuration'] = $this->fixture('github' . DIRECTORY_SEPARATOR . 'GH420' . DIRECTORY_SEPARATOR . 'phpunit.xml'); $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertEquals(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); } public function testRunnerSortTestEqualBySeed(): void @@ -196,14 +197,14 @@ public function testParatestEnvironmentVariableWithWrapperRunnerWithoutTestToken $runnerResult = $this->runRunner(); self::assertStringContainsString('Failures: 1', $runnerResult->output); - self::assertEquals(RunnerInterface::FAILURE_EXIT, $runnerResult->exitCode); + self::assertSame(RunnerInterface::FAILURE_EXIT, $runnerResult->exitCode); } public function testParatestEnvironmentVariable(): void { $this->bareOptions['path'] = $this->fixture('paratest_only_tests' . DIRECTORY_SEPARATOR . 'EnvironmentTest.php'); - self::assertEquals(0, $this->runRunner()->exitCode); + self::assertSame(0, $this->runRunner()->exitCode); } public function testPassthrus(): void @@ -219,7 +220,7 @@ public function testPassthrus(): void } $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); } /** @group github */ @@ -230,7 +231,7 @@ public function testReadPhpunitConfigPhpSectionBeforeLoadingTheSuiteManualBootst $this->bareOptions['--bootstrap'] = $this->fixture('github' . DIRECTORY_SEPARATOR . 'GH420bis' . DIRECTORY_SEPARATOR . 'bootstrap.php'); $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); } public function testTeamcityOutput(): void @@ -248,7 +249,7 @@ public function testExitCodesPathWithoutTests(): void $this->bareOptions['path'] = $this->fixture('no_tests'); $runnerResult = $this->runRunner(); - self::assertEquals(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); } /** @requires OSFAMILY Linux */ @@ -354,26 +355,26 @@ public function testExitCodes(): void $runnerResult = $this->runRunner(); self::assertStringContainsString('Errors: 1', $runnerResult->output); - self::assertEquals(RunnerInterface::EXCEPTION_EXIT, $runnerResult->exitCode); + self::assertSame(RunnerInterface::EXCEPTION_EXIT, $runnerResult->exitCode); $this->bareOptions['path'] = $this->fixture('common_results' . DIRECTORY_SEPARATOR . 'FailureTest.php'); $runnerResult = $this->runRunner(); self::assertStringContainsString('Failures: 1', $runnerResult->output); - self::assertEquals(RunnerInterface::FAILURE_EXIT, $runnerResult->exitCode); + self::assertSame(RunnerInterface::FAILURE_EXIT, $runnerResult->exitCode); $this->bareOptions['path'] = $this->fixture('common_results' . DIRECTORY_SEPARATOR . 'SuccessTest.php'); $runnerResult = $this->runRunner(); self::assertStringContainsString('OK', $runnerResult->output); - self::assertEquals(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); $this->bareOptions['path'] = $this->fixture('common_results'); $runnerResult = $this->runRunner(); self::assertStringContainsString('Failures: 1', $runnerResult->output); self::assertStringContainsString('Errors: 1', $runnerResult->output); - self::assertEquals(RunnerInterface::EXCEPTION_EXIT, $runnerResult->exitCode); + self::assertSame(RunnerInterface::EXCEPTION_EXIT, $runnerResult->exitCode); } public function testWritesLogWithEmptyNameWhenPathIsNotProvided(): void @@ -426,7 +427,7 @@ public function testTokensAreAbsentWhenNoTestTokensIsSpecified(): void $this->bareOptions['path'] = $this->fixture('github' . DIRECTORY_SEPARATOR . 'GH505'); $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); } /** @@ -443,7 +444,7 @@ public function testRandomnessIsDeterministic(): void ]; $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); } public function testTeamcityLog(): void @@ -468,7 +469,7 @@ public function testRunningFewerTestsThanTheWorkersIsPossible(): void $this->bareOptions['--processes'] = '10'; $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); $glob = glob($this->tmpDir . '/*'); self::assertNotFalse($glob); self::assertCount(0, $glob); @@ -482,7 +483,7 @@ public function testResultsAreCorrect(): void $this->bareOptions['--cache-directory'] = $this->tmpDir; $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); $coveragePhp = include $this->bareOptions['--coverage-php']; self::assertInstanceOf(CodeCoverage::class, $coveragePhp); @@ -511,7 +512,7 @@ public function testIgnoreAttributes(): void Methods: 100.00% ( 1/ 1) Lines: 100.00% ( 1/ 1) EOF; - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); self::assertStringContainsString($expectedContains, $runnerResult->output); } @@ -558,12 +559,32 @@ public function testHandleUnexpectedOutput(): void self::assertStringMatchesFormat($expectedOutput, $runnerResult->output); } + /** + * \PHPUnit\Runner\Filter\NameFilterIterator uses `preg_match`, and in + * \ParaTest\Tests\fixtures\function_parallelization_tests\FunctionalParallelizationTest::dataProvider2 + * on the second data name we explicitly test a NULL-byte for our internal implementation, but + * NULL-byte isn't supported in PHP < 8.2 + * + * @see https://bugs.php.net/bug.php?id=77726 + * @see https://github.com/php/php-src/pull/8114 + */ + #[RequiresPhp('8.2')] + public function testFunctionalParallelization(): void + { + $this->bareOptions['path'] = $this->fixture('function_parallelization_tests'); + $this->bareOptions['--functional'] = true; + + $runnerResult = $this->runRunner(); + self::assertStringContainsString('.......', $runnerResult->output); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); + } + public function testProcessIsolation(): void { $this->bareOptions['path'] = $this->fixture('process_isolation' . DIRECTORY_SEPARATOR . 'FooTest.php'); $this->bareOptions['--process-isolation'] = true; $runnerResult = $this->runRunner(); - self::assertEquals(0, $runnerResult->exitCode); + self::assertSame(RunnerInterface::SUCCESS_EXIT, $runnerResult->exitCode); } } diff --git a/test/fixtures/function_parallelization_tests/FunctionalParallelizationTest.php b/test/fixtures/function_parallelization_tests/FunctionalParallelizationTest.php new file mode 100644 index 00000000..3436435f --- /dev/null +++ b/test/fixtures/function_parallelization_tests/FunctionalParallelizationTest.php @@ -0,0 +1,43 @@ + */ + public static function dataProvider1(): array + { + return [ + ['a', 'a'], + ['b', 'b'], + ['c', 'c'], + ]; + } + + /** @return array */ + public static function dataProvider2(): array + { + return [ + 'test1 with spaces' => ['a', 'a'], + "test2 with \0" => ['b', 'b'], + 'test3' => ['c', 'c'], + ]; + } + + /** @dataProvider dataProvider1 */ + public function testDataProvider1(string $a, string $b): void + { + self::assertEquals($a, $b); + } + + /** @dataProvider dataProvider2 */ + public function testDataProvider2(string $a, string $b): void + { + self::assertEquals($a, $b); + } +} diff --git a/test/fixtures/function_parallelization_tests/Regression.phpt b/test/fixtures/function_parallelization_tests/Regression.phpt new file mode 100644 index 00000000..3d6e7bd0 --- /dev/null +++ b/test/fixtures/function_parallelization_tests/Regression.phpt @@ -0,0 +1,7 @@ +--TEST-- +var_dump +--FILE-- +