wpmedia-phpunit binary breaks @runInSeparateProcess tests

[remyperona]

🤖 AI-generated — created with Claude Code, reviewed by @Tabrisrp. Discovered while migrating wp-media/mcp-oauth to wp-media/phpunit v3.2.

Summary

The wpmedia-phpunit binary is incompatible with PHPUnit's @runInSeparateProcess isolation: all @runInSeparateProcess tests fail when run through the binary, while identical tests pass under a direct vendor/bin/phpunit invocation with the same arguments.

Steps to reproduce / Evidence

Reproduction in a consumer plugin (wp-media/mcp-oauth):

• A test class with 9 @runInSeparateProcess + @preserveGlobalState disabled data sets
  • Via composer test-unit (runs wpmedia-phpunit unit): 0/9 pass (Test was run in child process and ended unexpectedly)
  • Via direct vendor/bin/phpunit --testsuite unit --colors=always --configuration Tests/Unit/phpunit.xml.dist: 9/9 pass
  • Same effective arguments; identical test code

PHPUnit instrumentation evidence:

• Instrumenting vendor/phpunit/phpunit/src/Util/PHP/Template/TestCaseMethod.tpl with counters showed the isolated child dies inside the {included_files} block: the included-file count grows from 58 → 142 in the passing (direct) run, whereas the binary-run child never survives that block to reach the configured bootstrap require.

Root cause

The binary (wpmedia-phpunit at repo root) and src/BootstrapManager.php bootstrap PHPUnit by require-ing vendor/bin/phpunit into the currently running PHP process (see BootstrapManager::runTestSuite(), the require_once "{$root}/bin/phpunit" line).

The failure chain:

1. wpmedia-phpunit and src/BootstrapManager.php are added to get_included_files() when loaded into the parent process.
2. For @runInSeparateProcess tests, PHPUnit serializes the parent's included-files list into each child and re-requires them (PHPUnit\Util\GlobalState::getIncludedFilesAsString()).
3. The child re-requires wpmedia-phpunit.
4. Because that script begins with a #!/usr/bin/env php shebang and, when included, self-executes (it calls BootstrapManager::setupConstants() and BootstrapManager::runTestSuite() at the top level), re-including it in the child re-runs the whole bootstrap manager and emits the shebang line to the child's STDOUT.
5. That corrupts the serialized child result the parent reads back → Test was run in child process and ended unexpectedly.
6. Every isolated data set fails.

A direct vendor/bin/phpunit run passes because its included files are all inert library files — harmless to re-require.

Proposed fix

Two options; the second is recommended as the robust, long-term solution.

Option 1 — Minimal / targeted

In BootstrapManager, before running the suite, register the binary entrypoints into PHPUnit's isolation exclusion list so they are not re-required in isolated children:

$GLOBALS['__PHPUNIT_ISOLATION_EXCLUDE_LIST'][] = realpath( $wpmedia_phpunit_binary_path );
$GLOBALS['__PHPUNIT_ISOLATION_EXCLUDE_LIST'][] = realpath( __DIR__ . '/BootstrapManager.php' );

PHPUnit already honors this list in vendor/phpunit/phpunit/src/Util/GlobalState.php (getIncludedFilesAsString() skips listed files). Composer's bin-proxy already populates it for the real phpunit entry; the binary's own entrypoints simply need to be added.

• Pros: minimal change; reuses PHPUnit's existing mechanism.
• Cons: does not address the structural issue of loading PHPUnit into-process.

Option 2 — Structural / robust (recommended)

Run PHPUnit as a genuine subprocess (proc_open/passthru) instead of require-ing it into the runner process, so the binary and BootstrapManager never enter the test process's included-files set at all. This mirrors how native phpunit runs.

• Pros: eliminates the root cause; robust against future PHPUnit versions and other edge cases.
• Cons: larger refactor; requires careful subprocess lifecycle/error handling.

Impact

• @runInSeparateProcess is common in WordPress plugin tests (e.g. tests that define() constants such as WP_DEBUG / WP_DEBUG_LOG). Affected consumers cannot use the README-recommended wpmedia-phpunit binary and must fall back to direct phpunit.
• Dogfooding/README inconsistency worth noting: the README recommends "test-unit": "wpmedia-phpunit unit", but this library's own composer.json scripts invoke vendor/bin/phpunit --configuration ... directly — i.e. they sidestep the binary and therefore this bug.

Additional notes

Minor related papercut: BootstrapManager::getConfigArgv() hardcodes --colors=always, which emits raw ANSI escape codes when output is piped to a non-TTY (CI logs / file capture). Consider --colors=auto or making it overridable.

[remyperona]

Note

Generated by the AI delivery pipeline (grooming-agent · Claude Opus 4.8).

Grooming Plan — Issue #51

