Skip to content

test(conformance): judge the Network tab in three BiDi modes, and fix the Python race it found - #440

Open
vishnuv688 wants to merge 2 commits into
mainfrom
test/conformance-network
Open

vishnuv688 wants to merge 2 commits into
mainfrom
test/conformance-network

Conversation

@vishnuv688

Copy link
Copy Markdown
Member

Summary

The conformance suite now checks the Network tab, and doing so exposed a race in the Python adapter's BiDi network capture. This PR carries both.

Conformance: three BiDi modes

Before this change, every scenario ran with and without BiDi but served no requests in either. An adapter that withheld the page's network rows whenever BiDi was attached therefore passed.

The fake browser now loads two subresources with every document and records them as ground truth. It reports them as Resource Timing rows, the way the page collector does. Every scenario runs in three modes:

Mode Behaviour Models
live events delivered Chrome
silent subscribed, nothing delivered Appium's XCUITest driver
off no BiDi socket Safari

Three new rules hold each adapter to the following:

  • listing every load
  • listing each load only once
  • stating a row's source where only the page could have served it

The first two are separate rules. That way a declared no-BiDi gap can excuse a withheld load without excusing a doubled one. The cross-adapter agreement now compares the requests listed too.

Fix: Python BiDi events out of order

Selenium runs each BiDi event callback on a new thread. A request's beforeRequestSent and responseCompleted handlers could therefore run concurrently. When the response won, the completed row reached the dashboard first. The late pending row then replaced it by id, and the request was left with no status or headers.

With real Chrome this shows up as a served request stuck as pending in the Network tab. The new rule request-row-carries-what-the-browser-served caught it in about one run in five.

The fix:

  • One lock: the three network handlers now share a lock, so a request's frames go out in order.
  • Early events held: a response or fetch error that arrives before its request is held, then sent right after the request's frame. Before this fix it was dropped.
  • Redirect-safe: held events are keyed by request id and redirect count, so a held hop cannot close the next hop.
  • Bounded: the held set is capped, and the teardown network summary counts the events whose request never arrived.

A changes/ fragment records it as a patch.

Testing

  • New Python tests: a threaded race, a response arriving before its request, a fetch error arriving before its request, a redirect hop, and the cap with its summary line. All except the redirect-hop test fail without the fix; that one passes on the old code too, which dropped the early events, and guards the new holding.
  • Python live scenario: 10 of 40 runs failed before the fix, and 0 of 80 after.
  • Conformance test, all adapters and modes: 42 of 42 runs passed.
  • Full pnpm test: green in 6 of 6 runs.
  • Lint and build: pnpm lint and pnpm build both pass.
  • Python suite: passes under unittest, as CI runs it.

Not verified against a real browser. The race was reproduced and fixed against the conformance suite's fake BiDi transport, which delivers events on separate threads the same way selenium does.

🤖 Generated with Claude Code

vishnuv688 and others added 2 commits October 19, 2026 18:33
The suite ran each scenario with and without BiDi and served no requests
in either, so an adapter that withheld the page's network rows whenever
BiDi was attached passed. The mode Appium's XCUITest shim presents, a
subscription that delivers nothing, was not modelled, and Safari's was
modelled with nothing to list.

The fake browser now loads two subresources with every document and
records them as ground truth, drains them as Resource Timing rows the
way the page collector reports them, and runs every scenario in three
modes: live (events delivered, as Chrome), silent (subscribed, nothing
delivered, as the Appium shim) and off (no socket, as Safari). Three
rules hold each adapter to listing every load, listing it once, and
stating a row's source where nothing but the page could have served it;
the first two are separate so the declared no-BiDi gaps can cover a
withheld load without excusing a doubled one. The cross-adapter
agreement compares the requests listed as well, and a gap explains a
difference there only where the holder's own run shows the load
withheld and the other adapter's run is clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Selenium delivers every BiDi event callback on a thread of its own, so a
request's beforeRequestSent and responseCompleted handlers ran
concurrently. When the response handler won, the completed row reached
the dashboard first and the late pending row replaced it by id, leaving
a served request with no status or headers. The conformance rule
request-row-carries-what-the-browser-served caught it about one run in
five.

One lock now covers the three network handlers. A response or fetch
error that arrives before its request is held, keyed by request id and
redirect count so a held hop cannot close the next one, and replayed
right after the request's own frame instead of being dropped. At most
BIDI_EARLY_EVENT_LIMIT are held, and the teardown summary counts the
ones whose request never arrived.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vishnuv688 vishnuv688 self-assigned this Oct 19, 2026
@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] Adds network request tracking to the conformance test suite.

The PR appears safe to merge, with a non-blocking correction needed in the teardown summary.

Findings

  1. P2 Dropped responses get mislabeled ▶

Summary

The conformance suite now checks network rows in three BiDi modes.

  • The conformance suite checks network rows in three BiDi modes.
  • Python network rows keep their request and response events in order.

One non-blocking issue remains: the teardown summary labels evicted responses as requests never seen.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[BiDi network callback] --> B[Acquire shared lock]
  B --> C{Request or closing event?}
  C -->|Request| D[Send pending row]
  D --> E{Matching held event?}
  E -->|Yes| F[Send completed or failed row]
  C -->|Closing event| G{Pending request exists?}
  G -->|Yes| F
  G -->|No| H[Hold by request id and redirect count]
  H --> I[Evict oldest entry above cap]
Loading

Reviews (1) · Last reviewed commit: "fix(selenium-devtools-py): send a reques..." · Reviewed by Greptile

captured = stats.get("captured", 0)
unanswered = len(stats.get("pending") or ())
if not captured and not unanswered:
orphaned = len(stats.get("early") or ()) + stats.get("early_dropped", 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Dropped responses get mislabeled

network_summary counts every evicted early response as one whose request was never seen. If the held set exceeds BIDI_EARLY_EVENT_LIMIT and those requests arrive afterward, their rows are listed, but early_dropped never decreases. The summary then blames missing request events instead of dropped responses. Report evicted responses separately from responses still waiting for their request.

This branch has not been deployed

No deployments
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.

1 participant