Skip to content

[py][bidi]: enable history_updated event test#16236

Merged
navin772 merged 3 commits intoSeleniumHQ:trunkfrom
navin772:enable-history-updated-test-py
Aug 25, 2025
Merged

[py][bidi]: enable history_updated event test#16236
navin772 merged 3 commits intoSeleniumHQ:trunkfrom
navin772:enable-history-updated-test-py

Conversation

@navin772
Copy link
Member

@navin772 navin772 commented Aug 22, 2025 •
edited by qodo-code-review bot
Loading

User description

🔗 Related Issues

💥 What does this PR do?

Enables the history_updated event test - https://w3c.github.io/webdriver-bidi/#event-browsingContext-historyUpdated for chrome, edge and firefox. Stable versions of these browsers now support this event.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Tests

Description

  • Enable history_updated event test for Chrome, Edge, and Firefox

  • Update test implementation to use BiDi script execution

  • Add explicit wait for event handling

  • Remove browser-specific xfail markers

Diagram Walkthrough

flowchart LR
  A["Remove xfail markers"] --> B["Update script execution"]
  B --> C["Add WebDriverWait"]
  C --> D["Test now passes on all browsers"]
Loading

File Walkthrough
Relevant files
Tests
bidi_browsing_context_tests.py
Enable history_updated event test across browsers               

py/test/selenium/webdriver/common/bidi_browsing_context_tests.py

  • Remove xfail markers for Chrome, Edge, and Firefox browsers
  • Update script execution from execute_script to BiDi script.execute
  • Add WebDriverWait for event handling synchronization
  • Improve assertion logic for event validation
 +5/-7     

@selenium-ci selenium-ci added the C-py Python Bindings label Aug 22, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Aug 22, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5678 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Investigate and resolve ConnectFailure during ChromeDriver instantiation.
  • Ensure consistent behavior across instantiations.
  • Validate against specified environment versions.
  • Provide reproducible steps or fixes.

Requires further human verification:

1234 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Fix click() not triggering JavaScript in href.
  • Verify on Firefox 42.0 and ensure alert behavior.

Requires further human verification:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Flaky Timing

The event wait uses a fixed 5s timeout and checks only that at least one event arrived; consider increasing robustness by asserting specific event fields after wait and handling potential multiple events or navigation delays.

 
WebDriverWait(driver, 5).until(lambda d: len(events_received) > 0)

assert len(events_received) >= 1
assert "/new-path" in events_received[0].url
assert events_received[0].context == context_id
Assertion Specificity

The test asserts only the first event contains the expected URL; if multiple events are fired (e.g., from initial navigation), ensure the assertion searches through all received events to avoid false negatives.

 
assert len(events_received) >= 1
assert "/new-path" in events_received[0].url
assert events_received[0].context == context_id

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Aug 22, 2025 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Filter events by matching context

Validate the event corresponds to the current context to avoid flakiness in
multi-context environments. Filter to the matching context_id before asserting
URL.

py/test/selenium/webdriver/common/bidi_browsing_context_tests.py [777-779]

 assert len(events_received) >= 1
-assert "/new-path" in events_received[0].url
-assert events_received[0].context == context_id
+matching = [e for e in events_received if e.context == context_id]
+assert matching, "No history_updated event for the current context"
+assert "/new-path" in matching[0].url
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion improves the assertion logic by filtering events by context_id before checking their properties, making the test more robust against flakiness from other concurrent browsing contexts.

Medium
Add explicit wait failure message

Guard against hanging waits by adding a timeout-safe condition and raise a clear
assertion if the event doesn't arrive. Use an explicit message to aid debugging
when the event fails to fire.

py/test/selenium/webdriver/common/bidi_browsing_context_tests.py [775]

-WebDriverWait(driver, 5).until(lambda d: len(events_received) > 0)
+WebDriverWait(driver, 5).until(
+    lambda d: len(events_received) > 0,
+    message="Timed out waiting for history_updated event"
+)
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: This is a good practice suggestion that improves the test's maintainability by adding a descriptive message to the WebDriverWait, which aids in debugging test failures.

Low
Possible issue
Target script to correct context

Ensure the BiDi script is executed in the correct browsing context. Pass the
explicit target so the script runs in the current window when multiple contexts
exist, avoiding missed events.

py/test/selenium/webdriver/common/bidi_browsing_context_tests.py [774]

-driver.script.execute("() => { history.pushState({}, '', '/new-path'); }")
+driver.script.execute(
+    function_declaration="() => { history.pushState({}, '', '/new-path'); }",
+    target={"context": driver.current_window_handle}
+)
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly recommends explicitly setting the target context for script.execute, which enhances the test's robustness by preventing potential ambiguity in multi-context environments.

Low
Learned
best practice
Validate event data before use

Guard against empty or malformed event payloads before indexing into the list
and accessing fields. Validate that the first event exists and contains the
expected attributes to prevent intermittent IndexError or AttributeError.

py/test/selenium/webdriver/common/bidi_browsing_context_tests.py [775-779]

 WebDriverWait(driver, 5).until(lambda d: len(events_received) > 0)
 
-assert len(events_received) >= 1
-assert "/new-path" in events_received[0].url
-assert events_received[0].context == context_id
+assert events_received, "No history_updated events received"
+first = events_received[0]
+assert hasattr(first, "url") and "/new-path" in first.url
+assert hasattr(first, "context") and first.context == context_id
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add null checks and validation for parameters and variables before using them to prevent runtime errors.

Low
  • Update

@navin772 navin772 merged commit ca3b7cf into SeleniumHQ:trunk Aug 25, 2025
3 checks passed
This was referenced Oct 2, 2025
Closed
Closed
Closed
Closed
Closed
Merged
Merged
Closed
Merged
Closed
Closed
Closed
Merged
Merged
Closed
Merged
Merged
Closed
Closed
Closed
Merged
@dependabot dependabot bot mentioned this pull request Oct 6, 2025
Closed
This was referenced Oct 20, 2025
Closed
Merged
Closed
Closed
Closed
Closed
@dependabot dependabot bot mentioned this pull request Oct 28, 2025
Closed
This was referenced Dec 9, 2025
Open
Closed
Closed
This was referenced Dec 16, 2025
Closed
Merged
Open
Closed
Closed
Closed
Closed
This was referenced Jan 19, 2026
Open
Open
Open
Open
Open
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgoldberg cgoldberg cgoldberg approved these changes

Assignees

No one assigned

Labels

C-py Python Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@navin772 @cgoldberg @selenium-ci