Skip to content

Commit

Permalink
Inline SKIPIF evaluation without side-effects
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm committed Oct 18, 2024
1 parent 6321d41 commit a39878b
Show file tree
Hide file tree
Showing 9 changed files with 213 additions and 15 deletions.
7 changes: 7 additions & 0 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@
</fileset>
</copy>

<copy file="${basedir}/vendor/staabm/side-effects-detector/LICENSE" tofile="${basedir}/build/tmp/phar/staabm-side-effects-detector/LICENSE"/>
<copy todir="${basedir}/build/tmp/phar/staabm-side-effects-detector">
<fileset dir="${basedir}/vendor/staabm/side-effects-detector/lib">
<include name="**/*.php" />
</fileset>
</copy>

<copy file="${basedir}/vendor/myclabs/deep-copy/LICENSE" tofile="${basedir}/build/tmp/phar/myclabs-deep-copy/LICENSE"/>
<copy todir="${basedir}/build/tmp/phar/myclabs-deep-copy">
<fileset dir="${basedir}/vendor/myclabs/deep-copy/src">
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
"sebastian/global-state": "^7.0.2",
"sebastian/object-enumerator": "^6.0.1",
"sebastian/type": "^5.1.0",
"sebastian/version": "^5.0.2"
"sebastian/version": "^5.0.2",
"staabm/side-effects-detector": "^1.0"
},
"config": {
"platform": {
Expand Down
57 changes: 54 additions & 3 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 62 additions & 9 deletions src/Runner/PHPT/PhptTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
use function is_readable;
use function is_string;
use function ltrim;
use function ob_get_clean;
use function ob_start;
use function preg_match;
use function preg_replace;
use function preg_split;
Expand Down Expand Up @@ -67,6 +69,8 @@
use SebastianBergmann\CodeCoverage\TestIdMissingException;
use SebastianBergmann\CodeCoverage\UnintentionallyCoveredCodeException;
use SebastianBergmann\Template\Template;
use staabm\SideEffectsDetector\SideEffect;
use staabm\SideEffectsDetector\SideEffectsDetector;
use Throwable;

/**
Expand Down Expand Up @@ -426,19 +430,26 @@ private function shouldTestBeSkipped(array $sections, array $settings): bool
return false;
}

$jobResult = JobRunnerRegistry::run(
new Job(
$this->render($sections['SKIPIF']),
$this->stringifyIni($settings),
),
);
$skipIfCode = $this->render($sections['SKIPIF']);

Facade::emitter()->testRunnerFinishedChildProcess($jobResult->stdout(), $jobResult->stderr());
if ($this->shouldRunSkipIfInSubprocess($sections, $skipIfCode)) {
$jobResult = JobRunnerRegistry::run(
new Job(
$skipIfCode,
$this->stringifyIni($settings),
),
);
$output = $jobResult->stdout();

Facade::emitter()->testRunnerFinishedChildProcess($output, $jobResult->stderr());
} else {
$output = $this->runCodeInLocalSandbox($skipIfCode);
}

if (!strncasecmp('skip', ltrim($jobResult->stdout()), 4)) {
if (!strncasecmp('skip', ltrim($output), 4)) {
$message = '';

if (preg_match('/^\s*skip\s*(.+)\s*/i', $jobResult->stdout(), $skipMatch)) {
if (preg_match('/^\s*skip\s*(.+)\s*/i', $output, $skipMatch)) {
$message = substr($skipMatch[1], 2);
}

Expand All @@ -455,6 +466,48 @@ private function shouldTestBeSkipped(array $sections, array $settings): bool
return false;
}

/**
* @param array<non-empty-string, non-empty-string> $sections
*/
private function shouldRunSkipIfInSubprocess(array $sections, string $skipIfCode): bool
{
if (isset($sections['INI'])) {
// to get per-test INI settings, we need a dedicated subprocess
return true;
}

$detector = new SideEffectsDetector;
$sideEffects = $detector->getSideEffects($skipIfCode);

if ($sideEffects === []) {
return false; // no side-effects

Check warning on line 483 in src/Runner/PHPT/PhptTestCase.php

View check run for this annotation

Codecov / codecov/patch

src/Runner/PHPT/PhptTestCase.php#L483

Added line #L483 was not covered by tests
}

foreach ($sideEffects as $sideEffect) {
if ($sideEffect === SideEffect::STANDARD_OUTPUT) {
// stdout is fine, we will catch it using output-buffering
continue;
}

return true;
}

return false;
}

private function runCodeInLocalSandbox(string $code): string
{
$code = preg_replace('/^<\?(php)?/', '', $code);
$code = preg_replace('/declare\S?\([^)]+\)\S?;/', '', $code);

// wrap in immediately invoked function to isolate local-side-effects of $code from our own process
$code = '(function() {' . $code . '})();';
ob_start();
@eval($code);

return ob_get_clean();
}

/**
* @param array<non-empty-string, non-empty-string> $sections
*/
Expand Down
14 changes: 14 additions & 0 deletions tests/end-to-end/_files/phpt-ini-subprocess.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
PHPT uses a subprocess when --INI-- is present, even if --SKIPIF-- has no side-effect
--INI--
error_reporting=-1
--SKIPIF--
<?php declare(strict_types=1);
if (1 == 2) {
echo "skip this test\n";
}
--FILE--
<?php declare(strict_types=1);
echo "Hello, World!\n";
--EXPECT--
Hello, World!
9 changes: 9 additions & 0 deletions tests/end-to-end/_files/phpt-skipif-exit-subprocess.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
--TEST--
PHPT skip condition which exits the subprocess (side-effect)
--FILE--
<?php declare(strict_types=1);
--SKIPIF--
<?php declare(strict_types=1);
echo "skip this test\n";
exit(1);
--EXPECT--
33 changes: 33 additions & 0 deletions tests/end-to-end/event/phpt-ini-subprocess.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
--TEST--
The right events are emitted in the right order for a PHPT test using a subprocess via --INI--
--FILE--
<?php declare(strict_types=1);
$_SERVER['argv'][] = '--do-not-cache-result';
$_SERVER['argv'][] = '--no-configuration';
$_SERVER['argv'][] = '--debug';
$_SERVER['argv'][] = __DIR__ . '/../_files/phpt-ini-subprocess.phpt';

require __DIR__ . '/../../bootstrap.php';

(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
--EXPECTF--
PHPUnit Started (PHPUnit %s using %s)
Test Runner Configured
Event Facade Sealed
Test Suite Loaded (1 test)
Test Runner Started
Test Suite Sorted
Test Runner Execution Started (1 test)
Test Suite Started (%s%ephpt-ini-subprocess.phpt, 1 test)
Test Preparation Started (%s%ephpt-ini-subprocess.phpt)
Test Prepared (%s%ephpt-ini-subprocess.phpt)
Child Process Started
Child Process Finished
Child Process Started
Child Process Finished
Test Passed (%s%ephpt-ini-subprocess.phpt)
Test Finished (%s%ephpt-ini-subprocess.phpt)
Test Suite Finished (%s%ephpt-ini-subprocess.phpt, 1 test)
Test Runner Execution Finished
Test Runner Finished
PHPUnit Finished (Shell Exit Code: 0)
32 changes: 32 additions & 0 deletions tests/end-to-end/event/phpt-skipif-subprocess.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
The right events are emitted in the right order for a skipped PHPT test using a skipif subprocess
--FILE--
<?php declare(strict_types=1);
$_SERVER['argv'][] = '--do-not-cache-result';
$_SERVER['argv'][] = '--no-configuration';
$_SERVER['argv'][] = '--debug';
$_SERVER['argv'][] = __DIR__ . '/../_files/phpt-skipif-exit-subprocess.phpt';

require __DIR__ . '/../../bootstrap.php';

(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
--EXPECTF--
PHPUnit Started (PHPUnit %s using %s)
Test Runner Configured
Event Facade Sealed
Test Suite Loaded (1 test)
Test Runner Started
Test Suite Sorted
Test Runner Execution Started (1 test)
Test Suite Started (%s%ephpt-skipif-exit-subprocess.phpt, 1 test)
Test Preparation Started (%s%ephpt-skipif-exit-subprocess.phpt)
Test Prepared (%s%ephpt-skipif-exit-subprocess.phpt)
Child Process Started
Child Process Finished
Test Skipped (%s%ephpt-skipif-exit-subprocess.phpt)
is test
Test Finished (%s%ephpt-skipif-exit-subprocess.phpt)
Test Suite Finished (%s%ephpt-skipif-exit-subprocess.phpt, 1 test)
Test Runner Execution Finished
Test Runner Finished
PHPUnit Finished (Shell Exit Code: 0)
2 changes: 0 additions & 2 deletions tests/end-to-end/event/phpt-skipif.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ Test Runner Execution Started (1 test)
Test Suite Started (%s%ephpt-skipif-location-hint-example.phpt, 1 test)
Test Preparation Started (%s%ephpt-skipif-location-hint-example.phpt)
Test Prepared (%s%ephpt-skipif-location-hint-example.phpt)
Child Process Started
Child Process Finished
Test Skipped (%s%ephpt-skipif-location-hint-example.phpt)
something terrible happened
Test Finished (%s%ephpt-skipif-location-hint-example.phpt)
Expand Down

0 comments on commit a39878b

Please sign in to comment.