Repository navigation
test(console): wait for controls to enable through a shared helper - #3167
Merged
Merged
Conversation
A control's enabled state can settle a task or more after the text a test
waits for, so a single isEnabled()/isDisabled() read right after that wait
can see the earlier state (finding 2116: Create Agent on a provisioning
Namespace read disabled at load ~64).
Add waitForEnabled() to the shared browser helpers and move every
"expect usable" read that follows a waitFor onto it (54 reads in 13 files),
plus the inline `.and(page.locator(":enabled")).waitFor()` waits. Reads that
check a control is disabled at a given moment stay one-shot.
…abled Internal review found one-shot "expect enabled" reads of the same shape that the first pass missed: a declaration or a one-shot assert sat between the wait and the read, or the wait was login(), waitForInputValue(), waitForCreateFormReads() or a local ready(). Move those 57 reads onto the shared helper too, and drop the member test's local waitForEnabled copy (it waited on one element handle). Disabled reads stay one-shot. waitForEnabled() now waits for the element first, so a missing control fails within the helper's own timeout instead of a 10 s isEnabled() wait.
Internal review notes: the steady-state Deploy reads after Channel Secret saves, the Create Agent form unlock after a provisioning refusal, and the Codex save after its sign-in refusal now wait too. waitForEnabled keeps one deadline across its attached wait and its poll.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Browser tests often wait for some text, then read a control's state once:
assert.equal(await create.isDisabled(), false). The control's enabled state comes from page state that can settle a task or more after that text appears (a read finishing, a refusal unlocking the form, a reload). Under load the one read sees the earlier state. That was the flake in "Create Agent names a provisioning Namespace's status before any start" (a full console run at load ~64 read Create Agent as still disabled right after the Embedded draft hint). #3101 fixed that one site with an inline.and(page.locator(":enabled")).waitFor(); the same one-shot shape is spread across the browser tests.Change
waitForEnabled(locator, timeoutMs = 10_000)intests/browser/console-agents-browser-helpers.mjs. It waits for the element, then pollslocator.isEnabled()(Playwright's own enabled check, soaria-disabledand disabled fieldsets count), and fails with the locator and the timeout.isEnabled() === true/isDisabled() === false) that follow a wait (waitFor(),login(),waitForInputValue(),waitForCreateFormReads(), ...) move onto it, in 22 browser test files. The 6 inline:enabledwaits move onto it too, andconsole-create-agent-member.test.mjsdrops its local copy (it waited on one element handle, so a re-render could leave it watching a detached node).selectOption.console-agent-logs.test.mjsandconsole-agent-overview.test.mjs(another open change is editing them; two same-shape reads there can follow).Verification
Risks
Test-only. A wait for "enabled" would also pass if a control flickered disabled and back; each moved site was read for intent, and the ones that assert a moment (held requests, "stays editable", synchronous reads) were kept one-shot. A control that never enables now fails after 10 s instead of at once.