Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,12 @@ Documented divergences from the conventions above. They exist today as debt to b
- **A command row is stamped at COMPLETION, and the DOM anchor carries the document's own birth time.** These two together are what make the replay line up; both adapters got them wrong in the same way and the fix is symmetric. (a) `selenium-devtools/src/driverPatcher.ts` and `nightwatch-devtools/src/helpers/browserProxy.ts` both ran their capture at completion but stamped `timestamp` with the *invocation* clock, keeping the invocation time as `startTime` only after this fix. The page-side mutation stream is on real time, so an invocation-stamped row ended before its own effect landed and replayed the page from before it — the `#username` fill rendered an empty field, the `#password` fill rendered only the username, and a navigation row rendered the page it had just left. Rows also now span their real duration instead of a synthetic 1 ms. (b) `collector.captureCurrentDom` (the only producer of a mutation with a `url`) stamps `performance.timeOrigin`, not the drain clock. A drain is forced from Node whenever a collector might be fresh, which is always after the navigation — a round trip at best, a whole page load at worst — so drain-stamping put the anchor after several later actions (measured: 9/15 Selenium and 8/15 Nightwatch rows on the wrong DOM). With both in place a navigation row ends after its destination document was born, so the anchor needs no repositioning at all.
- `core/trace-mutations.ts` `reattributeDomAnchors` remains as a narrow backstop for the one case the stamps can't cover: an anchor born *after* the last logged command, i.e. a click whose navigation commits once the click has already returned. It snaps such an anchor to the newest logged command, but **only when no logged command completed after it** — if one did, that command's row already resolves the anchor and pulling it earlier mis-credits it to a preceding action and steals the new page's DOM from rows still on the old one (measured: a 206 ms pull moved `/login` onto two rows that were on `/add_remove_elements`). Anchors are only pulled earlier, never past the newest timestamp already in the stream, or replay would apply the outgoing document's refs to the incoming tree.
- Residual, accepted: Nightwatch's `click` resolves *before* its navigation commits (measured 5 ms), so a submit-click row can still show its pre-navigation page. Selenium is immune — its click waits for page load. Not worth another heuristic; every heuristic tried here regressed a different row.
- **A pushed screencast needs bounding at both ends, and the obvious bound biases toward the end of the run.** Per-command capture is self-limiting — one frame per command, so the test's own length caps it — which is why `selenium-devtools-py` had no frame cap at all. Chrome's `Page.startScreencast` removes that property: `cdp_screencast.py` subscribes over a websocket of its OWN, which is a *different connection* from the session's command channel and therefore safe where the poll thread the module's docstring warns about was not. Every frame must be acked (Chrome sends nothing after an unacknowledged one, so a missed ack ends the recording rather than degrading it), and the rate is thinned at the source with `every_nth_frame` rather than buffered and discarded here.
- The buffer cap then needs care. Halving the buffer and keeping first/last — core's documented `maxBufferFrames` shape — drifts toward the run's end, because each decimation thins what is already held while new frames keep arriving unthinned: measured on a 40-frame run at a cap of 6, it kept frames 0, 1, 35, 37, 38, 39, i.e. the last moments and nothing from the middle. `_buffer` therefore thins the INCOMING frames by the same factor it has halved the buffer (`_stride` doubles per decimation), giving 0, 1, 11, 23, 31 for the same run and, at the real 2000 cap over 12000 frames, 1503 frames with 751 from the middle half. Thinning then costs the END of the run, because the last frame offered is only kept when the run happens to stop on a stride position — 41 frames at a cap of 6 ended on frame 31, eight frames stale, and 12000 at 2000 kept its last only because the two aligned. The newest skipped frame is therefore HELD rather than dropped and folded in by `_keep_tail` when the recorder stops (`finalize` stops before reading the buffer), so the video always ends where the run did — which is the part a failure is inspected for. Asserting only the endpoints does not catch this — tail truncation also leaves frame 0 plus whatever arrived since the last decimation, so the test has to assert something from the *middle* survives.
- **`driver.start_devtools()` cannot be used for this, because selenium caches ONE `_websocket_connection` per driver and hands it to whichever of BiDi or CDP asks first.** The adapter attaches BiDi before arming the screencast, so `start_devtools()` returned the BiDi socket and `Page.startScreencast` reached a BiDi endpoint: measured, `unknown command: Unknown command 'Page.startScreencast'`, followed by `BiDi command has no 'params' of type dictionary: {"method": "Page.stopScreencast"}` — that second line being the proof of which endpoint it was, and coming from a `stop` the failed start should never have sent. BiDi carries console and network, so it keeps the shared connection and the screencast opens its own.
- Resolving that endpoint needs BOTH routes. `se:cdp` is a **Grid** capability and is absent for a locally started chromedriver — the common case, and the one the demo runs — so without the `debuggerAddress` → `/json/version` → `webSocketDebuggerUrl` lookup that selenium's own `_get_cdp_details` performs, push mode would decline on every local run and the feature would be dead code. Done with stdlib urllib rather than that private method, since selenium moving internals is what broke network capture in #293. An `se:cdp` equal to `webSocketUrl` is rejected as the BiDi socket.
- **Performance timings ride on the command ROW, not a scope of their own** (`CommandLog.performance`, plus `cookies`/`documentInfo`/`result`), so the row is sent when the command completes and sent again under `replaceCommand` once the page has answered — which is why `capture_command` returns the row it built and `send_replace_command` keys on its `timestamp` rather than the per-process `id` counter. Python does **not** sleep before reading, where the JS adapters wait 500 ms: their navigation command can resolve before the load event, selenium's `get()` returns after it, and a sleep on this thread would be a real delay in the user's test rather than a detached await. A read that lands early anyway carries no `navigation` entry and is discarded rather than replacing a good row with an empty one. The read goes through `_guarded_execute_script`, or it lands in the same `execute` hook it was called from and shows up as an `executeScript` row beside every navigation — a fake driver whose `execute_script` does not route through `execute` cannot catch that, and did not.
- Per-command screenshots keep being taken for the command ROWS while a stream is live, but stop feeding the video: the pushed frames already cover the timeline and interleaving would duplicate one of them a few milliseconds off.
- **A drain must anchor the document it reads, and the flag for that has only ever had one value.** `core/script-loader.ts` `collectorDrainExpression(forceAnchor)` prepends `captureCurrentDom()` so a freshly injected collector's *async* initial anchor is not lost: the collector schedules it after `waitForBody`, so a drain issued right after a navigation beats it, reads an empty buffer, and the destination's buffer then dies with the page — leaving the navigating action with no DOM. Every production caller in both JS adapters passes `true` (selenium's `drainAfterLiveCommand`, its re-inject-after-navigation and teardown paths; nightwatch's five sites), so the `false` default is vestigial. Python's drain read `getTraceData()` with no anchor at all, which is the same missing backstop the preload does not cover; `selenium-devtools-py/src/selenium_devtools/snapshot.py` `_DRAIN_SCRIPT` now forces it **unconditionally and carries no flag** — one setting is not a knob. Forcing is free after the first anchor of a document because `packages/script` guards `captureCurrentDom` with an `#anchored` flag that deliberately survives its `reset()`, which is why selenium anchors on every live command and still emits ~3 anchors across a 16-row run rather than 16.
- **Document-start injection is what removes the whole race class; everything else is reconstruction.** `<script>`-append injection only instruments the document loaded at the time it runs, and a `<script>` dies with its document — so a navigation always yields a document we learn about afterwards, and every question that follows (when to re-inject, when to drain, which action owns the new DOM) is guesswork. `core/bidi-preload.ts` `registerCollectorPreload` registers the collector via BiDi `script.addPreloadScript` with **no browsing-context id**, which scopes it globally so contexts created later are covered: every document then instruments itself before any of its own script runs and anchors its own DOM at its own `performance.timeOrigin`. Measured on the Nightwatch example: 5 of 5 documents anchored and **0 of 19 rows on the wrong DOM**, versus 4 of 5 and 1–5 wrong with the polling/attribution approach. The service has always done this (`browser.scriptAddPreloadScript`), which is why it never had this bug class.
- **All three adapters now register it.** Selenium does so per driver in `session-lifecycle.ts` `registerPreload`, inside the `Promise.all` that `onDriverCreated` awaits — the patched `build()` thenable waits on that, so the preload is live before the first `get`; `ensureBidiCapability` already sets `webSocketUrl: true` on the Builder. Measured on a local two-page form, the appended-`<script>` path captured **21 of 29 input events** (all 8 username keystrokes lost — the collector came up ~1 s after `get`, behind `injectScript`'s ≥200 ms readiness poll and `capturePerformance`'s 500 ms settle) and **3 of 4 DOM anchors** (a destination that lived 150 ms never anchored, the recovery injection's poll never finishing); with the preload, **29 of 29** and **4 of 4**, 3/3 runs. On the cucumber example: **0** injections and **0** "collector missing" recoveries (was 6-9 per run), rows-on-wrong-document 0 of 15, trace zip 1.66-1.71 MB → 1.31-1.41 MB — the injected `<script>`'s own source is no longer part of the captured DOM.
Expand Down
17 changes: 12 additions & 5 deletions packages/app/src/components/browser/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,20 +336,27 @@ export class DevtoolsBrowser extends Element {
commandPageUrl(command, this.commands ?? [], this.mutations ?? []) ??
this.#activeUrl
}
// Switch to snapshot mode so the command snapshot is visible instead of the video.
// Switch to snapshot mode so the command snapshot is visible instead of the
// video, and let that render LAND before replaying into it. In video mode
// `#renderViewport` renders the player and no iframe at all, so a replay
// issued here targeted an element that did not exist yet: the pane stayed
// blank until the row was clicked a second time, by which point the render
// this method requested had finally created the iframe. A recording arriving
// switches the view on its own (`#handleScreencastReady`), so the very first
// row clicked after any run with a screencast hit it.
this.#viewMode = 'snapshot'
this.requestUpdate()
await this.updateComplete
// DOM time-travel: rebuild the iframe DOM to the command's RESULT state (see
// #mutationForCommand). #renderBrowserState requestUpdates internally, so
// only request one here when there's no mutation stream (screenshot fallback).
// #mutationForCommand). The await above has already rendered the screenshot
// fallback, so only the replay is left to do.
const target = mutationForCommand(
command,
this.commands ?? [],
this.mutations ?? []
)
if (target) {
await this.#renderBrowserState(target)
} else {
this.requestUpdate()
}
}

