Skip to content

test: make compile-fixture verify files explicit typed closures#26

Merged
math3usmartins merged 1 commit into
0.3.xfrom
refactor/verify-closure-fixture
Jul 24, 2026
Merged

test: make compile-fixture verify files explicit typed closures#26
math3usmartins merged 1 commit into
0.3.xfrom
refactor/verify-closure-fixture

Conversation

@math3usmartins

Copy link
Copy Markdown
Member

Each executable verify file under test/fixture/compile/*/verify/ relied on an implicit, untyped $fixture variable being in the enclosing test method's scope at the point of require. The dependency was invisible from the file, untyped, and enforced only by a docblock convention across 26 driver methods.

Each verify file now returns a function (CompiledFixture $fixture): void that runs its assertions when invoked, and each driver calls it explicitly:

$runtime = require __DIR__ . '/.../verify/runtime.php';
$runtime($fixture);

The fixture dependency is now an explicit typed parameter. Behavior is unchanged: the inner require $fixture->targetDir . '/Use.php' still runs in the closure scope (so locals like $result are produced as before), assertion failures still propagate through the invocation, and the try/finally still cleans up the temp dir.

Also drop the decorative echo "OK\n" lines from the 11 files that had them: stdout is never inspected, so the Assert:: calls already signal pass/fail.

Each executable verify file under test/fixture/compile/*/verify/ relied on
an implicit, untyped $fixture variable being in the enclosing test method's
scope at the point of `require`. The dependency was invisible from the file,
untyped, and enforced only by a docblock convention across 26 driver methods.

Each verify file now returns a `function (CompiledFixture $fixture): void`
that runs its assertions when invoked, and each driver calls it explicitly:

    $runtime = require __DIR__ . '/.../verify/runtime.php';
    $runtime($fixture);

The fixture dependency is now an explicit typed parameter. Behavior is
unchanged: the inner `require $fixture->targetDir . '/Use.php'` still runs in
the closure scope (so locals like $result are produced as before), assertion
failures still propagate through the invocation, and the try/finally still
cleans up the temp dir.

Also drop the decorative `echo "OK\n"` lines from the 11 files that had them:
stdout is never inspected, so the `Assert::` calls already signal pass/fail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@math3usmartins

Copy link
Copy Markdown
Member Author

@greptile-apps

@greptile-apps

greptile-apps Bot commented Jul 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR mechanically refactors 111 compile-fixture verify files from relying on an implicit $fixture variable injected via the enclosing test method's scope into an explicit, typed closure contract: each verify file now returns a function (CompiledFixture $fixture): void that the driver loads with require and invokes with $runtime($fixture). It also drops 11 decorative echo "OK\n" statements that added no signal beyond the existing assertions.

  • Closure contract: Every verify file under test/fixture/compile/*/verify/ now returns a Closure typed as function (CompiledFixture $fixture): void; drivers call $runtime = require ...; $runtime($fixture) instead of relying on an in-scope $fixture variable.
  • Scoped require behavior preserved: Files that require $fixture->targetDir . '/Use.php' inside the returned closure still work correctly — PHP's require inside a function/closure injects variables into that scope, so locals like $result, $a, and $y remain accessible for assertions.
  • Uniform interface, but some files leave $fixture unused: A handful of verify files (e.g. nullable_return.php, specialized_interface_runtime.php) use no $fixture->targetDir requires and don't reference $fixture inside the closure body at all; they accept it purely for interface uniformity, which is intentional but may produce static-analysis warnings.

Confidence Score: 5/5

Safe to merge — this is a purely mechanical, test-infrastructure refactoring with no changes to production code.

Every verify file has been consistently updated to return a Closure, every driver has been updated to capture and invoke it, and a grep confirms no old-pattern require calls remain. PHP's require-inside-closure scoping rules mean variables produced by require inside the closure remain accessible for assertions, preserving all existing behavior.

Files Needing Attention: No files require special attention. The handful of verify files that declare $fixture as a parameter without using it (e.g. nullable_return.php) are intentionally uniform and carry no runtime risk.

Important Files Changed

Filename Overview
test/TestSupport/CompiledFixture.php Updated docblock to document the new closure-return contract with a corrected usage example; no behavioral changes to the class itself.
test/Transpiler/Monomorphize/GenericFunctionIntegrationTest.php Driver updated to $runtime = require ...; $runtime($fixture) for both verify files; pre-existing snapshot assertions are untouched.
test/Transpiler/Monomorphize/EnclosingParamBoundIntegrationTest.php Largest driver file; all 11+ require call-sites consistently updated to the new two-line pattern.
test/Transpiler/Monomorphize/ClosureConformanceIntegrationTest.php All 11 require call-sites correctly updated to the two-line pattern.
test/fixture/compile/array_sugar/verify/nullable_return.php Wrapped in closure; $fixture parameter declared but never used inside the body — safe but may trigger static-analysis unused-parameter warnings.
test/fixture/compile/closure_use_by_ref/verify/runtime.php require inside closure preserves by-reference variable semantics from the generated Use.php; assertions remain valid.
test/fixture/compile/generic_function/verify/runtime_execution.php require funcs.php inside closure still registers free functions into the global symbol table; FQN assertions unaffected.
test/fixture/compile/builtin_interface_via_use/verify/runtime.php Large verify file; try/catch block fully enclosed inside the returned closure, preserving exception-catch semantics.
test/fixture/compile/generic_exception_catch/verify/catch_runtime.php All assertSame and assertTrue(is_subclass_of(...)) checks preserved inside the closure; no regressions.
test/fixture/compile/multi_type/verify/type_error_on_wrong_slot.php try/catch for TypeError is inside the returned closure; exception propagation behavior is unchanged.

Sequence Diagram

sequenceDiagram
    participant Driver as Test Driver (testFoo method)
    participant VerifyFile as verify/runtime.php
    participant UsePhp as $fixture->targetDir/Use.php
    participant Assert as PHPUnit Assert

    Note over Driver,Assert: New pattern (this PR)

    Driver->>Driver: "$fixture = CompiledFixture::compile(...)"
    Driver->>Driver: "$fixture->registerAutoload(...)"
    Driver->>VerifyFile: "$runtime = require 'verify/runtime.php'"
    VerifyFile-->>Driver: returns Closure(CompiledFixture $fixture): void
    Driver->>VerifyFile: $runtime($fixture)
    VerifyFile->>UsePhp: "require $fixture->targetDir . '/Use.php'"
    UsePhp-->>VerifyFile: populates $result, $a, $y... in closure scope
    VerifyFile->>Assert: Assert::assertSame(...)
    Assert-->>Driver: pass / throws on failure
    Driver->>Driver: "$fixture->cleanup() [in finally]"
Loading

Reviews (1): Last reviewed commit: "test: make compile-fixture verify files ..." | Re-trigger Greptile

@math3usmartins
math3usmartins merged commit b2bdfa6 into 0.3.x Jul 24, 2026
7 checks passed
@math3usmartins
math3usmartins deleted the refactor/verify-closure-fixture branch July 25, 2026 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant