Repository navigation
Conversation
waitForExpectation() re-runs the whole call under a 1s cap per attempt. For an assertion that is the right tool. For an action it re-fires a side effect: a click whose first attempt needs more than 1s has already opened its dialog, and the retry then aims at a button the overlay now covers. The test burns the whole timeout and fails on a click that worked. A retried Enter double-submits a form; a retried drag starts from an element the first attempt already moved. typeSlowly is already excluded for exactly this reason. Extend the same treatment to the other methods that repeat their effect when re-fired. The excluded branch calls the method directly, so the action gets the full timeout in one attempt — and Playwright's actionability check (attached, visible, stable, receives events, enabled) waits inside that one call, so nothing fires twice.
|
Confirmed on unmodified v5.0.1. Here's a minimal reproduction in case it helps review, drop it in use Illuminate\Support\Facades\Route;
use Pest\Browser\Playwright\Playwright;
it('issues click() exactly once even when the handler is slower than the 1000ms attempt timeout', function (): void {
Playwright::setTimeout(5000); // AwaitableWebpage skips the retry loop at <= 1000ms
Route::get('/', fn (): string => '
<input type="checkbox" id="toggle">
<script>
window.clickCount = 0;
document.getElementById("toggle").addEventListener("click", function () {
window.clickCount++;
var start = Date.now();
while (Date.now() - start < 1500) {} // blocks past the 1000ms cap
});
</script>
');
$page = visit('/');
$page->click('#toggle');
expect($page->script('() => window.clickCount'))->toBe(1);
});On v5.0.1 that gives No competing fix intended — this PR and #227 already cover it. |
|
Diagnosis looks right to me.
The bigger one is
Leaving |
|
Here is a browser-level reproduction of the data corruption this PR prevents, on current
it('appends the value only once even when the field is slow to react', function (): void {
Route::get('/', fn (): string => '<input id="field" type="text" value="A">
<script>
document.getElementById("field").addEventListener("input", () => {
const end = Date.now() + 1500;
while (Date.now() < end) {}
});
</script>');
$page = visit('/');
$page->append('#field', 'B');
expect($page->script('document.getElementById("field").value'))->toBe('AB');
});The input handler blocks the renderer for 1500ms.
What I think this adds: the failure is silent and lands far from its cause. Nothing in the eventual failure points at a retry loop — the field holds a value no one typed, whatever validates that value rejects it, and the assertion that finally fails is several steps downstream, usually about a missing element. Worth keeping in mind when judging how much this matters in practice. The one case already fixed proves the shape: Related: pestphp/pest#1755 describes the hardcoded 1000ms — applied to each Playwright command inside a retry attempt — that makes this reachable at all, independently of the configured timeout. Update, 29 September 2026 — the reproduction above is green now, and that is worth saying out loud. #269 landed on What remains is everything #269 did not name, still cut off at a fixed second. Measured on the same So this PR still has ground of its own: it excludes more actions than #269 did. The reproduction above just no longer demonstrates it. |
|
Any chance this gets merged soon? I'm assuming a ton of people are running into this issue and it really is a major time waste. It also makes tests extremely flaky |
What this fixes
AwaitableWebpage::__call()routes every page method throughExecution::waitForExpectation(), which re-runs the whole call under a hard-coded 1s cap per attempt until the configured timeout runs out, then makes one final uncapped attempt. Every Playwright error surfaces asExpectationFailedException, so a timeout is retried exactly like a failed assertion.That is correct for an assertion and wrong for an action. A click changes the page it clicks. When the first attempt needs more than 1s to complete — routine on a 2-core CI runner with a heavy SPA page — the click has already fired and a dialog is open, but the attempt is judged failed. The retry then aims at a button the dialog's overlay now covers, which can never become actionable. The test spends the whole timeout plus one uncapped attempt and fails with
Timeout Nms exceededon a click that succeeded.In practice this shows up as browser suites that are green locally going red on CI on a shifting set of tests (whichever click crossed 1s that run), and raising the timeout makes each failure take longer instead of fixing it. Related report: pestphp/pest#1511.
The change
typeSlowlyis already excluded from the retry loop, with the comment "Retrying this action would append the value to what was already typed." This PR extends the same treatment to the other methods that repeat their effect when re-fired:click/rightClick— the retry aims at a page the first attempt already changeddrag— the retry starts from an element the first attempt already movedkeys/press/pressAndWaitFor/withKeyDown— a re-sent Enter submits the form again; a multi-key sequence is replayed from the startappend— the identical hazardtypeSlowlywas excluded forDeliberately NOT excluded: the state-setting inputs (
fill,type,clear,check,uncheck,radio,select,attach,hover), where a retry is harmless, and every assertion, where retry-until-timeout is exactly right.The excluded branch calls the method directly, so the action gets the full configured timeout in one attempt. That is also the stronger wait: Playwright's actionability check (attached → visible → stable → receives events → enabled) runs inside the one call, so it waits without ever firing twice.
Verification
The included regression test simulates a slow main thread: the button's first
pointerdownbusy-loops 1.2s, and the click shows an overlay covering the button.window.downs === 1, overlay open.pintandphpstanclean.No signatures change. Configurations with a timeout ≤ 1000ms already bypass the loop entirely, so they see no behavior change.
If you would prefer a smaller first step,
click+dragalone fixes the loudest failure mode — the keyboard methods andappendare the same hazard class, just quieter. Happy to adjust.