Expand Down
16 changes: 16 additions & 0 deletions packages/app/test-ui/workbench/player/snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,22 @@ describe('wdio-devtools-browser', () => {
expect(text(shadow(el, ADDRESS_BAR))).toBe(LOGIN_URL)
})

it('replays on the FIRST selection made after a recording arrives', async () => {
// A recording switches the view to video on its own, and in video mode the
// template renders the player and NO iframe. Selecting a command used to
// replay into that missing iframe and leave the pane blank, so the row had
// to be clicked twice — and since every run with a screencast auto-switches,
// the first row clicked after any run hit it.
const el = await mountBrowser(loginTrace)
await replayedPage(el)
recordingArrives()
await settle(el)

const doc = await replayAfter(el, () => selectCommand(loginTrace.submit))

expect(doc.querySelector('#flash')).toBeTruthy()
})

it('shows the DOM of the first capture before any command is selected', async () => {
const el = await mountBrowser(loginTrace)
const doc = await replayedPage(el)
Expand Down
6 changes: 4 additions & 2 deletions packages/selenium-devtools-py/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ is on `PATH`; otherwise keep it current (`brew upgrade chromedriver`).
| Network requests | Selenium **BiDi** (`Network.add_event_handler`, observe-only — never an intercept, which would pause every request) | `networkRequests` | 2 |
| Assertions | pytest hooks under pytest; line tracing for a plain script | `commands` | 2 |
| DOM snapshot (preview iframe) | inject `packages/script`, re-inject per navigation, drain mutations with a forced document anchor | `mutations` | 2 |
| Screencast video | screenshot polling → ffmpeg-encoded `.webm` | `screencast` | 2 |
| Screencast video | Chrome: CDP `Page.startScreencast` (pushed frames); elsewhere one screenshot per command → ffmpeg-encoded `.webm` | `screencast` | 2 |
| Navigation + resource timing | read from the page after a navigation, then the command row is re-sent with it | `commands` | 2 |

Element actions (`click`, `send_keys`, `text`, …) are captured for free: they
delegate to `self._parent.execute`, so the one wrapper sees them as
Expand Down Expand Up @@ -318,7 +319,8 @@ rejects re-uploading an existing version).
screenshot-polling screencast. Not yet: a CDP `Page.startScreencast` push-mode
fast-path, per-command screenshots, and performance capture.
- **Phase 3** — trace export and action snapshots. Run controls (Run / Rerun /
Run-all) and Preserve & Rerun are done — see above. Per the
Run-all), Preserve & Rerun, the pushed screencast and performance timings are
done — see above. Per the
architecture, the heavy post-processing is a candidate to live server-side in
the backend (written once) rather than re-implemented here.

