Skip to content

[java] fix two flaky BiDi tests that left timing-dependent state behind - #17918

Merged
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:fix-flaky-bidi-tests
Aug 15, 2026
Merged

[java] fix two flaky BiDi tests that left timing-dependent state behind#17918
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:fix-flaky-bidi-tests

Conversation

@titusfortner

Copy link
Copy Markdown
Member

💥 What does this PR do?

  • Fixes flaky tests

🔧 Implementation Notes

  • Fixes NetworkCommandsTest.canContinueWithoutAuthCredentials, a test left open a native credential prompt in Firefox, and an @AfterEach added in [java] Fix "secure vs non-secure" error in tests #17046 issues a WebDriver command that hangs and fails. Fix also reduces test time.
  • Fixes BrowserCommandsTest.canSetDownloadBehaviorWithUserContext by filtering in-progress temp files (.part, .crdownload) out of the shared files() helper rather than patching assertions.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: investigation of the CI history, and both fixes
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Bug fix (backwards compatible)

@selenium-ci selenium-ci added C-java Java Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Aug 15, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

[java] Stabilize BiDi flaky tests by cleaning auth prompts and filtering temp downloads

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Dismiss Firefox native auth prompt left behind by BiDi auth interception test.
• Ignore in-progress download temp files to make directory comparisons timing-independent.
Diagram

graph TD
  A["BrowserCommandsTest"] --> B["files() filters temp"] --> C[("Temp download dir")]
  A --> H["BiDi Browser API"] --> C
  D["NetworkCommandsTest"] --> E["Auth failure nav"] --> G["Browser credential UI"] --> F["Dismiss prompt"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Assert on completed download events instead of filesystem listings
  • ➕ Avoids filesystem polling and temp-file naming conventions
  • ➕ More semantically aligned with 'download completed'
  • ➖ Requires event plumbing/coverage across browsers and versions
  • ➖ Higher implementation complexity for a test-only flake fix
2. Explicitly wait for temp files to settle before comparing directory contents
  • ➕ Keeps existing assertions and helper behavior intact
  • ➖ Adds sleeps/polling, increasing test runtime and still timing-sensitive under load

Recommendation: Current approach is the best tradeoff: filter known in-progress temp extensions in the shared helper (removes timing sensitivity without adding waits) and explicitly dismiss the auth prompt when browsers surface one (prevents leaked UI state and downstream hangs).

Files changed (2) +20 / -1

Bug fix (1) +11 / -0
NetworkCommandsTest.javaDismiss native credential prompt after no-credentials auth continuation +11/-0

Dismiss native credential prompt after no-credentials auth continuation

• After intentionally failing navigation with continueWithAuthNoCredentials, waits briefly for an auth alert/prompt and dismisses it when present. This prevents Firefox from leaving a native credential dialog open, keeping the session usable for subsequent cleanup and tests.

java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java

Tests (1) +9 / -1
BrowserCommandsTest.javaFilter in-progress download temp files from directory listing helper +9/-1

Filter in-progress download temp files from directory listing helper

• Introduces a pattern for common in-progress download extensions (e.g., .part/.crdownload) and updates the shared files(Path) helper to exclude them. This makes before/after directory comparisons deterministic across browsers and timing variations.

java/test/org/openqa/selenium/bidi/browser/BrowserCommandsTest.java

@qodo-code-review

qodo-code-review Bot commented Aug 15, 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. Unconditional 5s alert wait ✗ Dismissed 🐞 Bug ➹ Performance
Description
NetworkCommandsTest.canContinueWithoutAuthCredentials always executes
shortWait.until(alertIsPresent()) even though the test itself states Chrome/Edge do not surface a
prompt, so Chromium runs will reliably wait for the full shortWait timeout. This introduces a
predictable ~5s slowdown for that test on Chromium configurations and can accumulate meaningfully in
CI.
Code

java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java[R251-254]

+      try {
+        shortWait.until(alertIsPresent()).dismiss();
+      } catch (TimeoutException ignored) {
+        // Chromium-based browsers do not surface a prompt here
Evidence
The new try/catch uses shortWait.until(alertIsPresent()) and comments that Chromium doesn’t
surface a prompt, meaning it will time out on Chrome/Edge. The shared test harness defines
shortWait as a 5-second WebDriverWait, so the timeout cost is ~5 seconds when no alert exists.

java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java[231-256]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[80-87]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[263-266]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[337-341]

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

### Issue description
`NetworkCommandsTest.canContinueWithoutAuthCredentials` unconditionally waits for an alert using `shortWait` (configured as 5 seconds). The test comment explicitly notes Chromium-based browsers don’t surface a prompt here, so this wait will time out on Chrome/Edge every run, adding a consistent delay.

### Issue Context
- `shortWait` is configured to 5 seconds in the shared test harness (`SeleniumExtension`).
- The new code catches `TimeoutException` for Chromium, but that still means the full timeout is paid.

### Fix Focus Areas
- java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java[248-255]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[80-87]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[263-266]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[337-341]

### Suggested fix
Make the dismissal attempt cheap on Chromium by either:
1) Only waiting/dismissing on Firefox (e.g., use `Browser.detect()` and gate the wait), or
2) Replace `shortWait` with a much smaller, local `WebDriverWait` (e.g., 200–500ms) for the alert dismissal, or
3) Do a single non-waiting probe (`driver.switchTo().alert().dismiss()`) wrapped in `NoAlertPresentException` handling if that’s sufficient for the Firefox prompt timing.

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


Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java
@titusfortner
titusfortner merged commit 7c6f386 into SeleniumHQ:trunk Aug 15, 2026
23 checks passed
This was referenced Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants