Skip to content

[java] [test] Unignore bidi network conditions tests for Firefox#17385

Merged
nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:java-unignore-networkconditions-tests
Apr 24, 2026
Merged

[java] [test] Unignore bidi network conditions tests for Firefox#17385
nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:java-unignore-networkconditions-tests

Conversation

@nvborisenko
Copy link
Copy Markdown
Member

Seems now FF supports it.

💥 What does this PR do?

This pull request makes a small change to the SetNetworkConditionsTest.java test suite by removing the @NotYetImplemented(FIREFOX) annotation from two test methods. This means these tests will now run for Firefox as well, indicating improved cross-browser support. No other significant changes are present.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

To see whether CI is green.

🔄 Types of changes

  • Cleanup (formatting, renaming)

Copilot AI review requested due to automatic review settings April 24, 2026 21:33
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Unignore BiDi network conditions tests for Firefox

🧪 Tests

Grey Divider

Walkthroughs

Description
• Remove @NotYetImplemented(FIREFOX) annotations from two test methods
• Indicates Firefox now supports BiDi network conditions functionality
• Clean up unused imports (Browser.FIREFOX and NotYetImplemented)

Grey Divider

File Changes

1. java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java 🧪 Tests +0/-4

Enable Firefox BiDi network conditions tests

• Removed @NotYetImplemented(FIREFOX) annotation from
 canSetNetworkConditionsOfflineWithContext() test method
• Removed @NotYetImplemented(FIREFOX) annotation from
 canSetNetworkConditionsOfflineWithUserContext() test method
• Removed unused import statement for Browser.FIREFOX
• Removed unused import statement for NotYetImplemented class

java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 24, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. canSetNetworkConditionsOfflineWithContext ungated 📘 Rule violation ☼ Reliability
Description
This integration test now runs unconditionally (including on Firefox) without any
feature/driver-support gate, increasing the risk of CI failures in environments where BiDi network
emulation is still unsupported or flaky. The compliance checklist requires conditioning integration
tests on feature/driver availability.
Code

java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[R47-49]

  @Test
  @NeedsFreshDriver
-  @NotYetImplemented(FIREFOX)
  void canSetNetworkConditionsOfflineWithContext() {
Evidence
PR Compliance ID 12 requires integration tests to be gated/conditional based on feature or driver
support. The updated test method annotations show no gating condition (e.g.,
@NotYetImplemented(FIREFOX) or an equivalent runtime assumption), so it will execute across all
browsers/drivers.

java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[47-49]
Best Practice: Learned patterns

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

## Issue description
`canSetNetworkConditionsOfflineWithContext` runs without any feature/driver gating, which can cause failures when a driver/browser lacks BiDi network emulation support.

## Issue Context
Compliance requires integration tests to be conditional on feature/driver availability.

## Fix Focus Areas
- java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[47-49]

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


2. Missing network reset finally 🐞 Bug ☼ Reliability
Description
In canSetNetworkConditionsOfflineWithUserContext, network conditions are reset to online only on the
happy path; if an exception occurs after setting offline, the override is never cleared. Because the
test framework does not automatically restart the driver after each test, leaked state can affect
subsequent tests that reuse the same session.
Code

java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[76]

-  @NotYetImplemented(FIREFOX)
Evidence
The context-scoped test uses a try/finally to always reset network conditions, but the user-context
variant does not; it performs the reset only before the end of the inner try block, and its catch
path closes the context without resetting. SeleniumExtension restarts the driver only before tests
marked with @NeedsFreshDriver (or similar), and its afterEach does not remove/recreate the driver,
so a failed test can leave session-level state behind for the next test that reuses the same driver
instance.

java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[60-68]
java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[71-111]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[113-121]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[141-185]

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

### Issue description
`SetNetworkConditionsTest#canSetNetworkConditionsOfflineWithUserContext` sets offline network conditions for a user context, but the reset-to-online call is not in a `finally`. If any assertion/BiDi call throws after the offline override is applied, the test exits without clearing the override.

### Issue Context
The Selenium JUnit extension does not automatically restart the driver after each test, so session-level state can leak into later tests when the driver is reused.

### Fix Focus Areas
- java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[71-111]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[113-121]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[141-185]

### Suggested change
Wrap the offline/online override in a `try/finally` (similar to `canSetNetworkConditionsOfflineWithContext`) so `online().userContexts(...)` is always sent, even if assertions fail or navigation throws.

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


Grey Divider

Qodo Logo

@selenium-ci selenium-ci added C-java Java Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Apr 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Java BiDi emulation test suite to start running the “set network conditions” (offline/online) tests on Firefox by removing the Firefox-specific @NotYetImplemented exclusions.

Changes:

  • Removed @NotYetImplemented(FIREFOX) from two network-conditions tests so they now execute on Firefox.
  • Cleaned up now-unused imports (FIREFOX and NotYetImplemented) in the test file.

@nvborisenko
Copy link
Copy Markdown
Member Author

Only ruby fails.

@nvborisenko nvborisenko merged commit 54e42e7 into SeleniumHQ:trunk Apr 24, 2026
24 of 25 checks passed
@nvborisenko nvborisenko deleted the java-unignore-networkconditions-tests branch April 24, 2026 21:50
This was referenced May 13, 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.

3 participants