Skip to content

[js] Wait for async BiDi events and window resize in flaky tests - #17874

Merged
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:js-flaky-tests
Aug 5, 2026
Merged

[js] Wait for async BiDi events and window resize in flaky tests#17874
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:js-flaky-tests

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

  • Stops intermittent RBE failures in the JS BiDi script tests and the atoms window-size test.
  • Tests now wait for asynchronous BiDi events (console logs, JavaScript errors, DOM mutations, pinned-script output) to arrive before asserting on them, instead of reading them immediately after the triggering action.
  • The window setSize test polls the reported size (up to 2s) instead of asserting the resize took effect synchronously.

🔧 Implementation Notes

  • The waiter uses a loose != null check so tests whose handler var starts as undefined (not null) still wait rather than passing through instantly.
  • Verified locally with --runs_per_test=15 --cache_test_results=no: webdriver_script_test now passes 15/15 (previously failed several runs on the pin and DOM-mutation cases).

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the test wait/poll helpers and their usage
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • click_link_test still flakes locally on headed macOS (Tests timed out after 21 s), but it is not part of the RBE failures and is out of scope here.

🔄 Types of changes

  • Bug fix (backwards compatible)

@selenium-ci selenium-ci added B-atoms JavaScript chunks generated by Google closure C-nodejs JavaScript Bindings labels Aug 5, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Stabilize JS BiDi and window resize tests by waiting for async events

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Add explicit waits for async BiDi log, error, mutation, and pinned-script events.
• Poll for window resize completion before asserting the final dimensions.
• Reduce intermittent CI/RBE failures caused by timing-dependent assertions.
Diagram

graph TD
A["webdriver_script_test.js"] --> B["waitForLogEntry / waitForLogText"] --> C(["driver.wait (up to 5s)"]) --> D{{"BiDi async events"}} --> E["Test assertions"]
F["window_size_test.html"] --> G(["poll getSize (up to 2s)"]) --> H["verifySize assertions"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Promisify handlers (event-driven) instead of polling
  • ➕ More deterministic than polling loops
  • ➕ Clearer coupling between handler invocation and test continuation
  • ➖ Requires additional plumbing to resolve/reject promises from handler callbacks
  • ➖ More intrusive refactor across existing tests
2. Extract shared test wait utilities into a common helper module
  • ➕ Reduces duplication as more flaky async-event tests are added
  • ➕ Encourages consistent timeout messages and semantics
  • ➖ Slightly more overhead for a small set of tests
  • ➖ May require updating imports/structure across the test suite

Recommendation: Current approach is appropriate for stabilizing flakes with minimal churn: using driver.wait with a loose != null gate for single-entry handlers and includes() for log arrays. Consider extracting these helpers into a shared test utility if additional BiDi event-wait patterns appear elsewhere.

Files changed (2) +27 / -1

Tests (2) +27 / -1
window_size_test.htmlPoll window size after setSize before asserting +12/-1

Poll window size after setSize before asserting

• Converts the setSize test to async and polls bot.window.getSize() until the expected size is observed or 2s elapses. This avoids assuming the resize takes effect synchronously and reduces intermittent failures.

javascript/atoms/test/window_size_test.html

webdriver_script_test.jsWait for BiDi console/error/mutation/pinned-script events before assertions +15/-0

Wait for BiDi console/error/mutation/pinned-script events before assertions

• Adds local wait helpers that block until handler state is populated ('!= null') or until expected console text appears. Updates multiple BiDi script() tests to await event delivery before asserting, reducing timing-related flakes.

javascript/selenium-webdriver/test/lib/webdriver_script_test.js

@qodo-code-review

qodo-code-review Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Polling ignores IE tolerances ✓ Resolved 🐞 Bug ➹ Performance
Description
In window_size_test.html, the new polling loop only exits early on an exact width/height match, but
verifySize() explicitly skips assertions for IE_DOC_9 and allows approximate sizing for IE_DOC_PRE9.
In those modes the test may run until the 2s deadline even when verifySize() would already
pass/skip, adding avoidable latency to the test run.
Code

javascript/atoms/test/window_size_test.html[R76-79]

+        var actual = bot.window.getSize();
+        if ((actual.width === size.width && actual.height === size.height) ||
+            new Date().getTime() - start > 2000) {
+          verifySize(assert, size);
Evidence
The poll loop’s early-exit condition requires strict equality, but verifySize() defines the test’s
actual pass criteria (including skip/tolerance paths) which the poll condition does not account for.

javascript/atoms/test/window_size_test.html[40-59]
javascript/atoms/test/window_size_test.html[72-84]
javascript/atoms/window.js[221-244]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`QUnit.test('setSize')` now waits for an *exact* `getSize()` match before finishing early, but the existing `verifySize()` helper has browser-specific behavior:
- returns early (skip) on `bot.userAgent.IE_DOC_9`
- accepts approximate sizing on `bot.userAgent.IE_DOC_PRE9`

This mismatch can cause the poll loop to wait until the timeout even though `verifySize()` would already pass/skip.

### Issue Context
The polling is intended to reduce flakes by waiting for resize to take effect, but its completion predicate should be consistent with the assertion predicate already encoded in `verifySize()`.

### Fix Focus Areas
- javascript/atoms/test/window_size_test.html[40-85]

Suggested direction: extract a `sizeSettled(actual, expected)` predicate that mirrors `verifySize()`’s special cases (including IE_DOC_9 skip and IE_DOC_PRE9 tolerance) and use that predicate in the poll loop’s early-exit condition.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread javascript/atoms/test/window_size_test.html
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 82ce962

@titusfortner
titusfortner merged commit 88e2d0c into SeleniumHQ:trunk Aug 5, 2026
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-atoms JavaScript chunks generated by Google closure C-nodejs JavaScript Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants