WrapperRunner: add --max-batch-size support (#714)

Co-authored-by: CheapHasz <[email protected]>
Co-authored-by: Filippo Tessarotto <[email protected]>

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.

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": {

infection.json

@@ -4,7 +4,7 @@
             "src"
         ]
     },
-    "timeout": 10,
+    "timeout": 30,
     "logs": {
         "stryker": {
             "report": "/^\\d+\\.x$/"

src/Runners/PHPUnit/WrapperRunner.php

@@ -17,9 +17,12 @@
 /** @internal */
 final class WrapperRunner extends BaseRunner
 {
-    /** @var WrapperWorker[] */
+    /** @var array<int,WrapperWorker> */
     private $workers = [];
 
+    /** @var array<int,int> */
+    private $batches = [];
+
     protected function beforeLoadChecks(): void
     {
         if ($this->options->functional()) {
@@ -40,18 +43,18 @@ 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);
         }
     }
 
     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]);
+    }
 }

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<array{int,?int,int}> */
+    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<string> */
+    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);
+    }
 }

test/fixtures/phpunit-wrapper-batchsize-suite.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit bootstrap="../bootstrap.php" backupGlobals="true">
+    <testsuites>
+        <testsuite name="ParaTest Fixtures">
+            <directory>./wrapper_batchsize_suite/</directory>
+        </testsuite>
+    </testsuites>
+</phpunit>

test/fixtures/wrapper_batchsize_suite/FourTest.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ParaTest\Tests\fixtures\wrapper_batchsize_suite;
+
+class FourTest extends WrapperBatchTestCase
+{
+}

test/fixtures/wrapper_batchsize_suite/OneTest.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ParaTest\Tests\fixtures\wrapper_batchsize_suite;
+
+class OneTest extends WrapperBatchTestCase
+{
+}

test/fixtures/wrapper_batchsize_suite/ThreeTest.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ParaTest\Tests\fixtures\wrapper_batchsize_suite;
+
+class ThreeTest extends WrapperBatchTestCase
+{
+}

test/fixtures/wrapper_batchsize_suite/TwoTest.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ParaTest\Tests\fixtures\wrapper_batchsize_suite;
+
+class TwoTest extends WrapperBatchTestCase
+{
+}

test/fixtures/wrapper_batchsize_suite/WrapperBatchTestCase.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ParaTest\Tests\fixtures\wrapper_batchsize_suite;
+
+use PHPUnit\Framework\TestCase;
+use ReflectionClass;
+
+use function file_put_contents;
+use function getenv;
+use function getmypid;
+use function str_replace;
+
+abstract class WrapperBatchTestCase extends TestCase
+{
+    private const TMP_DIR_PATH = __DIR__ . DS . 'tmp';
+
+    public function testToken(): void
+    {
+        $tokenFile = self::TMP_DIR_PATH . DS . 'token' . DS . $this->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());
+    }
+}