pestphp / pestphp/pest

[pest-plugin-browser] Page::waitForLoadState() never sends its message — waitForEvent() is a silent no-op

Open Beginner friendly
#1,892 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
11.7k
Forks
538
Avg merge
4d 11h
Merged PRs (30d)
8

Description

Summary

Page::waitForLoadState() never sends anything. Client::execute() is declared : Generator and
its body yields, so calling it without iterating the returned generator runs none of its
body
— the websocket message is never written. Page::waitForLoadState() discards the return
value, so the wait is a silent no-op.

src/Playwright/Page.php (current default branch, line ~237):

public function waitForLoadState(string $state = 'load'): self
{
    Client::instance()->execute(       // <- Generator, never iterated
        $this->guid,
        'waitForLoadState',
        ['state' => $state]
    );

    return $this;
}

Compare processVoidResponse() in Api/Concerns/InteractsWithPlaywright.php, which does
iterator_to_array($response) — that is what actually drives the generator. evaluate() escapes
the bug only because processResultResponse() happens to foreach it.

Falsification

An invalid load state passes, which a real waitForLoadState could not do:

it('proves waitForEvent never reaches the server', function () {
    visit('/')->waitForEvent('banana');   // passes
});

'banana' is not a valid Playwright load state. If the message were sent, the server would
reject it. It passes because nothing is sent.

Impact

This is quiet in the worst way. waitForEvent('networkidle') reads like a guard against a
navigation race and provides none, so tests that look synchronised are not, and the resulting
failures point at whatever assertion happened to read the previous document — not at the wait.
It is worse on CI, where the race is likelier to be lost.

Suggested fix

Consume the generator, as the other call sites do:

foreach (Client::instance()->execute($this->guid, 'waitForLoadState', ['state' => $state]) as $_) {
    // drive the generator
}

A : Generator return type that is only correct when iterated is easy to misuse. It may be worth
auditing for other discarded execute() calls, and/or having execute() return a value rather
than a generator for the void cases.

Environment

pest-plugin-browser v4.3.1 (verified still present on the default branch and in v5.0.1), Pest
4.7.8, PHP 8.4, playwright 1.62.1, macOS.

Related: #1835 (the metadata.timeout protocol drift — different defect, same file, already
fixed).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/Playwright/Page.php around waitForLoadState(), then compare its Client::execute() call with processVoidResponse() in Api/Concerns/InteractsWithPlaywright.php. Use the invalid 'banana' load-state reproduction to verify the request reaches the server and is rejected, and check related discarded execute() call sites if the audit remains in scope.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.