Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC: lightweight subprocess isolation via pcntl_fork() #5751

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ jobs:
- "8.3"
- "8.4"

include:
- os: ubuntu-latest
php-version: "8.3"
add-ext: ", pcntl"

steps:
- name: Configure Git to avoid issues with line endings
if: matrix.os == 'windows-latest'
Expand All @@ -131,7 +136,7 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php-version }}
extensions: ${{ env.PHP_EXTENSIONS }}
extensions: "${{ env.PHP_EXTENSIONS }}${{ matrix.add-ext }}"
ini-values: ${{ env.PHP_INI_VALUES }}
tools: none

Expand Down Expand Up @@ -166,6 +171,11 @@ jobs:
- "8.3"
- "8.4"

include:
- os: ubuntu-latest
php-version: "8.3"
add-ext: ", pcntl"

steps:
- name: Configure Git to avoid issues with line endings
if: matrix.os == 'windows-latest'
Expand All @@ -178,7 +188,7 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php-version }}
extensions: ${{ env.PHP_EXTENSIONS }}
extensions: "${{ env.PHP_EXTENSIONS }}${{ matrix.add-ext }}"
ini-values: ${{ env.PHP_INI_VALUES }}
coverage: pcov
tools: none
Expand Down
11 changes: 11 additions & 0 deletions src/Framework/Attributes/RunClassInSeparateProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,15 @@
#[Attribute(Attribute::TARGET_CLASS)]
final readonly class RunClassInSeparateProcess
{
private ?bool $forkIfPossible;

public function __construct(?bool $forkIfPossible = null)
{
$this->forkIfPossible = $forkIfPossible;
}

public function forkIfPossible(): ?bool
{
return $this->forkIfPossible;
}
}
11 changes: 11 additions & 0 deletions src/Framework/Attributes/RunInSeparateProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,15 @@
#[Attribute(Attribute::TARGET_METHOD)]
final readonly class RunInSeparateProcess
{
private ?bool $forkIfPossible;

public function __construct(?bool $forkIfPossible = null)
{
$this->forkIfPossible = $forkIfPossible;
}

public function forkIfPossible(): ?bool
{
return $this->forkIfPossible;
}
}
11 changes: 11 additions & 0 deletions src/Framework/Attributes/RunTestsInSeparateProcesses.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,15 @@
#[Attribute(Attribute::TARGET_CLASS)]
final readonly class RunTestsInSeparateProcesses
{
private ?bool $forkIfPossible;

public function __construct(?bool $forkIfPossible = null)
{
$this->forkIfPossible = $forkIfPossible;
}

public function forkIfPossible(): ?bool
{
return $this->forkIfPossible;
}
}
63 changes: 61 additions & 2 deletions src/Framework/TestBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
use PHPUnit\Metadata\ExcludeStaticPropertyFromBackup;
use PHPUnit\Metadata\Parser\Registry as MetadataRegistry;
use PHPUnit\Metadata\PreserveGlobalState;
use PHPUnit\Metadata\RunClassInSeparateProcess;
use PHPUnit\Metadata\RunInSeparateProcess;
use PHPUnit\Metadata\RunTestsInSeparateProcesses;
use PHPUnit\TextUI\Configuration\Registry as ConfigurationRegistry;
use ReflectionClass;

Expand Down Expand Up @@ -51,6 +54,7 @@
$this->shouldTestMethodBeRunInSeparateProcess($className, $methodName),
$this->shouldGlobalStateBePreserved($className, $methodName),
$this->shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess($className),
$this->shouldForkIfPossible($className, $methodName),
$this->backupSettings($className, $methodName),
$groups,
);
Expand All @@ -59,11 +63,12 @@
/** @psalm-suppress UnsafeInstantiation */
$test = new $className($methodName);