Expand Down
18 changes: 18 additions & 0 deletions packages/selenium-devtools-py/scripts/gen_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
# the same fallbacks as sending nothing.
REQUIRED_RUNNER_ID = "selenium-webdriver"

# Control scopes the adapter SENDS (as opposed to receives). Generated for the
# same reason the data scopes are: a renamed scope is silent — the frame is
# delivered and dropped, so a command row simply never updates.
REQUIRED_CONTROL_SCOPES = {
"SCOPE_REPLACE_COMMAND": "replaceCommand",
}

# Data scopes the Python adapter emits — each must exist as a TraceLog key.
REQUIRED_DATA_SCOPES = {
"SCOPE_METADATA": "metadata",
Expand Down Expand Up @@ -178,6 +185,15 @@ def main() -> int:
"socket carries it, and without it every connect reads as a new run."
)

missing_control = [
v for v in REQUIRED_CONTROL_SCOPES.values() if v not in control.values()
]
if missing_control:
raise SystemExit(
f"contract drift: control scope(s) {missing_control} no longer in "
f"shared WS_SCOPE (present: {sorted(control.values())})."
)

if "testId" not in rerun_slot:
raise SystemExit(
"contract drift: `testId` is no longer a key of shared RERUN_SLOT "
Expand Down Expand Up @@ -208,6 +224,8 @@ def main() -> int:
]
for const, value in REQUIRED_DATA_SCOPES.items():
lines.append(f'{const} = "{value}"')
for const, value in REQUIRED_CONTROL_SCOPES.items():
lines.append(f'{const} = "{value}"')
lines += [
"",
f"DATA_SCOPES = frozenset({sorted(data_keys)!r})",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
SCOPE_SCREENCAST = "screencast"
SCOPE_SOURCES = "sources"
SCOPE_MUTATIONS = "mutations"
SCOPE_REPLACE_COMMAND = "replaceCommand"

DATA_SCOPES = frozenset(['actionSnapshots', 'commands', 'config', 'consoleLogs', 'logs', 'metadata', 'mutations', 'networkRequests', 'screencast', 'screencastFrames', 'sources', 'suites'])
CONTROL_SCOPES = frozenset(['clearCommands', 'clearExecutionData', 'clientConnected', 'clientDisconnected', 'config', 'replaceCommand', 'testStopped'])
Expand Down
20 changes: 18 additions & 2 deletions packages/selenium-devtools-py/src/selenium_devtools/capturer.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
SCOPE_METADATA,
SCOPE_MUTATIONS,
SCOPE_NETWORK_REQUESTS,
SCOPE_REPLACE_COMMAND,
SCOPE_SCREENCAST,
SCOPE_SOURCES,
SCOPE_SUITES,
)
from .types import SuiteStats
from .types import CommandLog, SuiteStats
from .utils import now_ms, to_jsonable


Expand Down Expand Up @@ -76,7 +77,7 @@ def capture_command(
start_time: int,
call_source: Optional[str],
screenshot: Optional[str] = None,
) -> None:
) -> CommandLog:
with self._lock:
self._command_counter += 1
command_id = self._command_counter
Expand All @@ -93,6 +94,21 @@ def capture_command(
screenshot=screenshot,
)
self._tx.send_json(SCOPE_COMMANDS, [entry])
# Returned so a caller that learns more about the command AFTER it was
# reported — a navigation's timings, which only the page can answer for
# — can enrich this exact row and replace it.
return entry

def send_replace_command(self, old_timestamp: int, entry: CommandLog) -> None:
"""Swap an already-reported row for an enriched copy of itself.

Keyed by the row's timestamp, which is what the dashboard matches on;
the adapter's own `id` counter restarts per process and would collide
across a rerun.
"""
self._tx.send_json(
SCOPE_REPLACE_COMMAND, {"oldTimestamp": old_timestamp, "command": entry}
)

# ── console / network ────────────────────────────────────────────────────────

Expand Down
Loading
Loading