Skip to content

Add Python examples for W3C BiDi network intercept commands#2639

Merged
diemol merged 27 commits into
SeleniumHQ:trunkfrom
ajithrao2509:add-python-bidi-network-examples
Jul 14, 2026
Merged

Add Python examples for W3C BiDi network intercept commands#2639
diemol merged 27 commits into
SeleniumHQ:trunkfrom
ajithrao2509:add-python-bidi-network-examples

Conversation

@ajithrao2509

@ajithrao2509 ajithrao2509 commented May 14, 2026

Copy link
Copy Markdown
Contributor

Closes #2638

Thanks for contributing to the Selenium site and documentation!

Description

Adds Python test examples for the W3C BiDi Network intercept commands, and mirrors the Python tabs across all translated docs.

Files changed:

  • examples/python/tests/bidi/test_network_commands.py — new Python test file with 4 tests
  • website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md — Python tab added to each section
  • website_and_docs/content/documentation/webdriver/bidi/w3c/network.ja.md — Python tabs mirrored
  • website_and_docs/content/documentation/webdriver/bidi/w3c/network.pt-br.md — Python tabs mirrored
  • website_and_docs/content/documentation/webdriver/bidi/w3c/network.zh-cn.md — Python tabs mirrored

Motivation and Context

The W3C BiDi Network documentation page had Java and JavaScript examples only. Python examples were completely absent — not even a badge-code placeholder. This PR adds Python equivalents for the commands we have tests for, honest badge-code placeholders for the ones we don't, and updates all translated language pages per the contributing guide.

Types of changes

  • Code example added
  • Code example added to all translated languages
  • Change to the site
  • Improved translation

Checklist

  • Follows existing file and naming conventions
  • Each test has an assertion
  • License header included
  • @pytest.mark.driver_type("bidi") marker on every test
  • gh-codeblock references use verified line numbers
  • badge-code placeholder used honestly where no test exists
  • Issue [🚀 Feature]: Add Python examples for W3C BiDi network intercept commands #2638 raised before this PR
  • Review feedback addressed (observable assertions, TimeoutException waits, safe intercept cleanup, trailing newline)

@netlify

netlify Bot commented May 14, 2026

Copy link
Copy Markdown

👷 Deploy request for selenium-dev pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 7e852cd

@CLAassistant

CLAassistant commented May 14, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ajithrao2509 ajithrao2509 marked this pull request as ready for review May 14, 2026 07:57
@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add Python examples for W3C BiDi network intercept commands

✨ Enhancement 📝 Documentation

Grey Divider

Walkthroughs

Description
• Add 4 Python test examples for W3C BiDi network commands
• Integrate Python code references into network documentation
• Include badge-code placeholders for unimplemented features
• Support network intercept, request handlers, and event operations
Diagram
flowchart LR
  A["Python Test File"] -->|"4 test functions"| B["Network Commands"]
  B -->|"add_intercept"| C["Add Intercept"]
  B -->|"remove_intercept"| D["Remove Intercept"]
  B -->|"request_handler"| E["Request Handler"]
  B -->|"fail_request"| F["Fail Request"]
  G["Documentation File"] -->|"references"| A
  G -->|"Python tabs"| H["Commands Section"]
  G -->|"Python tabs"| I["Events Section"]
Loading

Grey Divider

File Changes

1. examples/python/tests/bidi/test_network_commands.py 🧪 Tests +65/-0

Python BiDi network command test implementations

• New Python test file with 4 BiDi network command tests
• Tests cover add_intercept, remove_intercept, fail_request operations
• Tests cover add_request_handler and remove_request_handler operations
• All tests marked with @pytest.mark.driver_type("bidi") decorator

examples/python/tests/bidi/test_network_commands.py


2. website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md 📝 Documentation +37/-5

Integrate Python examples into network documentation

• Add Python tab with code references to 3 command examples (add_intercept, remove_intercept,
 fail_request)
• Add Python tab with code reference to 1 event example (add_request_handler)
• Add badge-code placeholders for 5 unimplemented Python features
• Minor whitespace cleanup (trailing spaces and final newline)

website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

Grey Divider


Action required

1. test_add_and_remove_request_handler skipped ✓ Resolved 📎 Requirement gap ☼ Reliability
Description
The required test_add_and_remove_request_handler is marked with @pytest.mark.skip, so it will
not run in CI and cannot provide the in-scope coverage required for this PR, leaving the requested
Python example/test incomplete for compliance. Because the “Before Request Sent” Python docs snippet
is sourced from this skipped test, the published documentation will also include the skip decorator
and its internal PR reference, making the example look non-runnable and potentially confusing.
Code

examples/python/tests/bidi/test_network_commands.py[R68-75]

