Conversation
|
|
Both PRs it overlapped with are now merged into main (#375 and #378), so it needs a rebase. Two things to watch when you do:
|
ecc35da to
22964d6
Compare
| const cdpOk = await this.tryStartCdp() | ||
| if (generation !== this.#pollGeneration) { | ||
| return |
There was a problem hiding this comment.
If stop() runs while tryStartCdp() is awaiting after creating a CDP session or starting the screencast, stop() returns because #isRecording is still false. The generation check then exits without calling tryStopCdp(). This leaves the CDP stream and frame listener active after teardown, so capture work can leak and frames can be appended after stopping or during a later recording.
Knowledge Base Used: Visual artifacts and screencasts
| #enqueue(op: () => Promise<void>): Promise<void> { | ||
| const run = this.#queue.then(op, op) | ||
| this.#queue = run.then( | ||
| () => undefined, | ||
| () => undefined | ||
| ) | ||
| return run |
There was a problem hiding this comment.
If the service CDP handshake or the polling path's first screenshot never settles, the new serialized queue keeps stop() waiting behind start() indefinitely. Those startup operations have no timeout, so recorder finalization—and potentially the test process—cannot complete. Bound or cancel queued startup so teardown can still finish when the driver wedges.
Knowledge Base Used: Visual artifacts and screencasts
What & why
mode: 'trace'took two DOM captures per action plus a readyState poll hiding the second one's motion. On native Appium each capture is two serial round trips (GET /screenshot ~1.2 s at 1.86 MB, GET /source ~0.09s at 40 KB) — ~1.2 s per action, bracketing #351's 40–60 s/run. This restores the original one-capture design and removes the two patches built on it.Closes #351
The original approach
One capture per action in beforeCommand, taken before the command is issued — the one moment the driver is idle, so an action's result is the next action's "before" and no row resolves to a state in motion.
Stamped at the previous action's end (Date.now() for the session's first, which makes it the initial frame); afterCommand captures nothing in trace mode, it only drains the collector.
Nothing is waited for, because the gap it needs is the test's own: measured on Appium, a capture at a 0 s gap is 359–476 KB mid-transition against 1,871,924 B settled, and already settled at 0.1 s and 0.25 s. Cost: 1.19 s/action vs 2.41 s; end-to-end 12.4 s vs 19.1 s, live 5.9 s. Only the last action has no successor to hand its result to — hence a settle in exactly one place.
How it regressed
The doubling came first; the wait hid the consequence of the capture it added. The beforeCommand design was never the problem, and the native guard survived throughout — just paid twice. Nuance for review: the second capture was not only waste — the merge keeps the larger screenshot, so it supplied the settled frame whenever a test had a gap.
That accident is why a settle is needed for the last action and nowhere else.
What changed
Type of change
Packages touched
shared(types and contracts)core(framework-agnostic capture/reporting)elements(published element/snapshot API —@wdio/elements)service(WebdriverIO adapter)nightwatch-devtools(Nightwatch adapter)selenium-devtools(Selenium adapter)selenium-devtools-py(Selenium Python adapter)backend(server)app(UI)script(page-injected runtime)trace(Trace mode)Notes for reviewers
Screenshots / recordings