Closes #51: exclude self-executing entry points from PHPUnit isolation

[remyperona]

🤖 AI-generated — created by an automated pipeline. Review before acting on this.

Closes #51

Summary

Root cause: The wpmedia-phpunit bin and BootstrapManager bootstrap PHPUnit by require_once-ing vendor/bin/phpunit into the currently running PHP process. For @runInSeparateProcess tests with @preserveGlobalState enabled (PHPUnit's default), PHPUnit's process-isolation machinery re-requires every file from get_included_files() into the child process. PHPUnit's heuristic auto-skips the entry script and Composer's phpunit proxy when they appear in a specific position, but because this package's bin inserts require __DIR__ . '/src/BootstrapManager.php' between itself and PHPUnit, the real wpmedia-phpunit file and BootstrapManager.php end up in a position not special-cased by the heuristic and get re-required into the child, corrupting the result.

Fix: Added BootstrapManager::registerIsolationExcludeList() method that appends the four self-executing entry-point realpaths (the bin, BootstrapManager, Composer's phpunit proxy, and the real PHPUnit binary) to $GLOBALS['__PHPUNIT_ISOLATION_EXCLUDE_LIST'] at the moment the bootstraps load (after Composer's own clobbering line in vendor/bin/phpunit has already run). The method is called from src/Unit/bootstrap.php and src/Integration/bootstrap.php. Additionally, --colors=always was replaced with --colors=auto in getConfigArgv() to prevent ANSI corruption in piped/CI output, and Tests/Unit/init-tests.php define() calls were guarded to prevent double-define warnings.

Acceptance Criteria

1. @runInSeparateProcess tests pass when run through the wpmedia-phpunit binary, matching a direct vendor/bin/phpunit run.
2. No "Test was run in child process and ended unexpectedly" corruption from the binary/BootstrapManager being re-required in isolated children.
3. --colors=always hardcoding replaced with --colors=auto so piped/CI output isn't corrupted with raw ANSI.

What was tested

TBD by QA — Placeholder for reviewer/QA to fill in after review. Expected baseline:

• composer test-unit: 126 tests OK
• composer test-unit-bin-smoke: 2 tests OK via the real bin, with documented unpatched-FAIL → patched-PASS on the isolation smoke test
• composer phpstan and composer phpcs: clean

Known Verification Gap / Reviewer Action

⚠ IMPORTANT — include this in your review decision:

The src/Integration/bootstrap.php registration was applied by symmetry with the proven unit-side path but could NOT be independently proven against a live WordPress integration suite locally: with @preserveGlobalState enabled (the annotation that exercises the bug), WP's own tests_add_filter() closures trip a pre-existing, orthogonal "Serialization of Closure is not allowed" error identically on patched and unpatched code, masking the target bug; with @preserveGlobalState disabled the case never reaches the affected PHPUnit code path. A human/CI reviewer should confirm the integration-side fix against any closure-free isolated integration test that exists downstream before merge.

Follow-up

Option B (run PHPUnit as a genuine proc_open/passthru subprocess) is deliberately out of scope — it has a real constant-propagation blocker (WPMEDIA_PHPUNIT_ROOT_DIR/ROOT_TEST_DIR don't cross a process boundary) and should be a separate issue.

[remyperona]

Note

Generated by the AI delivery pipeline (lead-reviewer · Claude Opus 4.8).

Review: ✅ PASS

Empirically verified: composer test-unit-bin-smoke fails with "Test was run in child process and ended unexpectedly" on the pre-fix code and passes after registerIsolationExcludeList() is wired into src/Unit/bootstrap.php; piping output to a file confirms no raw ANSI escapes after the --colors=auto change; composer phpstan is clean; composer phpcs is clean (BootstrapManager.php is intentionally outside the phpcs allow-list). No blockers.

Nice-to-haves:

• composer.json / README.md — test-unit-bin-smoke isn't wired into the run-tests composite script, the npm test:php* scripts, or documented in the "Running This Package's Tests" section, so a contributor following the documented wp-env workflow never runs the isolation smoke test locally (CI does catch it, but local repro requires knowing to run it manually).
• src/BootstrapManager.php:28,52 — new @since x.x.x placeholders are inconsistent with the rest of the file/history, which always uses a concrete version (e.g. 1.0.0); fill in the target version before release.
• Tests/Unit/BootstrapManager/findPhpunitRoot.php, registerIsolationExcludeList.php — the "root not found" / realpath() returns false branches aren't exercised (no mocking infra for core functions exists in this repo yet, so this is a real gap rather than an oversight — just flagging for awareness).
• Integration-side registration (src/Integration/bootstrap.php) could not be independently verified against a live WP integration suite: tests_add_filter()'s pre-existing muplugins_loaded closure (unrelated to this fix, and explicitly out of scope to change) trips "Serialization of Closure is not allowed" under @preserveGlobalState enabled, masking the target bug on any integration-side reproduction attempt. The fix is applied at the structurally identical point (same BootstrapManager/bin chain, same PHPUnit bootstrap= timing) as the proven unit-side fix, and the full integration suite runs in CI on every PR, so a coarse regression (e.g. a broken registerIsolationExcludeList() call) would still be caught — but there's no fine-grained regression test for the narrow isolation scenario on the integration side. Worth a tracked follow-up issue once there's a way to probe isolation in the integration suite without the closure blocker (e.g. after any future refactor of that hook to a named function).

[remyperona]

Note

Generated by the AI delivery pipeline (qa-engineer · Claude Opus 4.8).

QA: ⚠️ PARTIAL (unit/bin path fully PASSES; integration-side fix cannot be behaviorally proven locally — orthogonal blocker, confirmed empirically)

Environment: PHP 7.4 (this site's default) fails to boot wpmedia-phpunit at all — Fatal error: Namespace declaration statement has to be the very first statement ... vendor/bin/phpunit on line 13 — because PHP<8 doesn't strip a required file's shebang line (Composer's own proxy comment confirms this: "prevent the shebang from being output on PHP<8"). Verified this is pre-existing and unrelated to this PR by checking out trunk and reproducing the identical failure there. All validation below was therefore run on PHP 8.3 (matches CI matrix 8.0–8.5 and the wp-env default), where boot succeeds.

Acceptance Criterion	Method	Result	Evidence
AC1: @runInSeparateProcess tests pass through the bin, matching direct vendor/bin/phpunit	API (ran composer test-unit-bin-smoke)	✅	OK (2 tests, 3 assertions) — real bin, real isolation
AC2: no "Test was run in child process and ended unexpectedly" corruption	API (fix neutralized then restored)	✅	Commenting out BootstrapManager::registerIsolationExcludeList() in src/Unit/bootstrap.php reproduced the exact reported failure (F. → Test was run in child process and ended unexpectedly, Tests: 2, Failures: 1). Restoring the line brought it back to OK (2 tests, 3 assertions). composer test-unit self-test stayed green throughout: OK (126 tests, 332 assertions).
AC3: --colors=always → --colors=auto, no raw ANSI in piped output	API + Analysis	✅	Piped a full wpmedia-phpunit unit run to a file; byte-scan (\x1b[) found 0 occurrences. Test_GetConfigArgv (4 data sets, covered by composer test-unit) asserts --colors=auto in both the assertion and the Tests/Fixtures/BootstrapManager/getConfigArgv.php fixture.
Integration-side fix (src/Integration/bootstrap.php)	Analysis + wp-env (Docker)	⚠️ CANNOT_VERIFY (as claimed)	Confirmed the PR body's characterization is accurate: added a temporary @runInSeparateProcess probe test (default @preserveGlobalState enabled) under Tests/Integration/, ran it via wp-env — it dies with Serialization of 'Closure' is not allowed identically with the fix present and with registerIsolationExcludeList() commented out in src/Integration/bootstrap.php. This confirms the closure-serialization error (from WP's tests_add_filter() closures / core hooks) masks the target bug before the isolation-exclude list is ever consulted — it is a genuine, orthogonal local-verification blocker, not a cop-out. Existing integration suites (composer test-integration: 29/29, composer test-integration-admin: 3/3) stayed green with the fix in place, i.e. no regression, but neither covers the isolation-exclude path. Temporary probe file was deleted after use; working tree confirmed clean.

Blockers: none for merge on the unit/bin path — AC1–AC3 are solid. The integration-side fix remains an accepted, explicitly-flagged known limitation (see PR body) rather than a failed criterion.

Smoke tests:

• composer test-unit (126 tests) — PASS, no regressions.
• composer test-integration (29 tests) / test-integration-admin (3 tests) via wp-env — PASS, no regressions.
• Working tree left clean (git status clean) after all fix-neutralization/restoration and temp-file experiments.

Recommendations:

• (SHOULD_HAVE) Land a closure-free integration isolation smoke test once a workaround for @preserveGlobalState enabled + tests_add_filter() closures is found (e.g. a static function reference instead of a closure in a purpose-built fixture), so the integration-side fix gets the same empirical proof the unit side has.
• (COULD_HAVE) Document the PHP<8 "require-ing a file with its own shebang corrupts output/namespace parsing" constraint in the README/CLAUDE.md, since it silently breaks wpmedia-phpunit on PHP 7.4 regardless of this fix (pre-existing, but worth flagging given the legacy CI matrix still tests PHP 7.4).

[remyperona]

Note

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

Closing this PR without merging, at the maintainer's direction.

Why: This PR implements Option A (append the self-executing entry points to PHPUnit's __PHPUNIT_ISOLATION_EXCLUDE_LIST). It was verified to fix @runInSeparateProcess through the wpmedia-phpunit bin on PHP 8.0–8.5 (fail→pass reproduced locally, --colors=auto confirmed, phpstan/phpcs clean, lead review PASS). However, it cannot satisfy the mandatory PHP 7.4 requirement.

The wpmedia-phpunit bin fatal-errors at startup on PHP < 8 for any invocation: BootstrapManager::runTestSuite() does require_once "{root}/bin/phpunit", and PHP < 8 does not strip the shebang from a required file, so Composer's proxy namespace Composer; is no longer the first statement (PHP Fatal error: Namespace declaration statement has to be the very first statement …). This is pre-existing (identical on trunk) and was masked because CI and the composer scripts bypass the bin and call vendor/bin/phpunit directly. Option A's in-process require is itself the thing that fatals on 7.4, so no exclude-list change can fix it.

Path forward — Option B (subprocess): run PHPUnit as a genuine child process (php vendor/bin/phpunit …), where the shebang is stripped (main script) and Composer's BinProxyWrapper handles the real binary — the same mechanism that already passes on 7.4 for this repo's own composer scripts. Option B also fixes the original isolation bug at the root (the bin/BootstrapManager never enter the test process), making the exclude-list unnecessary. Its one real complication is bridging WPMEDIA_PHPUNIT_ROOT_DIR / WPMEDIA_PHPUNIT_ROOT_TEST_DIR (+ addon) across the process boundary via env vars in src/{Unit,Integration}/bootstrap.php.

The branch fix/51-wpmedia-phpunit-binary-breaks is left intact for reference. See issue #51 for the full write-up.