Do not run SKIPIF section of PHPT test in separate process when it is free of side effects

build.xml

@@ -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">

composer.json

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

src/Runner/PHPT/PhptTestCase.php

@@ -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;
@@ -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;
 
 /**
@@ -426,19 +430,26 @@
             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);
             }
 
@@ -455,6 +466,48 @@
         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
+        }
+
+        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
      */

tests/end-to-end/_files/phpt-ini-subprocess.phpt

@@ -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!

tests/end-to-end/_files/phpt-skipif-exit-subprocess.phpt

@@ -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--

tests/end-to-end/event/phpt-ini-subprocess.phpt

@@ -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)

tests/end-to-end/event/phpt-skipif-subprocess.phpt

@@ -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)

tests/end-to-end/event/phpt-skipif.phpt

@@ -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)