Skip to content

fix: detect visit() when wrapped in another function call - #238

Open
pINKmUNster wants to merge 1 commit into
pestphp:4.xfrom
pINKmUNster:fix/detect-visit-wrapped-in-function-call
Open

fix: detect visit() when wrapped in another function call#238
pINKmUNster wants to merge 1 commit into
pestphp:4.xfrom
pINKmUNster:fix/detect-visit-wrapped-in-function-call

Conversation

@pINKmUNster

Copy link
Copy Markdown

Summary

BrowserTestIdentifier::usesFunction() requires the token immediately preceding visit( (or \visit() to be whitespace to classify a test as a browser test:

return $tokens[$i - 1][0] === T_WHITESPACE;

This misses the common pattern of wrapping visit() in another function call, e.g.:

it('opens the dialog', function () {
    someHelper(visit('/'))->click('Add new')->assertSee('New item');
});

Here the token right before visit is (, not whitespace, so isBrowserTest() returns false. The test is then never registered as a browser test, Playwright never starts, and visit() still executes — failing instantly with a WebSocket-not-connected error instead of running as intended.

We hit this in production with a project-wide helper (acknowledgeEnvironment(visit(...)) — clicks through a "testing environment" interstitial before proceeding) used across ~50 test files, all silently never running as real browser tests.

Fix

Also accept ( as a valid token preceding visit:

return $tokens[$i - 1][0] === T_WHITESPACE || $tokens[$i - 1] === '(';

Covers both the plain and fully-qualified (\visit() cases, since the fully-qualified name token check already runs before this line.

Test plan

  • Added two regression cases to tests/Unit/Support/BrowserTestIdentifierTest.php: visit() and \visit() wrapped in another function call.
  • Verified via a standalone reflection-based script (existing test file's own test closures contain literal visit( tokens, which trips the plugin's own detector and requires a real Playwright install to execute through pest) that all existing cases (plain visit(), \visit(), Livewire::visit(), method-call $page->visit(), string literal, wrapped in another call) resolve correctly with this change — no regressions.

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