$this->configureTestCase(

Check warning on line 66 in src/Framework/TestBuilder.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "MethodCallRemoval": --- Original +++ New @@ @@ } /** @psalm-suppress UnsafeInstantiation */ $test = new $className($methodName); - $this->configureTestCase($test, $this->shouldTestMethodBeRunInSeparateProcess($className, $methodName), $this->shouldGlobalStateBePreserved($className, $methodName), $this->shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess($className), $this->shouldForkIfPossible($className, $methodName), $this->backupSettings($className, $methodName)); + return $test; } /**
$test,
$this->shouldTestMethodBeRunInSeparateProcess($className, $methodName),
$this->shouldGlobalStateBePreserved($className, $methodName),
$this->shouldAllTestMethodsOfTestClassBeRunInSingleSeparateProcess($className),
$this->shouldForkIfPossible($className, $methodName),
$this->backupSettings($className, $methodName),
);

Expand All @@ -76,7 +81,7 @@
* @psalm-param array{backupGlobals: ?bool, backupGlobalsExcludeList: list<string>, backupStaticProperties: ?bool, backupStaticPropertiesExcludeList: array<string,list<string>>} $backupSettings
* @psalm-param list<non-empty-string> $groups
*/
private function buildDataProviderTestSuite(string $methodName, string $className, array $data, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, array $backupSettings, array $groups): DataProviderTestSuite
private function buildDataProviderTestSuite(string $methodName, string $className, array $data, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, bool $forkIfPossible, array $backupSettings, array $groups): DataProviderTestSuite
{
$dataProviderTestSuite = DataProviderTestSuite::empty(
$className . '::' . $methodName,
Expand All @@ -93,11 +98,12 @@

$_test->setData($_dataName, $_data);

$this->configureTestCase(

Check warning on line 101 in src/Framework/TestBuilder.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "MethodCallRemoval": --- Original +++ New @@ @@ /** @psalm-suppress UnsafeInstantiation */ $_test = new $className($methodName); $_test->setData($_dataName, $_data); - $this->configureTestCase($_test, $runTestInSeparateProcess, $preserveGlobalState, $runClassInSeparateProcess, $forkIfPossible, $backupSettings); + $dataProviderTestSuite->addTest($_test, $groups); } return $dataProviderTestSuite;
$_test,
$runTestInSeparateProcess,
$preserveGlobalState,
$runClassInSeparateProcess,
$forkIfPossible,
$backupSettings,
);

Expand All @@ -110,7 +116,7 @@
/**
* @psalm-param array{backupGlobals: ?bool, backupGlobalsExcludeList: list<string>, backupStaticProperties: ?bool, backupStaticPropertiesExcludeList: array<string,list<string>>} $backupSettings
*/
private function configureTestCase(TestCase $test, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, array $backupSettings): void
private function configureTestCase(TestCase $test, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, bool $forkIfPossible, array $backupSettings): void
{
if ($runTestInSeparateProcess) {
$test->setRunTestInSeparateProcess(true);
Expand All @@ -120,6 +126,10 @@
$test->setRunClassInSeparateProcess(true);
}

if ($forkIfPossible) {

Check warning on line 129 in src/Framework/TestBuilder.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "IfNegation": --- Original +++ New @@ @@ if ($runClassInSeparateProcess) { $test->setRunClassInSeparateProcess(true); } - if ($forkIfPossible) { + if (!$forkIfPossible) { $test->setForkIfPossible(true); } if ($preserveGlobalState !== null) {
$test->setForkIfPossible(true);

Check warning on line 130 in src/Framework/TestBuilder.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "TrueValue": --- Original +++ New @@ @@ $test->setRunClassInSeparateProcess(true); } if ($forkIfPossible) { - $test->setForkIfPossible(true); + $test->setForkIfPossible(false); } if ($preserveGlobalState !== null) { $test->setPreserveGlobalState($preserveGlobalState);

Check warning on line 130 in src/Framework/TestBuilder.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "MethodCallRemoval": --- Original +++ New @@ @@ $test->setRunClassInSeparateProcess(true); } if ($forkIfPossible) { - $test->setForkIfPossible(true); + } if ($preserveGlobalState !== null) { $test->setPreserveGlobalState($preserveGlobalState);
}

if ($preserveGlobalState !== null) {
$test->setPreserveGlobalState($preserveGlobalState);
}
Expand Down Expand Up @@ -272,4 +282,53 @@
{
return MetadataRegistry::parser()->forClass($className)->isRunClassInSeparateProcess()->isNotEmpty();
}

/**
* @psalm-param class-string $className
* @psalm-param non-empty-string $methodName
*/
private function shouldForkIfPossible(string $className, string $methodName): bool
{
$metadataForMethod = MetadataRegistry::parser()->forMethod($className, $methodName);

if ($metadataForMethod->isRunInSeparateProcess()->isNotEmpty()) {
$metadata = $metadataForMethod->isRunInSeparateProcess()->asArray()[0];

assert($metadata instanceof RunInSeparateProcess);

$forkIfPossible = $metadata->forkIfPossible();

if ($forkIfPossible !== null) {

Check warning on line 301 in src/Framework/TestBuilder.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "NotIdentical": --- Original +++ New @@ @@ $metadata = $metadataForMethod->isRunInSeparateProcess()->asArray()[0]; assert($metadata instanceof RunInSeparateProcess); $forkIfPossible = $metadata->forkIfPossible(); - if ($forkIfPossible !== null) { + if ($forkIfPossible === null) { return $forkIfPossible; } }
return $forkIfPossible;
}
}

$metadataForClass = MetadataRegistry::parser()->forClass($className);

if ($metadataForClass->isRunTestsInSeparateProcesses()->isNotEmpty()) {
$metadata = $metadataForClass->isRunTestsInSeparateProcesses()->asArray()[0];

assert($metadata instanceof RunTestsInSeparateProcesses);

$forkIfPossible = $metadata->forkIfPossible();

if ($forkIfPossible !== null) {
return $forkIfPossible;

Check warning on line 316 in src/Framework/TestBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/Framework/TestBuilder.php#L316

Added line #L316 was not covered by tests
}
}

if ($metadataForClass->isRunClassInSeparateProcess()->isNotEmpty()) {
$metadata = $metadataForClass->isRunClassInSeparateProcess()->asArray()[0];

assert($metadata instanceof RunClassInSeparateProcess);

$forkIfPossible = $metadata->forkIfPossible();

if ($forkIfPossible !== null) {
return $forkIfPossible;
}
}

return false;

Check warning on line 332 in src/Framework/TestBuilder.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "FalseValue": --- Original +++ New @@ @@ return $forkIfPossible; } } - return false; + return true; } }
}
}
10 changes: 10 additions & 0 deletions src/Framework/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ abstract class TestCase extends Assert implements Reorderable, SelfDescribing, T
*/
private ?array $backupGlobalExceptionHandlers = null;
private ?bool $runClassInSeparateProcess = null;
private ?bool $forkIfPossible = null;
private ?bool $runTestInSeparateProcess = null;
private bool $preserveGlobalState = false;
private bool $inIsolation = false;
Expand Down Expand Up @@ -340,6 +341,7 @@ final public function run(): void
$this,
$this->runClassInSeparateProcess && !$this->runTestInSeparateProcess,
$this->preserveGlobalState,
$this->forkIfPossible === true,
);
}
}
Expand Down Expand Up @@ -709,6 +711,14 @@ final public function setRunClassInSeparateProcess(bool $runClassInSeparateProce
$this->runClassInSeparateProcess = $runClassInSeparateProcess;
}

/**
* @internal This method is not covered by the backward compatibility promise for PHPUnit
*/
final public function setForkIfPossible(bool $forkIfPossible): void
{
$this->forkIfPossible = $forkIfPossible;
}

/**
* @internal This method is not covered by the backward compatibility promise for PHPUnit
*/
Expand Down
12 changes: 11 additions & 1 deletion src/Framework/TestRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use PHPUnit\TextUI\Configuration\Registry as ConfigurationRegistry;
use PHPUnit\Util\GlobalState;
use PHPUnit\Util\PHP\AbstractPhpProcess;
use PHPUnit\Util\PHP\PcntlFork;
use ReflectionClass;
use SebastianBergmann\CodeCoverage\Exception as OriginalCodeCoverageException;
use SebastianBergmann\CodeCoverage\InvalidArgumentException;
Expand Down Expand Up @@ -248,8 +249,17 @@
* @throws ProcessIsolationException
* @throws StaticAnalysisCacheNotConfiguredException
*/
public function runInSeparateProcess(TestCase $test, bool $runEntireClass, bool $preserveGlobalState): void
public function runInSeparateProcess(TestCase $test, bool $runEntireClass, bool $preserveGlobalState, bool $forkIfPossible): void
{
if ($forkIfPossible && PcntlFork::isPcntlForkAvailable()) {
// forking the parent process is a more lightweight way to run a test in isolation.
// it requires the pcntl extension though.
$fork = new PcntlFork;
$fork->runTest($test);

Check warning on line 258 in src/Framework/TestRunner.php

View check run for this annotation

Codecov / codecov/patch

src/Framework/TestRunner.php#L257-L258

Added lines #L257 - L258 were not covered by tests

return;

Check warning on line 260 in src/Framework/TestRunner.php

View check run for this annotation

Codecov / codecov/patch

src/Framework/TestRunner.php#L260

Added line #L260 was not covered by tests
}

$class = new ReflectionClass($test);

if ($runEntireClass) {
Expand Down
12 changes: 6 additions & 6 deletions src/Metadata/Metadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -390,19 +390,19 @@ public static function requiresSettingOnMethod(string $setting, string $value):
return new RequiresSetting(self::METHOD_LEVEL, $setting, $value);
}

public static function runClassInSeparateProcess(): RunClassInSeparateProcess
public static function runClassInSeparateProcess(?bool $forkIfPossible = null): RunClassInSeparateProcess
{
return new RunClassInSeparateProcess(self::CLASS_LEVEL);
return new RunClassInSeparateProcess(self::CLASS_LEVEL, $forkIfPossible);
}

public static function runTestsInSeparateProcesses(): RunTestsInSeparateProcesses
public static function runTestsInSeparateProcesses(?bool $forkIfPossible = null): RunTestsInSeparateProcesses
{
return new RunTestsInSeparateProcesses(self::CLASS_LEVEL);
return new RunTestsInSeparateProcesses(self::CLASS_LEVEL, $forkIfPossible);
}

public static function runInSeparateProcess(): RunInSeparateProcess
public static function runInSeparateProcess(?bool $forkIfPossible = null): RunInSeparateProcess
{
return new RunInSeparateProcess(self::METHOD_LEVEL);
return new RunInSeparateProcess(self::METHOD_LEVEL, $forkIfPossible);
}

public static function test(): Test
Expand Down
18 changes: 15 additions & 3 deletions src/Metadata/Parser/AttributeParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,20 @@ public function forClass(string $className): MetadataCollection
break;

case RunClassInSeparateProcess::class:
$result[] = Metadata::runClassInSeparateProcess();
assert($attributeInstance instanceof RunClassInSeparateProcess);

$result[] = Metadata::runClassInSeparateProcess(
$attributeInstance->forkIfPossible(),
);

break;

case RunTestsInSeparateProcesses::class:
$result[] = Metadata::runTestsInSeparateProcesses();
assert($attributeInstance instanceof RunTestsInSeparateProcesses);

$result[] = Metadata::runTestsInSeparateProcesses(
$attributeInstance->forkIfPossible(),
);

break;

Expand Down Expand Up @@ -638,7 +646,11 @@ public function forMethod(string $className, string $methodName): MetadataCollec
break;

case RunInSeparateProcess::class:
$result[] = Metadata::runInSeparateProcess();
assert($attributeInstance instanceof RunInSeparateProcess);

$result[] = Metadata::runInSeparateProcess(
$attributeInstance->forkIfPossible(),
);

break;

Expand Down
17 changes: 17 additions & 0 deletions src/Metadata/RunClassInSeparateProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,23 @@
*/
final readonly class RunClassInSeparateProcess extends Metadata
{
private ?bool $forkIfPossible;

/**
* @psalm-param 0|1 $level
*/
protected function __construct(int $level, ?bool $forkIfPossible = null)
{
$this->forkIfPossible = $forkIfPossible;

parent::__construct($level);
}

public function forkIfPossible(): ?bool
{
return $this->forkIfPossible;
}

/**
* @psalm-assert-if-true RunClassInSeparateProcess $this
*/
Expand Down
17 changes: 17 additions & 0 deletions src/Metadata/RunInSeparateProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,23 @@
*/
final readonly class RunInSeparateProcess extends Metadata
{
private ?bool $forkIfPossible;

/**
* @psalm-param 0|1 $level
*/
protected function __construct(int $level, ?bool $forkIfPossible = null)
{
$this->forkIfPossible = $forkIfPossible;

parent::__construct($level);
}

public function forkIfPossible(): ?bool
{
return $this->forkIfPossible;
}

/**
* @psalm-assert-if-true RunInSeparateProcess $this
*/
Expand Down
17 changes: 17 additions & 0 deletions src/Metadata/RunTestsInSeparateProcesses.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,23 @@
*/
final readonly class RunTestsInSeparateProcesses extends Metadata
{
private ?bool $forkIfPossible;

/**
* @psalm-param 0|1 $level
*/
protected function __construct(int $level, ?bool $forkIfPossible = null)
{
$this->forkIfPossible = $forkIfPossible;

parent::__construct($level);
}

public function forkIfPossible(): ?bool
{
return $this->forkIfPossible;
}

/**
* @psalm-assert-if-true RunTestsInSeparateProcesses $this
*/
Expand Down
Loading
Loading