From 4b70abf4c2ffa08c64e2e89c7b5b7e43cdf26d52 Mon Sep 17 00:00:00 2001 From: CheapHasz Date: Thu, 29 Dec 2022 10:42:35 +0100 Subject: [PATCH] WrapperRunner: add `--max-batch-size` support (#714) Co-authored-by: CheapHasz Co-authored-by: Filippo Tessarotto --- README.md | 3 + composer.json | 8 +-- infection.json | 2 +- src/Runners/PHPUnit/WrapperRunner.php | 38 +++++++++-- .../Runners/PHPUnit/WrapperRunnerTest.php | 65 +++++++++++++++++++ .../phpunit-wrapper-batchsize-suite.xml | 8 +++ .../wrapper_batchsize_suite/FourTest.php | 9 +++ .../wrapper_batchsize_suite/OneTest.php | 9 +++ .../wrapper_batchsize_suite/ThreeTest.php | 9 +++ .../wrapper_batchsize_suite/TwoTest.php | 9 +++ .../WrapperBatchTestCase.php | 43 ++++++++++++ .../tmp/pid/.gitignore | 0 .../tmp/token/.gitignore | 0 13 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 test/fixtures/phpunit-wrapper-batchsize-suite.xml create mode 100644 test/fixtures/wrapper_batchsize_suite/FourTest.php create mode 100644 test/fixtures/wrapper_batchsize_suite/OneTest.php create mode 100644 test/fixtures/wrapper_batchsize_suite/ThreeTest.php create mode 100644 test/fixtures/wrapper_batchsize_suite/TwoTest.php create mode 100644 test/fixtures/wrapper_batchsize_suite/WrapperBatchTestCase.php create mode 100644 test/fixtures/wrapper_batchsize_suite/tmp/pid/.gitignore create mode 100644 test/fixtures/wrapper_batchsize_suite/tmp/token/.gitignore diff --git a/README.md b/README.md index 616a7f2c..fd27c607 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,8 @@ To get the most out of ParaTest, you have to adjust the parameters carefully. bootstrapping once and reuses these processes for each test executed. That way the overhead of process spawning and bootstrapping is reduced to the minimum. + Using the `--max-batch-size` option with the WrapperRunner will reset each worker after `--max-batch-size` testcases, which might help solve memory leaks problems. + 2. **Adjust the number of processes with `-p`** To allow full usage of your cpu cores, you should have at least one process per core. More processes allow better @@ -68,6 +70,7 @@ To get the most out of ParaTest, you have to adjust the parameters carefully. 4. **Tune batch max size `--max-batch-size`** Batch size will affect the max amount of atomic tests which will be used for a single test method. + Please note that it only works with either the `functional mode` OR `--runner WrapperRunner` (mutually exclusive). The following describes the `functional mode` system. One atomic test will be either one test method from test class if no data provider available for method or will be only one item from dataset for method. Increase this value to reduce per-process overhead and in most cases it will also reduce parallel efficiency. diff --git a/composer.json b/composer.json index 6d7dcf41..47327968 100644 --- a/composer.json +++ b/composer.json @@ -38,14 +38,14 @@ "ext-pcre": "*", "ext-reflection": "*", "ext-simplexml": "*", - "fidry/cpu-core-counter": "^0.4.0", + "fidry/cpu-core-counter": "^0.4.1", "jean85/pretty-package-versions": "^2.0.5", - "phpunit/php-code-coverage": "^9.2.19", + "phpunit/php-code-coverage": "^9.2.23", "phpunit/php-file-iterator": "^3.0.6", "phpunit/php-timer": "^5.0.3", "phpunit/phpunit": "^9.5.27", "sebastian/environment": "^5.1.4", - "symfony/console": "^5.4.16 || ^6.2.1", + "symfony/console": "^5.4.16 || ^6.2.3", "symfony/process": "^5.4.11 || ^6.2" }, "require-dev": { @@ -55,7 +55,7 @@ "infection/infection": "^0.26.16", "squizlabs/php_codesniffer": "^3.7.1", "symfony/filesystem": "^5.4.13 || ^6.2", - "vimeo/psalm": "^5.1" + "vimeo/psalm": "^5.4" }, "autoload": { "psr-4": { diff --git a/infection.json b/infection.json index ef1a0948..e72ee937 100644 --- a/infection.json +++ b/infection.json @@ -4,7 +4,7 @@ "src" ] }, - "timeout": 10, + "timeout": 30, "logs": { "stryker": { "report": "/^\\d+\\.x$/" diff --git a/src/Runners/PHPUnit/WrapperRunner.php b/src/Runners/PHPUnit/WrapperRunner.php index 00a7c7c2..976c30c7 100644 --- a/src/Runners/PHPUnit/WrapperRunner.php +++ b/src/Runners/PHPUnit/WrapperRunner.php @@ -17,9 +17,12 @@ /** @internal */ final class WrapperRunner extends BaseRunner { - /** @var WrapperWorker[] */ + /** @var array */ private $workers = []; + /** @var array */ + private $batches = []; + protected function beforeLoadChecks(): void { if ($this->options->functional()) { @@ -40,8 +43,7 @@ protected function doRun(): void private function startWorkers(): void { for ($token = 1; $token <= $this->options->processes(); ++$token) { - $this->workers[$token] = new WrapperWorker($this->output, $this->options, $token); - $this->workers[$token]->start(); + $this->startWorker($token); } } @@ -49,9 +51,10 @@ private function assignAllPendingTests(): void { $phpunit = $this->options->phpunit(); $phpunitOptions = $this->options->filtered(); + $batchSize = $this->options->maxBatchSize(); while (count($this->pending) > 0 && count($this->workers) > 0) { - foreach ($this->workers as $worker) { + foreach ($this->workers as $token => $worker) { if (! $worker->isRunning()) { throw $worker->getWorkerCrashedException(); } @@ -61,10 +64,17 @@ private function assignAllPendingTests(): void } $this->flushWorker($worker); + + if ($batchSize !== null && $batchSize !== 0 && $this->batches[$token] === $batchSize) { + $this->destroyWorker($token); + $worker = $this->startWorker($token); + } + if ($this->exitcode > 0 && $this->options->stopOnFailure()) { $this->pending = []; } elseif (($pending = array_shift($this->pending)) !== null) { $worker->assign($pending, $phpunit, $phpunitOptions, $this->options); + $this->batches[$token]++; } } @@ -125,4 +135,24 @@ private function waitForAllToFinish(): void usleep(self::CYCLE_SLEEP); } } + + private function startWorker(int $token): WrapperWorker + { + $this->workers[$token] = new WrapperWorker($this->output, $this->options, $token); + $this->workers[$token]->start(); + $this->batches[$token] = 0; + + return $this->workers[$token]; + } + + private function destroyWorker(int $token): void + { + // Mutation Testing tells us that the following `unset()` already destroys + // the `WrapperWorker`, which destroys the Symfony's `Process`, which + // automatically calls `Process::stop` within `Process::__destruct()`. + // But we prefer to have an explicit stops. + $this->workers[$token]->stop(); + + unset($this->workers[$token]); + } } diff --git a/test/Unit/Runners/PHPUnit/WrapperRunnerTest.php b/test/Unit/Runners/PHPUnit/WrapperRunnerTest.php index b48f4c1c..8c2316e5 100644 --- a/test/Unit/Runners/PHPUnit/WrapperRunnerTest.php +++ b/test/Unit/Runners/PHPUnit/WrapperRunnerTest.php @@ -7,6 +7,15 @@ use InvalidArgumentException; use ParaTest\Runners\PHPUnit\WrapperRunner; +use function array_diff; +use function array_unique; +use function file_get_contents; +use function min; +use function scandir; +use function unlink; + +use const FIXTURES; + /** * @internal * @@ -17,6 +26,8 @@ */ final class WrapperRunnerTest extends RunnerTestCase { + protected const NUMBER_OF_CLASS_TESTS_FOR_BATCH_SIZE = 4; + protected const UNPROCESSABLE_FILENAMES = ['..', '.', '.gitignore']; /** {@inheritdoc } */ protected $runnerClass = WrapperRunner::class; @@ -42,4 +53,58 @@ public function testWrapperRunnerWorksWellWithManyTests(): void $this->runRunner(); } + + /** @dataProvider provideForWrapperRunnerHandlesBatchSize */ + public function testWrapperRunnerHandlesBatchSize(int $processes, ?int $batchSize, int $expectedPidCount): void + { + $this->bareOptions['--path'] = $this->fixture('wrapper_batchsize_suite'); + $this->bareOptions['--configuration'] = $this->fixture('phpunit-wrapper-batchsize-suite.xml'); + $this->bareOptions['--processes'] = (string) $processes; + if ($batchSize !== null) { + $this->bareOptions['--max-batch-size'] = (string) $batchSize; + } + + $tmpDir = FIXTURES . DS . 'wrapper_batchsize_suite' . DS . 'tmp'; + $pidFilesDir = $tmpDir . DS . 'pid'; + $tokenFilesDir = $tmpDir . DS . 'token'; + + $this->cleanContentFromDir($pidFilesDir); + $this->cleanContentFromDir($tokenFilesDir); + + $this->runRunner(); + + self::assertCount($expectedPidCount, $this->extractContentFromDirFiles($pidFilesDir)); + self::assertCount(min([self::NUMBER_OF_CLASS_TESTS_FOR_BATCH_SIZE, $processes]), $this->extractContentFromDirFiles($tokenFilesDir)); + } + + /** @return iterable */ + public function provideForWrapperRunnerHandlesBatchSize(): iterable + { + yield 'One process with batchsize = null should have 1 pids and 1 token' => [1, null, 1]; + yield 'One process with batchsize = 0 should have 1 pids and 1 token' => [1, 0, 1]; + yield 'One process with batchsize = 1 should have 4 pids and 1 token' => [1, 1, 4]; + yield 'One process with batchsize = 2 should have 2 pids and 1 token' => [1, 2, 2]; + yield 'Two processes with batchsize = 2 should have 2 pids and 2 tokens' => [2, 2, 2]; + } + + private function cleanContentFromDir(string $path): void + { + $cleanableFiles = array_diff(scandir($path), self::UNPROCESSABLE_FILENAMES); + foreach ($cleanableFiles as $cleanableFile) { + unlink($path . DS . $cleanableFile); + } + } + + /** @return array */ + private function extractContentFromDirFiles(string $path): array + { + $res = []; + $processableFiles = array_diff(scandir($path), self::UNPROCESSABLE_FILENAMES); + self::assertCount(self::NUMBER_OF_CLASS_TESTS_FOR_BATCH_SIZE, $processableFiles); + foreach ($processableFiles as $processableFile) { + $res[] = file_get_contents($path . DS . $processableFile); + } + + return array_unique($res); + } } diff --git a/test/fixtures/phpunit-wrapper-batchsize-suite.xml b/test/fixtures/phpunit-wrapper-batchsize-suite.xml new file mode 100644 index 00000000..e86861a1 --- /dev/null +++ b/test/fixtures/phpunit-wrapper-batchsize-suite.xml @@ -0,0 +1,8 @@ + + + + + ./wrapper_batchsize_suite/ + + + diff --git a/test/fixtures/wrapper_batchsize_suite/FourTest.php b/test/fixtures/wrapper_batchsize_suite/FourTest.php new file mode 100644 index 00000000..b5552749 --- /dev/null +++ b/test/fixtures/wrapper_batchsize_suite/FourTest.php @@ -0,0 +1,9 @@ +getFileSuffix(); + $token = getenv('TEST_TOKEN'); + static::assertIsString($token); + file_put_contents($tokenFile, $token); + static::assertStringEqualsFile($tokenFile, $token); + } + + public function testPid(): void + { + $pidFile = self::TMP_DIR_PATH . DS . 'pid' . DS . $this->getFileSuffix(); + $pid = (string) getmypid(); + file_put_contents($pidFile, $pid); + + static::assertStringEqualsFile($pidFile, $pid); + } + + private function getFileSuffix(): string + { + $refClass = new ReflectionClass(static::class); + + return str_replace(['\\', '/'], '_', $refClass->getName()); + } +} diff --git a/test/fixtures/wrapper_batchsize_suite/tmp/pid/.gitignore b/test/fixtures/wrapper_batchsize_suite/tmp/pid/.gitignore new file mode 100644 index 00000000..e69de29b diff --git a/test/fixtures/wrapper_batchsize_suite/tmp/token/.gitignore b/test/fixtures/wrapper_batchsize_suite/tmp/token/.gitignore new file mode 100644 index 00000000..e69de29b