Approach: Implement Option 1 (register wpmedia-phpunit and BootstrapManager.php's realpaths in $GLOBALS['__PHPUNIT_ISOLATION_EXCLUDE_LIST']), not Option 2 (subprocess). Also fix the --colors=always papercut (→ --colors=auto).

Effort: S · Risk: LOW · Complexity: MEDIUM

Key decisions:

1. Option 1 chosen over Option 2. Grooming traced the actual bug through PHPUnit's vendored Util/GlobalState.php: PHPUnit already auto-skips the entry script (index 0) and, when index 1 matches the Composer bin-proxy pattern, that too — but this bin's own require __DIR__ . '/src/BootstrapManager.php' shifts the real wpmedia-phpunit file out of that special-cased position, so it never gets skipped and its shebang line corrupts the isolated child. Registering both files' realpaths in __PHPUNIT_ISOLATION_EXCLUDE_LIST (confirmed present, unchanged, from PHPUnit 9.6 through the current 13.3.1) directly closes that gap.
2. Option 2 (subprocess) has a hidden blocker the issue doesn't mention: WPMEDIA_PHPUNIT_ROOT_DIR / WPMEDIA_PHPUNIT_ROOT_TEST_DIR are define()-d by setupConstants() in the parent process and are only visible to PHPUnit today because it runs in the same process. A genuine proc_open subprocess would silently break every consumer relying on the package's bundled default config (src/Unit/phpunit.xml.dist → bootstrap.php directly, no intervening init-tests.php) unless the constants are bridged across the process boundary (env vars + updates to src/Unit/bootstrap.php / src/Integration/bootstrap.php). That raises Option 2 to a materially larger, higher-risk change — recommended as a separate follow-up issue, not folded into this fix.
3. Testability given the CLAUDE.md self-test caveat: since the package's own suite bypasses the bin, a new composer test-unit-bin-smoke script (php wpmedia-phpunit unit WPMEDIA_PHPUNIT_ROOT_DIR=. --group IsolationSmoke) is added and wired into both CI workflows — this is the one check that actually exercises the real bin end-to-end with an @runInSeparateProcess test, closing the verification gap the caveat describes.

Files touched: src/BootstrapManager.php (new registerIsolationExcludeList() + --colors=auto), composer.json (colors fix on existing scripts + new smoke script), Tests/Fixtures/BootstrapManager/getConfigArgv.php, new Tests/Unit/BootstrapManager/registerIsolationExcludeList.php, new Tests/Unit/IsolationSmoke/runsThroughTheBin.php, .github/workflows/tests.yml + tests_legacy.yml (new smoke step).

Test plan: composer test-unit-bin-smoke exits 0 (proves the real bin survives @runInSeparateProcess); composer test-unit stays green (no regression to existing argv-parsing tests); piping either command's output to a file shows no raw ANSI escapes.

No PR splitting plan required — effort is S/M, single cohesive change (the fix isn't meaningfully complete without the regression-proving smoke test, so it ships as one PR).

[remyperona]

Note

Generated by the AI delivery pipeline (grooming-agent · Claude Opus 4.8). Plan revision 2 — supersedes the previous grooming comment on this issue after challenger review found the originally-specified mechanism would have been a no-op.

Grooming Plan — Issue #51 (revised)

A challenger review of the first spec found that the proposed fix location and file set would not actually have worked. All three findings were reproduced empirically against a real Composer-installed consumer project (not just re-derived from source) before revising.

What was confirmed empirically:

1. Registering the isolation exclude list at the top of BootstrapManager::runTestSuite() is a no-op. vendor/bin/phpunit (Composer's own generated proxy) unconditionally overwrites (not appends to) $GLOBALS['__PHPUNIT_ISOLATION_EXCLUDE_LIST'] the moment it's required — which happens immediately after that registration call, wiping it before isolation ever reads it. Confirmed by a live before/after test: registering post-clobber (from src/Unit/bootstrap.php, which PHPUnit loads via its own bootstrap= config attribute — always after vendor/bin/phpunit's top-level code has run) is not clobbered, and the isolated test flips from FAIL to PASS.
2. The excluded set needed to be four files, not two: the real wpmedia-phpunit bin, BootstrapManager.php, and Composer's own vendor/bin/phpunit proxy all had to be added; the real PHPUnit binary (vendor/phpunit/phpunit/phpunit) turned out to already be excluded by Composer's own proxy (as long as we don't clobber it either) but is registered defensively anyway.
3. Important discrepancy found: the bug only reproduces with @runInSeparateProcess + @preserveGlobalState left at its default (enabled). With @preserveGlobalState disabled — the exact annotation this issue's own reproduction describes — PHPUnit never calls the code path that reads the exclude list at all ($includedFiles is hardcoded to '' when preserveGlobalState is disabled, per TestCase::run()), so that scenario was already passing before any fix and is unaffected by it either way. This may indicate a different PHPUnit/PHP version in the original mcp-oauth repro; flagged as an open question for the reporter, not a blocker.
4. The double-define() warning MUST_HAVE-3 raised (from self-targeting the package's own Tests/Unit/init-tests.php through the real bin) is real but, empirically, does not trip convertWarningsToExceptions or fail the run — it fires during bootstrap, before PHPUnit's test-execution error handler is installed. Still fixed via guarded define()s for cleanliness.

Revised approach: BootstrapManager gets a new findPhpunitRoot() (extracted from runTestSuite()) and registerIsolationExcludeList() (new, registers all 4 paths, always appends). The registration call moves to src/Unit/bootstrap.php and src/Integration/bootstrap.php — not runTestSuite(). Tests/Unit/init-tests.php's define()s are guarded. --colors=auto change unchanged from v1. A new composer test-unit-bin-smoke script + CI step runs a real @runInSeparateProcess (enabled) test through the actual bin — verified empirically to fail pre-fix and pass post-fix.

Effort: M (was S) · Risk: LOW-MEDIUM (was LOW) · Complexity: MEDIUM-HIGH

README + CHANGELOG updates are now mandatory (not optional) since --colors=auto is an observable CLI-output change for consumers. The integration-suite half of the fix (src/Integration/bootstrap.php) was not independently re-verified against a live WordPress test suite in this grooming pass and is called out as a pre-merge verification step for the implementer.

Full detail in the updated implementation spec (51-spec.md, plan v2).

[remyperona]

Note

Generated by the AI delivery pipeline (orchestrator · Claude Opus 4.8).

Pipeline outcome: stopped by maintainer; PR #54 closed unmerged. A full Option-A implementation was built and validated but cannot meet the (mandatory) PHP 7.4 requirement. Recording the findings so an Option-B follow-up can start from solid ground.

Decisive finding: the wpmedia-phpunit bin has never worked on PHP 7.4

BootstrapManager::runTestSuite() bootstraps PHPUnit with require_once "{root}/bin/phpunit". On PHP < 8, require does not strip a shebang line, so Composer's proxy (vendor/bin/phpunit, which is #!/usr/bin/env php on line 1, namespace Composer; on line 13) fatals:

PHP Fatal error: Namespace declaration statement has to be the very first statement … in vendor/bin/phpunit on line 13

This fires at startup for any bin invocation (not just isolation), on trunk as well. It has been invisible because CI and the composer.json scripts bypass the bin and call vendor/bin/phpunit directly (the dogfooding gap already noted in the issue). It surfaced here only because the proposed fix added a CI step that actually runs the bin.

Why Option A can't be the fix

Option A (append the entry points to $GLOBALS['__PHPUNIT_ISOLATION_EXCLUDE_LIST']) does fix @runInSeparateProcess on PHP 8.0–8.5 — this was empirically verified: neutralizing the registration reproduced "Test was run in child process and ended unexpectedly", restoring it went green through the real bin. But Option A relies on the in-process require that itself fatals on 7.4, so it structurally cannot deliver 7.4. No exclude-list tweak changes that.

Recommended path: Option B (subprocess execution)

Run PHPUnit as a genuine child process (php vendor/bin/phpunit … via passthru/proc_open). As a main script the shebang is stripped, and Composer's BinProxyWrapper handles the real binary — the exact mechanism that already passes on 7.4 for this repo's own composer scripts. Benefits:

• Delivers PHP 7.4 (the blocking requirement).
• Fixes the original bug at the root: the bin and BootstrapManager never enter the test process, so nothing is re-required into @runInSeparateProcess children — the exclude-list workaround becomes unnecessary.

Known complication (must be handled): WPMEDIA_PHPUNIT_ROOT_DIR / WPMEDIA_PHPUNIT_ROOT_TEST_DIR (and WPMEDIA_PHPUNIT_ADDON_ROOT_TEST_DIR) are define()-d in the parent and only reach the config today because PHPUnit runs in the same process. PHP constants don't cross a proc_open boundary — bridge them via environment variables, with src/Unit/bootstrap.php / src/Integration/bootstrap.php calling define() from getenv() when not already defined. This is a consumer-facing contract change (document it, likely a minor/major bump).

Other verified facts from this run (useful for Option B)

• --colors=always → --colors=auto is a real, worthwhile papercut fix (raw ANSI in piped/CI logs). Confirmed no ANSI leakage with auto. Carry it into Option B.
• @preserveGlobalState discrepancy: the repro here used @preserveGlobalState disabled, but in PHPUnit 9.6 that annotation hardcodes $includedFiles = '' and never reaches the included-files mechanism (TestCase.php:862-878) — so disabled passes even unpatched, and only the default (enabled) case reproduces the failure. Worth reconciling against the exact PHPUnit/PHP versions in the mcp-oauth repro. (Note: Option B removes this whole class of concern, since isolation no longer re-requires the bin at all.)
• Integration side: a live @preserveGlobalState enabled integration test currently trips an orthogonal Serialization of 'Closure' is not allowed from WP's own tests_add_filter('muplugins_loaded', <closure>) at src/Integration/bootstrap.php:33-43 — independent of this bug, but it will block fine-grained integration isolation testing until that closure is refactored to a named function.

The branch fix/51-wpmedia-phpunit-binary-breaks (Option A, PHP 8+ only) is retained for reference; PR #54 is closed.