+@pytest.mark.skip(
+    reason="request.continue_request() called from the BiDi event callback thread "
+    "races with the main thread's WebDriver calls on the shared websocket "
+    "connection, causing an intermittent KeyError in websocket_connection.py's "
+    "execute() (self._messages.pop(current_id)). Reproduced consistently in "
+    "isolation, not CI flakiness. Appears to be a thread-safety issue in the "
+    "Python BiDi client itself — see PR #2639 discussion."
+)
Evidence
PR Compliance ID 2 requires implementing the scoped tests, including
test_add_and_remove_request_handler, but the cited region in
examples/python/tests/bidi/test_network_commands.py shows the test decorated with
@pytest.mark.skip(...), which prevents execution and therefore does not satisfy the requirement
for implemented coverage. Additionally, the documentation embeds the same code region via
gh-codeblock (referencing examples/python/tests/bidi/test_network_commands.py#L68-L89), so the
skip decorator and its reason (including an internal PR-number reference) will be included verbatim
in the published “Before Request Sent” Python snippet, demonstrating the doc-quality impact.

Implement required Python BiDi network intercept tests in scope
examples/python/tests/bidi/test_network_commands.py[68-75]
examples/python/tests/bidi/test_network_commands.py[82-84]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[151-161]

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

## Issue description
`test_add_and_remove_request_handler` is currently skipped, so the PR does not actually implement/routinely exercise the required “add and remove request handler” coverage, and the Python “Before Request Sent” documentation snippet that is sourced from this test will publish a visibly skipped/non-runnable example (including an internal PR-number reference).

## Issue Context
PR compliance scope requires an implemented test for `test_add_and_remove_request_handler`; a skipped test does not satisfy this because it never executes in CI or for users running the examples. The documentation page includes a `gh-codeblock` that embeds `examples/python/tests/bidi/test_network_commands.py#L68-L89`, which includes the `@pytest.mark.skip(...)` decorator and its reason, so readers will see a skipped snippet.

## Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[68-75]
- examples/python/tests/bidi/test_network_commands.py[82-84]

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


2. No wait for handler ✓ Resolved 🐞 Bug ☼ Reliability
Description
test_fail_request asserts failed_requests is non-empty immediately after driver.get() raises
TimeoutException, but BiDi callbacks are asynchronous so the handler may not have populated the
list yet. This can cause intermittent failures where navigation times out as expected but the
callback arrives slightly later.
Code

examples/python/tests/bidi/test_network_commands.py[R56-60]

+    try:
+        driver.set_page_load_timeout(5)
+        with pytest.raises(TimeoutException):
+            driver.get("https://www.selenium.dev/selenium/web/blank.html")
+        assert len(failed_requests) > 0
Evidence
Repo BiDi examples treat handler/event callbacks as asynchronous and explicitly wait for their side
effects. test_fail_request currently does not wait before asserting, while both
test_bidi_logging and the other new network handler test do.

examples/python/tests/bidi/test_network_commands.py[56-60]
examples/python/tests/bidi/test_bidi_logging.py[7-16]
examples/python/tests/bidi/test_network_commands.py[78-80]

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

### Issue description
In `test_fail_request`, after the navigation times out, the test immediately asserts `len(failed_requests) > 0`. Since BiDi event delivery is asynchronous, this creates a race and can make the test flaky.

### Issue Context
Other BiDi example tests in this repo use `WebDriverWait` before asserting that callback-driven lists are populated.

### Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[56-60]

### Suggested change
After the `with pytest.raises(TimeoutException): driver.get(...)` block, add a short `WebDriverWait` for `failed_requests` to become non-empty, then assert.

Example:
```python
with pytest.raises(TimeoutException):
   driver.get(url)
WebDriverWait(driver, 5).until(lambda _: failed_requests)
assert failed_requests
```

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


3. driver.network._handlers accessed directly ✓ Resolved 📎 Requirement gap ⚙ Maintainability
Description
test_add_and_remove_request_handler asserts handler removal by reading the private
driver.network._handlers structure, which is not part of a stable/public API and does not follow
the established example pattern. This makes the example brittle and can break the docs snippet that
embeds this test section.
Code

examples/python/tests/bidi/test_network_commands.py[R77-80]

+    driver.network.remove_request_handler("before_request", callback_id)
+    assert callback_id not in [
+        h for h in driver.network._handlers.get("before_request", [])
+    ]
Evidence
The checklist requires the new Python BiDi network examples to follow the established
test_bidi_logging.py pattern; directly accessing private internal state (_handlers) makes the
test/example brittle and non-idiomatic. The documentation page also embeds this code range via
gh-codeblock, so this private access is published as an example.

Add Python BiDi network intercept examples test file
examples/python/tests/bidi/test_network_commands.py[77-80]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[160-160]
examples/python/tests/bidi/test_bidi_logging.py[23-27]

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

## Issue description
The example test verifies handler removal by inspecting the private `driver.network._handlers` attribute, which is an internal implementation detail and not aligned with the established Python BiDi example pattern.

## Issue Context
PR Compliance ID 1 requires the new Python examples to follow the style/pattern used in `examples/python/tests/bidi/test_bidi_logging.py`, which validates behavior via observable effects (e.g., triggering actions and asserting results) without reaching into private/internal driver state.

## Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[77-80]

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


View more (1)
4. test_fail_request never fails ✓ Resolved 📎 Requirement gap ≡ Correctness
Description
test_fail_request does not actually invoke any BiDi “fail request” capability and contains no
assertion, and it also broadly swallows exceptions, so it can pass without verifying behavior and
misrepresents the intended command example. This breaks parity with the Java example and makes the
Python “Fail request” documentation snippet unreliable and unable to catch regressions.
Code

examples/python/tests/bidi/test_network_commands.py[R35-50]

+def test_fail_request(driver):
+    import time
+    from selenium.webdriver.common.bidi.network import Request
+
+    failed = []
+
+    def on_request(request: Request):
+        failed.append(request)
+
+    driver.network.add_request_handler("before_request", on_request)
+    try:
+        driver.get("https://www.selenium.dev/selenium/web/blank.html")
+    except Exception:
+        pass
+    time.sleep(1)
+    driver.network.clear_request_handlers()
Evidence
The cited Python test registers a before_request handler and performs navigation, but only appends
to a list, sleeps, and clears handlers; it never calls any fail/abort/cancel API and never asserts
an expected outcome, while also catching exceptions in a way that can let the test pass even if
navigation fails for unrelated reasons. The documentation embeds this region as the “Fail request”
Python snippet, yet the corresponding Java example demonstrates the intended pattern—intercept,
explicitly fail the request (e.g., failRequest(...)), and assert the resulting
timeout/failure—showing the current Python example is not meeting the scoped “fail request” intent
or parity requirements.

Include scoped Python tests for specified BiDi network commands
Align Python examples with existing Java tests and Python BiDi patterns
examples/python/tests/bidi/test_network_commands.py[35-50]
examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/NetworkCommandsTest.java[96-105]
examples/python/tests/bidi/test_network_commands.py[34-51]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[125-145]
examples/python/tests/bidi/test_bidi_logging.py[7-16]

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

## Issue description
`test_fail_request` is used as the Python example for the BiDi “Fail request” command, but it currently only registers a request handler, does not actually fail/abort the request via any BiDi API, and contains no assertion; it also catches broad exceptions and ignores them, allowing the test to pass without validating behavior.

## Issue Context
The docs embed this exact test region as the Python snippet for “Fail request”, so it must demonstrate the real fail/abort capability and verify the expected outcome. Compliance also expects parity with the existing Java intent (intercept + explicit fail + assert timeout/failure), and the current Python test’s `time.sleep(1)` and non-guaranteed cleanup further weaken determinism and reliability.

## Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[34-51]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[125-145]

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



Remediation recommended

5. Docs codeblock truncated 🐞 Bug ≡ Correctness
Description
The “Before Request Sent” Python gh-codeblock references test_network_commands.py#L68-L89, which
cuts off the rest of the example and omits the wait/assert/removal logic (the test continues past
L89). This results in an incomplete documentation example across English and mirrored translations.
Code

website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[R158-161]

+{{< tab header="Python" >}}
+{{< badge-version version="4.32" >}}
+{{< gh-codeblock path="/examples/python/tests/bidi/test_network_commands.py#L68-L89" >}}
+{{< /tab >}}
Evidence
The docs reference a line range that ends at line 89, while the referenced Python function continues
through line 98; therefore, the published snippet omits part of the example’s logic.

website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[151-172]
examples/python/tests/bidi/test_network_commands.py[68-98]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.ja.md[161-171]

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

### Issue description
The Python `gh-codeblock` range for the “Before Request Sent” section ends at `#L68-L89`, but the referenced test continues through line 98. This causes the documentation snippet to omit important lines (wait/assert + handler removal), presenting an incomplete example.

### Issue Context
The same truncated range is mirrored in the translated pages, so the issue is duplicated across multiple language variants.

### Fix Focus Areas
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[158-161]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.ja.md[168-171]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.pt-br.md[168-171]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.zh-cn.md[168-171]

### Suggested fix
Update the `gh-codeblock` reference to include the complete intended example (e.g., change `#L68-L89` to `#L68-L98`, or adjust to another contiguous range that fully captures the example you want to show). Apply the same range update to the mirrored translation pages.

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


6. Truncated finally in docs ✓ Resolved 🐞 Bug ≡ Correctness
Description
The Network “Fail request” Python gh-codeblock range ends at test_fail_request’s finally:
line, so the rendered snippet is syntactically invalid (finally without an indented body). This
affects the English page and the mirrored translated pages, so readers copying the snippet will hit
a syntax error.
Code

website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[R132-135]

+{{< tab header="Python" >}}
+{{< badge-version version="4.32" >}}
+{{< gh-codeblock path="/examples/python/tests/bidi/test_network_commands.py#L40-L63" >}}
+{{< /tab >}}
Evidence
All four docs pages reference /examples/python/tests/bidi/test_network_commands.py#L40-L63, but
line 63 in that file is finally: and the required body lines (64-65) are outside the cited range,
producing an invalid snippet.

website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[125-145]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.ja.md[135-145]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.pt-br.md[135-145]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.zh-cn.md[135-145]
examples/python/tests/bidi/test_network_commands.py[57-66]

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

### Issue description
The docs embed the Python “Fail request” example with a line range that ends at `finally:` (without including the indented cleanup statements). This renders an invalid Python snippet on the docs site.

### Issue Context
In `examples/python/tests/bidi/test_network_commands.py`, the `finally:` line is followed by the cleanup body on the next lines. The docs currently stop at the `finally:` line.

### Fix Focus Areas
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[125-145]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.ja.md[135-155]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.pt-br.md[135-155]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.zh-cn.md[135-155]

### Suggested change
Update the Python Fail request codeblock from:
- `...test_network_commands.py#L40-L63`

to a range that includes the full `try/finally` (e.g. `#L42-L65`, so it includes `finally:` plus its body). Apply the same change to EN/JA/PT-BR/ZH-CN pages.

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


7. Intercept not removed ✓ Resolved 🐞 Bug ☼ Reliability
Description
test_add_intercept registers a network intercept but never removes it, leaving extra BiDi state
active for the rest of the session. This is inconsistent with the repo’s other language examples
that explicitly close/tear down the Network module and can make failures harder to reason about if
the test expands or if teardown behavior changes.
Code

examples/python/tests/bidi/test_network_commands.py[R23-28]

+@pytest.mark.driver_type("bidi")
+def test_add_intercept(driver):
+    # _add_intercept is currently the available Python API for BiDi network intercepts.
+    # This will be updated when a public API is stabilized.
+    intercept = driver.network._add_intercept()
+    assert intercept is not None
Evidence
The Python test creates an intercept and does not remove it, while the repo’s Java/JS examples
explicitly scope/close the Network module, indicating expected cleanup behavior in examples.

examples/python/tests/bidi/test_network_commands.py[23-29]
examples/javascript/test/bidirectional/network_commands.spec.js[22-25]
examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/NetworkCommandsTest.java[33-40]

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

### Issue description
`test_add_intercept` creates an intercept via `driver.network._add_intercept()` but never removes it. This leaves extra BiDi network interception state active until `driver.quit()`, which is inconsistent with other repo examples that explicitly close/tear down the network module.

### Issue Context
Other language examples in this repo always release network resources (JS `afterEach` closes the Network module; Java uses try-with-resources). Align the Python example by removing the intercept after asserting.

### Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[23-28]

### Suggested change
Wrap the intercept creation in `try/finally` and call `_remove_intercept(intercept["intercept"])` in `finally` (after checking `intercept` is not None).

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


View more (7)
8. Intercept cleanup can mask errors ✓ Resolved 🐞 Bug ☼ Reliability
Description
In test_fail_request, the intercept returned by _add_intercept() is never validated before
indexing intercept["intercept"] in the finally block, so a None/unexpected return value will
raise during cleanup and can hide the original failure. This makes test failures harder to diagnose
and can turn a real failure into a misleading teardown error.
Code

examples/python/tests/bidi/test_network_commands.py[R46-61]

+    intercept = driver.network._add_intercept()
+
+    def on_request(request: Request):
+        failed_requests.append(request)
+        driver.network.fail_request(request.request["requestId"])
+
+    driver.network.add_request_handler("before_request", on_request)
+
+    try:
+        driver.set_page_load_timeout(5)
+        with pytest.raises(TimeoutException):
+            driver.get("https://www.selenium.dev/selenium/web/blank.html")
+        assert len(failed_requests) > 0
+    finally:
+        driver.network._remove_intercept(intercept["intercept"])
+        driver.network.clear_request_handlers()
Evidence
The test uses the _add_intercept() return value as a dictionary in finally without any
assertion/guard, unlike test_add_intercept which asserts the return is non-null before proceeding.

examples/python/tests/bidi/test_network_commands.py[46-61]
examples/python/tests/bidi/test_network_commands.py[23-28]

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

### Issue description
`test_fail_request` indexes `intercept["intercept"]` during cleanup without first validating the `_add_intercept()` return value. If `_add_intercept()` fails (returns `None` or an unexpected structure), the `finally` block can raise a new exception and obscure the original test failure.

### Issue Context
`test_add_intercept` already asserts the intercept is not `None`, but `test_fail_request` does not, even though it relies on the intercept shape during teardown.

### Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[46-61]
- examples/python/tests/bidi/test_network_commands.py[23-28]

### Suggested change
- Capture an `intercept_id` safely right after creation (and assert it is present), e.g.:
 - `intercept = driver.network._add_intercept()`
 - `assert intercept and "intercept" in intercept`
 - `intercept_id = intercept["intercept"]`
- In `finally`, guard removal with `if intercept_id is not None:` to avoid cleanup exceptions masking the real failure.

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


9. Swallowed navigation exception ✓ Resolved 🐞 Bug ◔ Observability
Description
test_fail_request catches and ignores any Exception from driver.get(), which makes unrelated
failures hard to diagnose and can mask the real cause when the test fails. The test should only
suppress the expected navigation failure (or at least capture/log the exception) instead of
discarding it unconditionally.
Code

examples/python/tests/bidi/test_network_commands.py[R56-59]

+        try:
+            driver.get("https://www.selenium.dev/selenium/web/blank.html")
+        except Exception:
+            pass
Evidence
The driver.get(...) failure is currently ignored entirely with no type/message validation or
capture, so unexpected failures become silent and harder to debug.

examples/python/tests/bidi/test_network_commands.py[56-62]

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

### Issue description
`test_fail_request` uses `except Exception: pass` around `driver.get(...)`, which discards all navigation errors and reduces debuggability.

### Issue Context
This test likely expects navigation to fail due to intentionally failing the request, but swallowing all exceptions also hides unexpected infrastructure/session/DNS errors.

### Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[56-59]

### Suggested fix
Narrow or preserve the exception information, for example:
- Catch the expected Selenium exception type(s) only, and re-raise anything else.
- Or capture the exception into a variable and include it in an assertion/failure message if `failed_requests` never arrives (so failures are diagnosable).

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


10. No wait after removal ✓ Resolved 🐞 Bug ☼ Reliability
Description
After removing the request handler, the test immediately asserts the request count is unchanged
right after a second driver.get(), so it can pass before any late-arriving before_request callback
would be appended. This makes the test timing-dependent and can miss regressions where the handler
is still active but fires slightly after the assertion.
Code

examples/python/tests/bidi/test_network_commands.py[R87-88]

+    driver.get("https://www.selenium.dev/selenium/web/blank.html")
+    assert len(requests) == request_count
Evidence
The first navigation already uses WebDriverWait to account for async callback delivery; the second
navigation lacks any wait, making the final assertion vulnerable to racing the callback queue.

examples/python/tests/bidi/test_network_commands.py[80-88]

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

### Issue description
The handler-removal verification asserts immediately after the second navigation, which can race with async event delivery.

### Issue Context
Earlier in the same test, `WebDriverWait(...).until(lambda _: requests)` is used to account for async callback delivery.

### Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[80-83]
- examples/python/tests/bidi/test_network_commands.py[85-88]

### Suggested fix
After the second `driver.get(...)`, add a deterministic *negative* wait to ensure no new callbacks arrive, e.g.:
- Wait for page load completion, then
- `WebDriverWait(driver, 1).until(lambda _: len(requests) > request_count)` and assert that this times out (i.e., handler truly removed).

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


11. test_fail_request uses time.sleep ✓ Resolved 📎 Requirement gap ☼ Reliability
Description
test_fail_request relies on a fixed time.sleep(1) and broadly swallows Exception from
driver.get(...), diverging from the established WebDriverWait-based approach used in
test_bidi_logging.py and making the example race-prone and potentially flaky on slower/loaded CI.
This both reduces reliability of the example test/documentation snippet and adds unnecessary fixed
latency instead of waiting only until the expected condition is met.
Code

examples/python/tests/bidi/test_network_commands.py[R56-62]

+        try:
+            driver.get("https://www.selenium.dev/selenium/web/blank.html")
+        except Exception:
+            pass
+
+        time.sleep(1)
+        assert len(failed_requests) > 0
Evidence
PR Compliance ID 4 calls for the new Python BiDi example tests to follow the deterministic
event-waiting pattern already used in examples/python/tests/bidi/test_bidi_logging.py, which
relies on WebDriverWait rather than fixed sleeps. In contrast, the cited code in
test_fail_request sleeps for a fixed duration before asserting the callback-populated list is
non-empty and uses a blanket except Exception: pass, indicating it is not waiting
deterministically for async BiDi callback delivery and may intermittently fail depending on timing
even when the feature works.

Follow the established Python BiDi example test pattern
examples/python/tests/bidi/test_network_commands.py[56-62]
examples/python/tests/bidi/test_network_commands.py[55-65]
examples/python/tests/bidi/test_bidi_logging.py[6-16]

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

## Issue description
`test_fail_request` currently waits for an async BiDi callback using a fixed `time.sleep(1)` and ignores all exceptions from `driver.get(...)`, which is inconsistent with the established Python BiDi example test pattern and can introduce race conditions, flakiness, and unnecessary fixed latency.

## Issue Context
Other BiDi tests in this repo (e.g., `examples/python/tests/bidi/test_bidi_logging.py`) wait for callback-populated lists/events deterministically using `WebDriverWait(...).until(...)` rather than sleeping for a fixed duration, which better matches the asynchronous nature of BiDi callback delivery and avoids intermittent failures on slower/loaded CI.

## Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[56-62]

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


12. Cleanup not in finally ✓ Resolved 🐞 Bug ☼ Reliability
Description
In test_fail_request, intercept/handler cleanup is performed after the assertion, so any earlier
exception or assertion failure will skip cleanup. This is test-hygiene risk that can obscure the
original failure and diverges from the repo’s established “always-cleanup” pattern (e.g., Java
try-with-resources).
Code

examples/python/tests/bidi/test_network_commands.py[R60-61]

+    assert len(failed_requests) > 0
+    driver.network._remove_intercept(intercept["intercept"])
Evidence
The cleanup lines are placed after the assertion in the Python test, meaning they are skipped if the
assert fails. The driver fixture always runs driver.quit() in teardown, and the Java example uses
a try (...) block to ensure cleanup even when assertions/exceptions occur.

examples/python/tests/bidi/test_network_commands.py[60-62]
examples/python/tests/conftest.py[39-42]
examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/NetworkCommandsTest.java[98-103]

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

## Issue description
`test_fail_request` performs cleanup (removing intercept / clearing handlers) only after the assertion, so cleanup will not run if the assertion fails or if an exception occurs before reaching those lines.

## Issue Context
These examples are used both as tests and as documentation snippets; keeping them robust and self-cleaning improves debuggability and reduces teardown noise.

## Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[52-63]

## Suggested change
Wrap the navigation/wait/assert portion in `try:` and put `_remove_intercept(...)` and `clear_request_handlers()` into `finally:` so cleanup always runs (even on assertion failure).

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


13. Private intercept APIs documented ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The Python intercept examples use underscored methods (_add_intercept/_remove_intercept), which
are private by convention and may change without notice. Publishing these in docs encourages
reliance on non-public APIs and makes the documentation/tests brittle.
Code

examples/python/tests/bidi/test_network_commands.py[R22-31]

+def test_add_intercept(driver):
+    intercept = driver.network._add_intercept()
+    assert intercept is not None
+
+
+@pytest.mark.driver_type("bidi")
+def test_remove_intercept(driver):
+    intercept = driver.network._add_intercept()
+    driver.network._remove_intercept(intercept["intercept"])
+    assert driver.network.intercepts == []
Evidence
The new Python tests call underscored methods and are embedded directly into the docs; existing
Python BiDi examples use non-underscored, public-style methods, showing a different (more stable)
pattern for documentation snippets.

examples/python/tests/bidi/test_network_commands.py[21-32]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[13-57]
examples/python/tests/bidi/test_bidi_logging.py[6-16]

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

## Issue description
The Python intercept examples rely on private-by-convention methods (leading underscore). If these are the only available APIs today, they should not be presented as the primary user-facing documentation snippet without a stable wrapper.

## Issue Context
These snippets are embedded into the public docs page under “Add network intercept” and “Remove network intercept”. Other Python BiDi examples in this repo use public APIs without leading underscores.

## Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[21-32]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[13-57]

## Implementation notes
- Prefer switching examples to the supported public API (if available) for adding/removing intercepts.
- If there is no public API yet, consider:
 - adding/using a documented public wrapper in the examples, or
 - replacing the Python tab with an honest placeholder (`badge-code`) until the API is public/stable.

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


14. Request handler removal unverified ✓ Resolved 📎 Requirement gap ☼ Reliability
Description
test_add_and_remove_request_handler removes the handler but never triggers any request/navigation
or asserts the handler was invoked (or stopped being invoked), diverging from established Python
BiDi example patterns and making the “Before Request Sent” docs snippet potentially misleading. As
written, the test can pass even if the event subscription/handler mechanism is broken.
Code

examples/python/tests/bidi/test_network_commands.py[R54-65]

+def test_add_and_remove_request_handler(driver):
+    from selenium.webdriver.common.bidi.network import Request
+
+    requests = []
+
+    def callback(request: Request):
+        requests.append(request)
+
+    callback_id = driver.network.add_request_handler("before_request", callback)
+    assert callback_id is not None
+
+    driver.network.remove_request_handler("before_request", callback_id)
Evidence
Rule 3 points to matching the Python BiDi example pattern used in test_bidi_logging.py, where
handler removal is validated by triggering the relevant event and asserting no additional entries
were collected; in contrast, the network request-handler example only checks for a non-null handler
id and never drives a network request nor uses/asserts on the collected list to prove the “Before
Request Sent” event was observed. Additionally, the docs embed this Python snippet as the “Before
Request Sent” example, while the existing JavaScript example shows the expected flow (subscribe,
navigate to trigger traffic, and assert an event was captured), highlighting the missing trigger and
assertions in the Python version.

Align Python examples with existing Java tests and Python BiDi patterns
examples/python/tests/bidi/test_network_commands.py[54-65]
examples/python/tests/bidi/test_bidi_logging.py[18-28]
examples/python/tests/bidi/test_network_commands.py[53-65]
website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[151-172]
examples/javascript/test/bidirectional/network_events.spec.js[22-34]

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

## Issue description
The Python “Before Request Sent” example/test registers and removes a request handler but never triggers a network request/navigation and never asserts that the callback fires (or stops firing after removal), so it doesn’t actually demonstrate the event and can pass even if the subscription is broken.

## Issue Context
This test block is embedded in the docs as the Python example for the “Before Request Sent” event, so it should follow the established BiDi example pattern seen in other Python examples (e.g., logging) and the structure of the existing JS example: trigger a deterministic request, wait for the event, assert at least one event was captured, and optionally trigger a second request after handler removal to verify the handler no longer fires.

## Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[53-65]
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[151-172]

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



Informational

15. Missing trailing newline 🐞 Bug ⚙ Maintainability ⭐ New
Description
examples/python/tests/bidi/test_network_commands.py ends without a final newline, which can cause
formatting/lint noise in repos that enforce POSIX newlines and makes future diffs messier.
Code

examples/python/tests/bidi/test_network_commands.py[R87-88]

+    driver.get("https://www.selenium.dev/selenium/web/blank.html")
+    assert not requests
Evidence
The file ends at line 88 (assert not requests) and the PR diff indicates it has no newline at EOF.

examples/python/tests/bidi/test_network_commands.py[87-88]

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

### Issue description
The newly added Python example test file is missing a terminating newline at end-of-file.

### Issue Context
This is a minor formatting/hygiene issue; some tooling and style checks expect a POSIX newline at EOF.

### Fix Focus Areas
- examples/python/tests/bidi/test_network_commands.py[87-88]

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


16. Missing trailing newline 🐞 Bug ⚙ Maintainability
Description
network.en.md ends without a final newline, which commonly causes minor lint/formatting noise and
makes future diffs messier. This is a low-severity documentation hygiene issue.
Code

website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[237]

+{{< /tabpane >}}
Evidence
The file ends at the closing {{< /tabpane >}} with no additional terminating line, matching the PR
diff’s “No newline at end of file” marker.

website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[231-237]

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

## Issue description
`website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md` does not end with a trailing newline. While this won’t change rendered output, it can trigger formatting/lint checks and creates noisy diffs.

## Issue Context
The file currently ends immediately after the final `{{< /tabpane >}}`.

## Fix Focus Areas
- website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md[237-237]

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


Grey Divider

Qodo Logo

Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
@rpallavisharma

Copy link
Copy Markdown
Member

please add details for all languages. please see the contributing doc - https://www.selenium.dev/documentation/about/contributing/

Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 4ae1fa1

…lations

Avoid private _handlers checks, always clean up fail_request in finally, and add Python tabs to ja/pt-br/zh-cn docs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit ad1316b

Expect TimeoutException for failed navigation, drop time.sleep, and use a negative WebDriverWait after handler removal.

Co-authored-by: Cursor <cursoragent@cursor.com>
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 0495a2e

Comment thread website_and_docs/content/documentation/webdriver/bidi/w3c/network.en.md Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 35d8edf

Address Qodo hygiene comment on PR 2639.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit dbbc57f

Assert intercept exists and use intercept_id in finally so teardown cannot mask the original failure.

Co-authored-by: Cursor <cursoragent@cursor.com>
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 2e6b273

@ajithrao2509

Copy link
Copy Markdown
Contributor Author

It seems that this is still failing. Have you tried this locally? Is it passing for you?

Checked both nightly failures, neither is in my changed files. tests/browsers/test_console_log.py fails on Windows nightly with a file-locking PermissionError, and tests/actions/api/test_mouse.py::test_move_by_offset_from_element fails on Ubuntu nightly with a ValueError parsing coordinates. Both look like pre-existing issues on the nightly channel, unrelated to this PR. I did also notice a warning (not a failure) in test_fail_request 'Request' object has no attribute 'fail_request' on nightly which suggests possible API drift between the stable and nightly Selenium builds.

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 14af512

@diemol

diemol commented Jul 12, 2026

Copy link
Copy Markdown
Member

No, I don't mean the other failures. I meant the specific test you are adding.

@ajithrao2509

Copy link
Copy Markdown
Contributor Author

No, I don't mean the other failures. I meant the specific test you are adding.

To directly answer: no, it did not pass for me locally, consistently I ran it in isolation multiple times and it fails the same way each time with the KeyError as I described. Happy to share the exact local run output if useful. Given that, is the skip the right call here, or would you prefer I keep digging into a fix on my end first?

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit e69f90d

Comment thread examples/python/tests/bidi/test_network_commands.py Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 6afbf2b

@ajithrao2509

Copy link
Copy Markdown
Contributor Author

hi @diemol Pushed an update that removes the skip and fixes the underlying issues. Summary of what I found and changed:

Root cause of the KeyError: an unsynchronized message-ID race in WebSocketConnection.execute() in selenium 4.41.0 (which examples/python/requirements.txt pins) BiDi event callbacks run on separate threads, and concurrent execute() calls can collide on the shared _id/_messages state. This was fixed upstream in selenium 4.44.0 via _id_lock. Rather than keep the test skipped until the pin is bumped, test_add_and_remove_request_handler now follows the same structure as the same-named test in py/test/selenium/webdriver/common/bidi_network_tests.py: the handler is removed before navigating, so no request is ever blocked awaiting continuation which avoids both the hang and the race on the pinned version.

While validating locally, I also found test_fail_request was passing without exercising the command. With classic driver.get(), network.failRequest consistently returns Invalid InterceptionId on Chrome, and the expected TimeoutException occurred only because the blocked request was never actually failed. This is consistent with the classic-navigation skip in the selenium test suite ("Request handlers don't yet work in Chrome using classic navigation"). The test now navigates via browsing_context.navigate the same approach the suite's fail-request tests use and asserts the raised WebDriverException, so the example genuinely demonstrates the command. The callback guards fail_request() with a narrow except WebDriverException, because the event subscription also delivers events for requests already cancelled by the aborted navigation. I also removed a redundant manual intercept (add_request_handler already creates one internally).

All four tests pass locally on selenium 4.41.0 + Chrome, no skips, no warnings:

tests/bidi/test_network_commands.py::test_add_intercept PASSED
tests/bidi/test_network_commands.py::test_remove_intercept PASSED
tests/bidi/test_network_commands.py::test_fail_request PASSED
tests/bidi/test_network_commands.py::test_add_and_remove_request_handler PASSED
==================================================== 4 passed in 16.05s ==========================================

Docs line ranges updated across all four language pages to match the final file.

On the expected CI results: stable should be green on both OSes. The two nightly failures visible on recent runs test_move_by_offset_from_element (ValueError, Ubuntu) and test_log_level (PermissionError, Windows) are pre-existing on the nightly channel and not in this PR's files; the scheduled nightly runs on trunk are currently failing as well, independent of any PR. The only expected reference to this PR's file on nightly is a warning from the fail_requestfail() API rename on trunk.

Note for later: selenium 4.44+ replaced this API with a high-level handler form (request.fail(), automatic continuation). When the examples' selenium pin is next bumped, I'd be happy to modernize these examples in a follow-up PR mirroring the current bidi_network_tests.py patterns.

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 95dd1bf

@diemol

diemol commented Jul 12, 2026

Copy link
Copy Markdown
Member

So, what was the issue? I tend to avoid reading a copy & pasted text from AI.

@ajithrao2509

Copy link
Copy Markdown
Contributor Author

So, what was the issue? I tend to avoid reading a copy & pasted text from AI.

The actual was the examples pin selenium 4.41, which has a threading bug in the BiDi websocket client (no lock on command ids, fixed in selenium 4.44). My callback was sending a command while another event was being processed and the responses collided, that was the KeyError. I rewrote the test the same way selenium's own test_add_and_remove_request_handler works (remove the handler before navigating) so nothing is blocked and the bug never triggers, skip is gone. While retesting I also found test_fail_request was passing without actually failing the request, Chrome rejects failRequest during driver.get, so it now uses browsing_context.navigate like selenium's own tests. All 4 pass locally on 4.41 + Chrome.

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 3d0eb81

@diemol

diemol commented Jul 13, 2026

Copy link
Copy Markdown
Member

Interesting. Now we bumped the test suite to 4.46, and this new test is failing again, could you please double check?

https://github.com/SeleniumHQ/seleniumhq.github.io/actions/runs/29239632949/job/86782316021?pr=2639#step:11:63

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 7e852cd

@ajithrao2509

Copy link
Copy Markdown
Contributor Author

Interesting. Now we bumped the test suite to 4.46, and this new test is failing again, could you please double check?

https://github.com/SeleniumHQ/seleniumhq.github.io/actions/runs/29239632949/job/86782316021?pr=2639#step:11:63

hi @diemol I have made the changes as necessary, earlier when I ran them in my local the selenium version was 4.41, so my validation was against the old pin. On 4.46 the request object has fail() instead of fail_request() which is why the tests broke after the version bump. I rewrote the test_fail_request using the 4.46 handler API same as test_fail_requests_matching_url_pattern in the selenium test suite and verfied the tests locally which are currently passing without any failures on Chrome. Updated the document line ranges as well.

@diemol diemol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great, thank you for your effort!

@diemol diemol dismissed their stale review July 14, 2026 07:52

All good

@diemol diemol merged commit 62e3d32 into SeleniumHQ:trunk Jul 14, 2026
4 of 7 checks passed
diemol pushed a commit that referenced this pull request Jul 14, 2026
#2699)

Fix Python doc line ranges and badge for BiDi network examples (#2639 follow-up)

Co-authored-by: Diego Molina <diemol@users.noreply.github.com>

[deploy site]
selenium-ci added a commit that referenced this pull request Jul 14, 2026
#2699)

Fix Python doc line ranges and badge for BiDi network examples (#2639 follow-up)

Co-authored-by: Diego Molina <diemol@users.noreply.github.com>

[deploy site] d1fecf0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🚀 Feature]: Add Python examples for W3C BiDi network intercept commands

4 participants