From 8b6a6dac388132f6032ef832dcf9c6c1ab51093d Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 02:59:57 +0530 Subject: [PATCH 01/14] fix(app): replay boolean attributes by presence, not by value --- .../components/browser/mutation-at-command.ts | 4 +- .../app/src/components/browser/snapshot.ts | 73 +++++++++- .../test-ui/workbench/player/snapshot.test.ts | 130 +++++++++++++++--- .../app/tests/mutation-at-command.test.ts | 13 +- 4 files changed, 195 insertions(+), 25 deletions(-) diff --git a/packages/app/src/components/browser/mutation-at-command.ts b/packages/app/src/components/browser/mutation-at-command.ts index add2d1cc..f605da2b 100644 --- a/packages/app/src/components/browser/mutation-at-command.ts +++ b/packages/app/src/components/browser/mutation-at-command.ts @@ -19,7 +19,9 @@ export function mutationForCommand( commands: CommandLog[], mutations: TraceMutation[] ): TraceMutation | undefined { - if (!command?.timestamp || !mutations.length) { + // `timestamp` is required by the contract and 0 is a real value — the first + // command of a normalized or standalone trace — so only absence bails out. + if (command === undefined || !mutations.length) { return undefined } const idx = commands.indexOf(command) diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index d31bcd66..45756360 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -52,6 +52,44 @@ const textChildren = (el: Node) => (node): node is Text => node.nodeType === Node.TEXT_NODE ) +/** HTML boolean content attributes: presence alone is the state, so a captured + * "false" has to REMOVE one — written verbatim, `checked="false"` reads as + * checked. Curated rather than probed off the element, because a probe both + * misses and misfires: `readonly` has no same-named property (it is `readOnly`), + * while `draggable`, `spellcheck` and `translate` do carry boolean properties + * yet their attributes are enumerated, where "false" is a meaningful value. */ +const BOOLEAN_ATTRIBUTES = new Set([ + 'allowfullscreen', + 'autofocus', + 'autoplay', + 'checked', + 'controls', + 'default', + 'disabled', + 'formnovalidate', + 'inert', + 'ismap', + 'itemscope', + 'loop', + 'multiple', + 'muted', + 'novalidate', + 'open', + 'playsinline', + 'readonly', + 'required', + 'reversed', + 'selected' +]) + +/** State a captured boolean attribute carries. The collector emits form-field + * state as `String(el.checked)` and every other record as the attribute's own + * value, so only a literal "false" — and a record carrying no value, which + * leaves no attribute state to set — means off: an empty value is a present + * attribute (`` reaches the wire as `''`). */ +const booleanAttributeOn = (value?: string) => + value !== undefined && value.toLowerCase() !== 'false' + declare global { interface WindowEventMap { 'screencast-ready': CustomEvent<{ @@ -451,7 +489,8 @@ export class DevtoolsBrowser extends Element { } #handleAttributeMutation(mutation: TraceMutation) { - if (!mutation.attributeName) { + const name = mutation.attributeName + if (!name) { return } @@ -460,15 +499,35 @@ export class DevtoolsBrowser extends Element { return } + if (BOOLEAN_ATTRIBUTES.has(name.toLowerCase())) { + this.#applyBooleanAttribute( + el, + name, + booleanAttributeOn(mutation.attributeValue) + ) + return + } + const value = mutation.attributeValue ?? '' - el.setAttribute(mutation.attributeName, value) + el.setAttribute(name, value) // Form-field state lives on the PROPERTY, not just the attribute — mirror it - // so a replayed input shows the captured value / checked state, including a - // field cleared back to empty. - if (mutation.attributeName === 'value' && 'value' in el) { + // so a replayed input shows the captured value, including a field cleared + // back to empty. + if (name === 'value' && 'value' in el) { ;(el as HTMLInputElement).value = value - } else if (mutation.attributeName === 'checked' && 'checked' in el) { - ;(el as HTMLInputElement).checked = value === 'true' + } + } + + /** Presence IS the state of a boolean attribute, so the captured state is + * toggled rather than written — the markup a re-serialization reads then says + * what the replayed page shows. `checked` is mirrored onto the property as + * well because checkedness stops tracking the attribute once anything sets it + * (the captured page's fields arrive as preact property writes); every other + * boolean attribute reflects its property, so the toggle moves both. */ + #applyBooleanAttribute(el: HTMLElement, name: string, on: boolean) { + el.toggleAttribute(name, on) + if (name === 'checked' && 'checked' in el) { + ;(el as HTMLInputElement).checked = on } } diff --git a/packages/app/test-ui/workbench/player/snapshot.test.ts b/packages/app/test-ui/workbench/player/snapshot.test.ts index 00297994..fa0bc410 100644 --- a/packages/app/test-ui/workbench/player/snapshot.test.ts +++ b/packages/app/test-ui/workbench/player/snapshot.test.ts @@ -13,6 +13,7 @@ import { import { mutationForCommand } from '@components/browser/mutation-at-command.js' import '@components/browser/snapshot.js' +import { commandLog, mutation } from '../../support/builders.js' import { mountWithContext, settle } from '../../support/mount.js' import { shadow, shadowAll, text, texts } from '../../support/queries.js' import { @@ -181,6 +182,19 @@ function input(doc: Document, selector: string): HTMLInputElement { return el } +/** The element as a re-parse of its own markup yields it — what an export, a + * copy-as-HTML or any other re-serialization of the replayed page reads, and the + * only reader that tells a boolean attribute's PRESENCE from its value. */ +function reparse(el: HTMLElement, selector: string): HTMLInputElement { + const parsed = new DOMParser() + .parseFromString(el.outerHTML, 'text/html') + .querySelector(selector) + if (!parsed) { + throw new Error(`No ${selector} in the re-parsed markup`) + } + return parsed +} + const boxesIn = (el: Browser, selector: string) => Array.from(replayDoc(el)?.querySelectorAll(selector) ?? []) @@ -296,23 +310,16 @@ describe('wdio-devtools-browser', () => { ) const remember = input(doc, '#remember') - // The property mirror is the half that is right, and the half the replayed - // page is drawn from. + // The rendered field... expect(remember.checked).toBe(false) - // SOURCE BUG, pinned as-is (snapshot.ts:464 `setAttribute` runs for every - // attribute mutation, before the property mirror on :470). `checked` is a - // BOOLEAN content attribute — its presence means checked whatever the value - // — so the replayed markup says TICKED while the property says otherwise. - // On screen the property wins, so replay is unaffected; anything that - // re-serializes the page (export, copy-as-HTML, outerHTML round-trip) reads - // the lie. Asserted through a real re-parse rather than the attribute's - // spelling, so a fix flips a visible assertion instead of sneaking past: - // the correct value is `null`, and `reticked` then becomes false. - const reticked = new DOMParser() - .parseFromString(remember.outerHTML, 'text/html') - .querySelector('#remember')! - expect(remember.getAttribute('checked')).toBe('false') - expect(reticked.checked).toBe(true) + // ...and its markup agree. `checked` is a BOOLEAN content attribute — its + // presence means checked whatever the value — so a captured "false" has to + // remove it; written verbatim, the markup reads as TICKED while the + // property says otherwise, and everything that re-serializes the page + // (export, copy-as-HTML, outerHTML round-trip) carries the lie. Asserted + // through a real re-parse rather than the attribute's spelling. + expect(remember.getAttribute('checked')).toBeNull() + expect(reparse(remember, '#remember').checked).toBe(false) }) it('moves the radio selection to the option the test picked', async () => { @@ -347,6 +354,72 @@ describe('wdio-devtools-browser', () => { }) }) + /** + * `checked` is the boolean attribute the collector actually emits state for + * (`String(el.checked)` on every input/change), but nothing about the replay is + * checkbox-specific — every boolean attribute reaches the same code, so these + * cases drive the general shape through `disabled` and `readonly` on a text + * field and through a cleared `checked` on the captured radio. + */ + describe('boolean attributes', () => { + /** + * Replays one boolean-attribute mutation on the login page. The typed + * username rides along as the signal that the window landed — the attribute + * under test cannot be that signal, since it is what the assertions read. + */ + async function replayAttribute( + target: string, + attributeName: string, + attributeValue?: string + ): Promise { + const entry = mutation({ target, attributeName, attributeValue }) + const el = await mountBrowser({ + commands: loginTrace.commands, + mutations: [loginTrace.loginDocument, loginTrace.usernameTyped, entry] + }) + await replayedPage(el) + const doc = await replayAfter(el, () => selectMutation(entry)) + await waitUntil( + () => input(doc, '#username').value === TYPED_USERNAME, + 'the replay window to be applied' + ) + return doc + } + + it('drops a boolean attribute the capture recorded as false', async () => { + const doc = await replayAttribute(REF.username, 'disabled', 'false') + + // Written verbatim, `disabled="false"` DISABLES the field — the state the + // capture says the page was not in. + const username = input(doc, '#username') + expect(username.getAttribute('disabled')).toBeNull() + expect(username.disabled).toBe(false) + expect(reparse(username, '#username').disabled).toBe(false) + }) + + it('removes a boolean attribute a mutation carries no value for', async () => { + // The captured radio has `checked="checked"`, so the removal is observable: + // a cleared attribute written as an empty one stays present and keeps + // reading as checked. + const doc = await replayAttribute(REF.planGuest, 'checked') + + const guest = input(doc, '#plan-guest') + expect(guest.getAttribute('checked')).toBeNull() + expect(guest.checked).toBe(false) + expect(reparse(guest, '#plan-guest').checked).toBe(false) + }) + + it('keeps a boolean attribute the capture carries with an empty value', async () => { + // `` reaches the wire as an empty value, so empty is the + // PRESENT state — reading the string for truthiness would drop it. + const doc = await replayAttribute(REF.username, 'readonly', '') + + const username = input(doc, '#username') + expect(username.readOnly).toBe(true) + expect(reparse(username, '#username').readOnly).toBe(true) + }) + }) + describe('command selection', () => { it('shows the page a navigating click produced, not the one it left', async () => { const el = await mountBrowser(loginTrace) @@ -399,6 +472,31 @@ describe('wdio-devtools-browser', () => { expect(doc.querySelector('form#login')).toBeTruthy() }) + it('replays the DOM window of a command captured at timestamp 0', async () => { + // `CommandLog.timestamp` is required and 0 is reachable — the first command + // of a normalized or standalone trace. Resolved for truthiness the player + // is handed no window at all and keeps whatever page it already showed. + const first = commandLog({ + command: 'url', + args: [LOGIN_URL], + startTime: 0, + timestamp: 0 + }) + const el = await mountBrowser({ + commands: [first, loginTrace.readFlash], + mutations: loginTrace.mutations + }) + await replayedPage(el) + + const doc = await replayAfter(el, () => selectCommand(first)) + + // `readFlash` starts after the secure-page anchor, so that anchor is where + // this command's window ends — the login form the initial replay showed is + // gone, which no stale page could produce. + expect(doc.querySelector('#flash')).toBeTruthy() + expect(doc.querySelector('form#login')).toBeNull() + }) + it('replays a text change recorded as a character-data mutation', async () => { const el = await mountBrowser(loginTrace) await replayedPage(el) diff --git a/packages/app/tests/mutation-at-command.test.ts b/packages/app/tests/mutation-at-command.test.ts index be8c70b5..88bd92eb 100644 --- a/packages/app/tests/mutation-at-command.test.ts +++ b/packages/app/tests/mutation-at-command.test.ts @@ -50,7 +50,18 @@ describe('mutationForCommand', () => { ).toEqual(mut(100)) }) - it('returns undefined without a command timestamp or mutations', () => { + it('resolves the window of a command captured at timestamp 0', () => { + // `CommandLog.timestamp` is required and 0 is reachable — the first command + // of a normalized or standalone trace. Read for truthiness it bails out and + // the snapshot player is handed no DOM at all for that command. + const first = cmd(0, 0) + const second = cmd(300, 250) + expect(mutationForCommand(first, [first, second], mutations)).toBe( + mutations[2] + ) + }) + + it('returns undefined without a command or without mutations', () => { expect(mutationForCommand(undefined, [], mutations)).toBeUndefined() expect(mutationForCommand(cmd(100), [], [])).toBeUndefined() }) From ed23bc31394001f2b0f256059111ebbcdc520e08 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 03:00:09 +0530 Subject: [PATCH 02/14] fix(app): make show-command the logs panel's only input, and tear it down --- packages/app/src/components/workbench/logs.ts | 142 ++++++++---- .../app/test-ui/workbench/panels/logs.test.ts | 214 ++++++++++++------ 2 files changed, 248 insertions(+), 108 deletions(-) diff --git a/packages/app/src/components/workbench/logs.ts b/packages/app/src/components/workbench/logs.ts index 2c065283..5a350107 100644 --- a/packages/app/src/components/workbench/logs.ts +++ b/packages/app/src/components/workbench/logs.ts @@ -1,6 +1,12 @@ import { Element } from '@core/element' -import { html, css, nothing, type TemplateResult } from 'lit' -import { customElement, property } from 'lit/decorators.js' +import { + html, + css, + nothing, + type PropertyValues, + type TemplateResult +} from 'lit' +import { customElement, state } from 'lit/decorators.js' import type { CommandLog } from '@wdio/devtools-shared' import type { CommandEndpoint } from '@wdio/protocols' @@ -9,15 +15,59 @@ import { commandCategory } from './actionItems/category.js' import { formatDuration } from './actionItems/duration.js' const SOURCE_COMPONENT = 'wdio-devtools-logs' + +// The protocol tables are a large payload the dashboard only needs once a +// command is selected, so they are imported on demand and memoised here. +let commandDefinitions: Record | undefined +let commandDefinitionsLoad: Promise> | undefined + +function loadCommandDefinitions(): Promise> { + commandDefinitionsLoad ??= import('@wdio/protocols').then((protocols) => { + const { + WebDriverProtocol, + MJsonWProtocol, + AppiumProtocol, + ChromiumProtocol, + SauceLabsProtocol, + SeleniumProtocol, + GeckoProtocol, + WebDriverBidiProtocol + } = protocols + commandDefinitions = Object.values({ + ...WebDriverProtocol, + ...MJsonWProtocol, + ...AppiumProtocol, + ...ChromiumProtocol, + ...SauceLabsProtocol, + ...SeleniumProtocol, + ...GeckoProtocol, + ...WebDriverBidiProtocol + }).reduce( + (acc, endpoint) => { + for (const cmdDesc of Object.values(endpoint)) { + acc[cmdDesc.command] = cmdDesc as CommandEndpoint + } + return acc + }, + {} as Record + ) + return commandDefinitions + }) + return commandDefinitionsLoad +} + +/** Detail view of one command in the Log tab. Its only input is the window + * `show-command` event, dispatched by the Actions row, the player timeline and + * keyboard command navigation. */ @customElement(SOURCE_COMPONENT) export class DevtoolsCommandLogs extends Element { + /** Derived from `command` on every change, so a definition can never outlive + * the command it describes. */ #commandDefinition?: CommandEndpoint - @property({ type: Object }) - command?: CommandLog + @state() private command?: CommandLog - @property({ type: Number }) - elapsedTime?: number + @state() private elapsedTime?: number static styles = [ ...Element.styles, @@ -143,46 +193,54 @@ export class DevtoolsCommandLogs extends Element { ` ] + /** A stable field rather than a bound method or inline arrow: only the exact + * reference that was added can be removed again. */ + #onShowCommand = (event: Event): void => { + const { command, elapsedTime } = (event as CustomEvent) + .detail + this.elapsedTime = elapsedTime + this.command = command + + // Source line-tracking is dispatched by the Actions handler; here we only + // surface the command's detail in the Log tab. + this.closest('wdio-devtools-tabs')?.activateTab('Log') + } + connectedCallback(): void { super.connectedCallback() - window.addEventListener('show-command', async (ev: CustomEvent) => { - const command = ev.detail.command - this.elapsedTime = ev.detail.elapsedTime + window.addEventListener('show-command', this.#onShowCommand) + } - const { - WebDriverProtocol, - MJsonWProtocol, - AppiumProtocol, - ChromiumProtocol, - SauceLabsProtocol, - SeleniumProtocol, - GeckoProtocol, - WebDriverBidiProtocol - } = await import('@wdio/protocols') - const endpoints = Object.values({ - ...WebDriverProtocol, - ...MJsonWProtocol, - ...AppiumProtocol, - ...ChromiumProtocol, - ...SauceLabsProtocol, - ...SeleniumProtocol, - ...GeckoProtocol, - ...WebDriverBidiProtocol - }).reduce( - (acc, endpoint) => { - for (const cmdDesc of Object.values(endpoint)) { - acc[cmdDesc.command] = cmdDesc as CommandEndpoint - } - return acc - }, - {} as Record - ) - this.#commandDefinition = endpoints[command.command] - this.command = command + // Lit calls connectedCallback again on every re-connect, so a listener left + // behind keeps a discarded panel alive on `window` — it leaks, and it keeps + // reacting to selections meant for the panel that replaced it. + disconnectedCallback(): void { + super.disconnectedCallback() + window.removeEventListener('show-command', this.#onShowCommand) + } - // Source line-tracking is dispatched by the Actions handler; here we only - // surface the command's detail in the Log tab. - this.closest('wdio-devtools-tabs')?.activateTab('Log') + // Untyped map: `keyof this` drops private members, so `PropertyValues` + // cannot name the internal `command` state this derivation keys on. + willUpdate(changed: PropertyValues): void { + if (changed.has('command')) { + this.#resolveDefinition(this.command) + } + } + + #resolveDefinition(command?: CommandLog): void { + this.#commandDefinition = + command && commandDefinitions + ? commandDefinitions[command.command] + : undefined + if (!command || commandDefinitions) { + return + } + void loadCommandDefinitions().then((definitions) => { + // Another command may have been selected while the tables loaded. + if (this.command === command) { + this.#commandDefinition = definitions[command.command] + this.requestUpdate() + } }) } diff --git a/packages/app/test-ui/workbench/panels/logs.test.ts b/packages/app/test-ui/workbench/panels/logs.test.ts index 641f2439..60d75795 100644 --- a/packages/app/test-ui/workbench/panels/logs.test.ts +++ b/packages/app/test-ui/workbench/panels/logs.test.ts @@ -69,16 +69,12 @@ const attrOf = (el: Element | null, name: string) => const prettyValue = (value: unknown) => JSON.stringify(value, null, 2).replace(/\s+/g, ' ') -/** Property path: what the workbench does when it renders the panel directly. - * No protocol definition is resolved, so no description or reference exists. */ -async function mountLogs( - command?: CommandLog, - elapsedTime?: number -): Promise { - const panel = await mount(PANEL, { - command, - elapsedTime - }) +/** One macrotask, so a listener the panel failed to detach has had its turn — + * without it, "the removed panel did not react" would pass vacuously. */ +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) + +async function mountLogs(): Promise { + const panel = await mount(PANEL) await settle(panel) return panel } @@ -94,9 +90,8 @@ async function waitFor(cond: () => boolean, what: string): Promise { throw new Error(`Timed out waiting for ${what}`) } -/** Event path: the Actions panel's row click. The handler resolves the command's - * protocol definition through a dynamic import before it assigns `command`, - * which is why this waits rather than awaiting one render. */ +/** The panel's only input: the window event the Actions row, the player timeline + * and keyboard command navigation all dispatch. */ async function showCommand( panel: DevtoolsCommandLogs, command: CommandLog, @@ -106,13 +101,38 @@ async function showCommand( new CustomEvent('show-command', { detail: { command, elapsedTime } }) ) await waitFor( - () => panel.command === command, - `the panel to pick up ${command.command}` + () => text(shadow(panel, NAME)) === command.command, + `the panel to render ${command.command}` ) await settle(panel) } +/** A panel shown one command and then detached. `show-command` is a window-wide + * broadcast, so detaching is what keeps one panel per selection — it works only + * because the panel unlistens on disconnect. */ +async function panelShowing( + command: CommandLog, + elapsedTime?: number +): Promise { + const panel = await mountLogs() + await showCommand(panel, command, elapsedTime) + panel.remove() + return panel +} + describe('wdio-devtools-logs', () => { + // The protocol tables are imported on demand, so the suite's first resolution + // lands an update after its command; every later one is synchronous. + before(async () => { + const panel = await mountLogs() + await showCommand(panel, commandLog({ command: 'navigateTo' })) + await waitFor( + () => shadowAll(panel, REFERENCE).length === 1, + 'the protocol tables to load' + ) + panel.remove() + }) + describe('empty state', () => { it('asks for a selection while no command has been chosen', async () => { const panel = await mountLogs() @@ -140,7 +160,7 @@ describe('wdio-devtools-logs', () => { describe('header', () => { it('names the selected command', async () => { - const panel = await mountLogs(commandLog({ command: 'elementClick' })) + const panel = await panelShowing(commandLog({ command: 'elementClick' })) expect(text(shadow(panel, NAME))).toBe('elementClick') }) @@ -149,7 +169,7 @@ describe('wdio-devtools-logs', () => { const commands = ['click', 'navigateTo', 'expect.toHaveText', 'getUrl'] const panels = [] for (const command of commands) { - panels.push(await mountLogs(commandLog({ command }))) + panels.push(await panelShowing(commandLog({ command }))) } // Derived: the dot must follow the classifier's verdict for that command. @@ -165,14 +185,16 @@ describe('wdio-devtools-logs', () => { }) it('falls back to the other category for a command it cannot classify', async () => { - const panel = await mountLogs(commandLog({ command: 'takeScreenshot' })) + const panel = await panelShowing( + commandLog({ command: 'takeScreenshot' }) + ) expect(categoryOf(panel)).toBe('cat-other') }) it("renders the command's elapsed time in human units", async () => { - const milliseconds = await mountLogs(commandLog(), 320) - const seconds = await mountLogs(commandLog(), 1500) + const milliseconds = await panelShowing(commandLog(), 320) + const seconds = await panelShowing(commandLog(), 1500) expect(text(shadow(milliseconds, DURATION))).toBe(formatDuration(320)) expect(text(shadow(seconds, DURATION))).toBe(formatDuration(1500)) @@ -181,21 +203,20 @@ describe('wdio-devtools-logs', () => { }) it('renders a zero elapsed time rather than dropping it', async () => { - const panel = await mountLogs(commandLog(), 0) + const panel = await panelShowing(commandLog(), 0) expect(text(shadow(panel, DURATION))).toBe(formatDuration(0)) expect(text(shadow(panel, DURATION))).toBe('0ms') }) it('renders no duration for a command selected without one', async () => { - const panel = await mountLogs(commandLog()) + const panel = await panelShowing(commandLog()) expect(shadowAll(panel, DURATION)).toHaveLength(0) }) it('links to the protocol reference of a command it can resolve', async () => { - const panel = await mountLogs() - await showCommand(panel, commandLog({ command: 'navigateTo' })) + const panel = await panelShowing(commandLog({ command: 'navigateTo' })) const link = shadow(panel, REFERENCE) expect(text(link)).toBe('Reference ↗') @@ -204,36 +225,38 @@ describe('wdio-devtools-logs', () => { }) it('renders no reference link for a command outside the protocols', async () => { - const panel = await mountLogs() - await showCommand(panel, commandLog({ command: 'click' })) + const panel = await panelShowing(commandLog({ command: 'click' })) expect(shadowAll(panel, REFERENCE)).toHaveLength(0) }) - // SOURCE BUG, pinned as it behaves today: `command` is a public - // `@property` (logs.ts:16-17) but the protocol lookup only runs inside the - // `show-command` listener (logs.ts:180), so one and the same command - // renders its Reference link and Description by event and neither by - // property. Resolving the definition on input flips the property column. - it('resolves the protocol of a command by event but not of one assigned as a property', async () => { - const byEvent = await mountLogs() - await showCommand(byEvent, commandLog({ command: 'navigateTo' })) - // Mounted after the event on purpose: `show-command` is listened for on - // `window` (logs.ts:148) and never unlistened, so a panel alive at - // dispatch time picks the event up whether or not it was the target. - const byProperty = await mountLogs(commandLog({ command: 'navigateTo' })) - - expect(shadowAll(byEvent, REFERENCE)).toHaveLength(1) - expect(sectionTitles(byEvent)).toContain('Description') - expect(shadowAll(byProperty, REFERENCE)).toHaveLength(0) - expect(sectionTitles(byProperty)).not.toContain('Description') + // Was pinned as a source bug: `command`/`elapsedTime` were public inputs + // nothing bound, and the protocol lookup lived inside the event listener, so + // the same command rendered its reference and description by event and + // neither by property. The inputs are internal state now, the event is the + // contract, and the definition is derived from the command the panel holds — + // so each panel keeps the protocol of its own selection. + it('resolves the protocol of each selection against the panel showing it', async () => { + const first = await panelShowing(commandLog({ command: 'navigateTo' })) + const second = await panelShowing( + commandLog({ command: 'executeScript', args: ['return 1', []] }) + ) + + expect(text(shadow(first, NAME))).toBe('navigateTo') + expect(attrOf(shadow(first, REFERENCE), 'href')).toBe(NAVIGATE_REF) + expect(sectionTitles(first)).toContain('Description') + expect(text(shadow(second, NAME))).toBe('executeScript') + expect(sectionTitles(second)).toContain('Description') + expect(sectionNamed(second, 'Parameters').keys).toEqual([ + 'script', + 'args' + ]) }) }) describe('description', () => { it("renders the protocol's description of the selected command", async () => { - const panel = await mountLogs() - await showCommand(panel, commandLog({ command: 'navigateTo' })) + const panel = await panelShowing(commandLog({ command: 'navigateTo' })) expect(sectionTitles(panel)[0]).toBe('Description') expect(text(shadow(panel, DESCRIPTION))).toMatch( @@ -242,19 +265,25 @@ describe('wdio-devtools-logs', () => { }) it('renders no description section for a command outside the protocols', async () => { + const panel = await panelShowing(commandLog({ command: 'click' })) + + expect(sectionTitles(panel)).not.toContain('Description') + expect(shadowAll(panel, DESCRIPTION)).toHaveLength(0) + }) + + it('drops the resolved protocol when the next command has none', async () => { const panel = await mountLogs() + await showCommand(panel, commandLog({ command: 'navigateTo' })) await showCommand(panel, commandLog({ command: 'click' })) expect(sectionTitles(panel)).not.toContain('Description') - expect(shadowAll(panel, DESCRIPTION)).toHaveLength(0) + expect(shadowAll(panel, REFERENCE)).toHaveLength(0) }) }) describe('parameters', () => { it("names each argument after the protocol's parameter", async () => { - const panel = await mountLogs() - await showCommand( - panel, + const panel = await panelShowing( commandLog({ command: 'navigateTo', args: [LOGIN_URL] }) ) @@ -266,9 +295,7 @@ describe('wdio-devtools-logs', () => { }) it('names every argument of a multi-parameter command', async () => { - const panel = await mountLogs() - await showCommand( - panel, + const panel = await panelShowing( commandLog({ command: 'executeScript', args: ['return document.title', []] @@ -281,7 +308,7 @@ describe('wdio-devtools-logs', () => { }) it('falls back to the argument position when no parameter name is known', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'setValue', args: ['#username', 'tomsmith'] }) ) @@ -290,7 +317,7 @@ describe('wdio-devtools-logs', () => { it('pretty-prints an object argument', async () => { const size = { width: 1600, height: 900 } - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'setWindowSize', args: [size] }) ) @@ -303,7 +330,7 @@ describe('wdio-devtools-logs', () => { }) it('renders a null argument as null and flags the value as empty', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'deleteCookies', args: [null] }) ) @@ -312,7 +339,7 @@ describe('wdio-devtools-logs', () => { }) it('renders an oversized argument in full rather than truncating it', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'execute', args: [LONG_ARG] }) ) @@ -320,7 +347,7 @@ describe('wdio-devtools-logs', () => { }) it('renders no parameters section for a command called without arguments', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'getTitle', args: [] }) ) @@ -332,7 +359,7 @@ describe('wdio-devtools-logs', () => { describe('result', () => { it('renders one row per entry of an object result', async () => { const rect = { x: 8, y: 240, width: 176, height: 32 } - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'getElementRect', args: [], result: rect }) ) @@ -350,7 +377,7 @@ describe('wdio-devtools-logs', () => { }) it('renders a string result as a single value row', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'getTitle', args: [], result: 'The Internet' }) ) @@ -362,7 +389,7 @@ describe('wdio-devtools-logs', () => { }) it('keys an array result by position', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'findElements', args: ['css selector', 'a'], @@ -374,7 +401,7 @@ describe('wdio-devtools-logs', () => { }) it('renders a false result rather than treating it as absent', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'isElementSelected', args: [], result: false }) ) @@ -382,7 +409,7 @@ describe('wdio-devtools-logs', () => { }) it('renders an empty-string result as a row rather than dropping it', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'getText', args: [], result: '' }) ) @@ -391,7 +418,7 @@ describe('wdio-devtools-logs', () => { }) it('renders no result section for a command that returned nothing', async () => { - const panel = await mountLogs( + const panel = await panelShowing( commandLog({ command: 'elementClick', args: [], result: null }) ) @@ -401,9 +428,7 @@ describe('wdio-devtools-logs', () => { describe('section order', () => { it('renders the description, then the parameters, then the result', async () => { - const panel = await mountLogs() - await showCommand( - panel, + const panel = await panelShowing( commandLog({ command: 'navigateTo', args: [LOGIN_URL], @@ -435,4 +460,61 @@ describe('wdio-devtools-logs', () => { expect(sectionTitles(panel)).toEqual(['Description', 'Result']) }) }) + + describe('teardown', () => { + it('stops reacting to selections once it leaves the document', async () => { + const panel = await mountLogs() + await showCommand(panel, commandLog({ command: 'navigateTo' })) + panel.remove() + + window.dispatchEvent( + new CustomEvent('show-command', { + detail: { command: commandLog({ command: 'getTitle' }) } + }) + ) + await flush() + await settle(panel) + + expect(text(shadow(panel, NAME))).toBe('navigateTo') + expect(attrOf(shadow(panel, REFERENCE), 'href')).toBe(NAVIGATE_REF) + }) + + it('hands the selection to the live panel without waking the replaced one', async () => { + const replaced = await mountLogs() + await showCommand(replaced, commandLog({ command: 'navigateTo' })) + replaced.remove() + const live = await mountLogs() + + await showCommand(live, commandLog({ command: 'getTitle' })) + await flush() + + expect(text(shadow(live, NAME))).toBe('getTitle') + expect(text(shadow(replaced, NAME))).toBe('navigateTo') + }) + + // The dispatchers sit outside the dock tab the panel renders in, so the + // event is a window-wide broadcast by contract: every connected panel + // follows the same selection, and detaching is what separates them. + it('follows one selection in every panel that is still connected', async () => { + const first = await mountLogs() + const second = await mountLogs() + + await showCommand(first, commandLog({ command: 'getTitle' })) + await settle(second) + + expect(text(shadow(first, NAME))).toBe('getTitle') + expect(text(shadow(second, NAME))).toBe('getTitle') + }) + + it('reacts again once it is re-connected', async () => { + const panel = await mountLogs() + panel.remove() + document.body.append(panel) + + await showCommand(panel, commandLog({ command: 'navigateTo' })) + + expect(text(shadow(panel, NAME))).toBe('navigateTo') + expect(shadowAll(panel, HEAD)).toHaveLength(1) + }) + }) }) From 1dc84aa52f45e8373e6a464430a91f5ef65bd77b Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 03:00:54 +0530 Subject: [PATCH 03/14] fix(app): time a console entry captured at zero like any other --- .../app/src/components/workbench/console.ts | 25 +++--- .../test-ui/workbench/panels/console.test.ts | 86 ++++++++++++++++--- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/packages/app/src/components/workbench/console.ts b/packages/app/src/components/workbench/console.ts index a9492d69..f5af8f31 100644 --- a/packages/app/src/components/workbench/console.ts +++ b/packages/app/src/components/workbench/console.ts @@ -208,13 +208,15 @@ export class DevtoolsConsoleLogs extends Element { @state() private activeLevel: ConsoleLevelFilter = 'all' - #startTime?: number - + // Read from the current logs on every call rather than cached: the context + // value is replaced when a new run starts, and a cached origin would elapse + // that run's rows from the previous run's first log. #formatElapsedTime(timestamp: number): string { - if (this.#startTime === undefined) { - this.#startTime = this.logs?.[0]?.timestamp ?? timestamp - } - const elapsed = (timestamp - this.#startTime!) / 1000 + const origin = this.logs?.[0]?.timestamp ?? timestamp + // Browser and terminal logs are stamped by different clocks, so an entry + // can predate the first captured one; clamped because a negative elapsed + // (and `-0.0s` from sub-100ms skew) describes nothing a reader can use. + const elapsed = Math.max(0, timestamp - origin) / 1000 return `${elapsed.toFixed(1)}s` } @@ -258,13 +260,14 @@ export class DevtoolsConsoleLogs extends Element { } #renderLogEntry(log: ConsoleLog) { - const icon = LOG_ICONS[log.type] || LOG_ICONS.log + // `LOG_ICONS` is keyed by the levels that have an icon of their own, so a + // miss here is a real absence — unlike `log.type`/`log.timestamp`, which + // `ConsoleLog` requires and which are therefore rendered unguarded. + const icon = LOG_ICONS[log.type] ?? LOG_ICONS.log const badge = log.source ? CONSOLE_SOURCE_BADGE[log.source] : undefined return html` -
-
- ${log.timestamp ? this.#formatElapsedTime(log.timestamp) : ''} -
+
+
${this.#formatElapsedTime(log.timestamp)}
${icon}
${badge ? html`${badge.label}` diff --git a/packages/app/test-ui/workbench/panels/console.test.ts b/packages/app/test-ui/workbench/panels/console.test.ts index 671e66f4..05e035ae 100644 --- a/packages/app/test-ui/workbench/panels/console.test.ts +++ b/packages/app/test-ui/workbench/panels/console.test.ts @@ -41,9 +41,12 @@ const MESSAGE_LITERALS = [ ] /** Elapsed time as the panel measures it: seconds since the *first captured* - * log, to one decimal. Written out because `#formatElapsedTime` is private. */ + * log, to one decimal, floored at zero for an entry stamped before that log. + * Written out because `#formatElapsedTime` is private. */ const elapsed = (timestamp: number) => - `${((timestamp - loginConsole.logs[0].timestamp) / 1000).toFixed(1)}s` + `${(Math.max(0, timestamp - loginConsole.logs[0].timestamp) / 1000).toFixed( + 1 + )}s` async function mountConsole(logs: ConsoleLog[]): Promise { const panel = await mountWithContext(PANEL, [ @@ -183,23 +186,82 @@ describe('wdio-devtools-console-logs', () => { expect(texts(panel, TIME)).toEqual(['0.0s', '0.4s', '1.2s', '2.5s']) }) - // SOURCE BUG, pinned as it behaves today: `ConsoleLog.timestamp` is - // required (`shared/src/types.ts:210`), so 0 is a captured time like any - // other, and the panel's own convention for the first captured log is - // `0.0s` — asserted above and again below. `console.ts:266` guards the cell - // on truthiness instead, so an entry timestamped 0 loses its time. Reading - // the cell as an element rather than through `text()` is what separates - // "blank cell" from "no cell at all": `text(null)` is `''` too. - it('blanks the time cell of an entry captured at timestamp 0', async () => { + // `ConsoleLog.timestamp` is required (`shared/src/types.ts:210`), so 0 is a + // captured time like any other and the first captured log always reads + // `0.0s` — whatever that log's own timestamp is. Reading the cell as an + // element rather than through `text()` is what separates "blank cell" from + // "no cell at all": `text(null)` is `''` too, so an assertion on text + // alone passes for a row that rendered no time element whatsoever. + it('times an entry captured at timestamp 0 like any other entry', async () => { const panel = await mountConsole([consoleLog({ timestamp: 0 })]) const nonZero = await mountConsole([consoleLog({ timestamp: RUN_START })]) const cells = shadowAll(panel, TIME) expect(cells).toHaveLength(1) - expect(text(cells[0])).toBe('') - // The same single entry at any other timestamp does get the 0.0s cell. + expect(text(cells[0])).toBe('0.0s') + // The same single entry at any other timestamp reads the same. expect(texts(nonZero, TIME)).toEqual(['0.0s']) }) + + it('measures later entries from a first entry captured at timestamp 0', async () => { + const panel = await mountConsole([ + consoleLog({ timestamp: 0 }), + consoleLog({ timestamp: 1500 }) + ]) + + expect(shadowAll(panel, TIME)).toHaveLength(2) + expect(texts(panel, TIME)).toEqual(['0.0s', '1.5s']) + }) + + it('gives two entries captured at the same moment the same time', async () => { + const panel = await mountConsole([ + consoleLog({ timestamp: RUN_START }), + consoleLog({ timestamp: RUN_START + 400 }), + consoleLog({ timestamp: RUN_START + 400 }) + ]) + + expect(texts(panel, TIME)).toEqual(['0.0s', '0.4s', '0.4s']) + }) + + // Browser and terminal logs are stamped by different clocks, so a later + // entry can carry a smaller timestamp than the first captured one. The + // column reads seconds into the run, so it floors at the origin instead of + // counting backwards — and a sub-100ms skew must not render as `-0.0s`. + it('floors an entry stamped before the first captured log at 0.0s', async () => { + const panel = await mountConsole([ + consoleLog({ timestamp: RUN_START }), + consoleLog({ timestamp: RUN_START - 40, source: 'terminal' }), + consoleLog({ timestamp: RUN_START - 900, source: 'terminal' }), + consoleLog({ timestamp: RUN_START + 600 }) + ]) + + expect(texts(panel, TIME)).toEqual(['0.0s', '0.0s', '0.0s', '0.6s']) + expect(texts(panel, TIME).filter((cell) => cell.includes('-'))).toEqual( + [] + ) + }) + + // The panel outlives a run: the logs context is replaced when the next one + // starts. Elapsed time is measured from whichever logs it currently holds, + // so a remembered origin from the previous run can't leak into this one. + it('re-bases elapsed time when a new run replaces the captured logs', async () => { + const panel = await mountConsole([ + consoleLog({ timestamp: RUN_START }), + consoleLog({ timestamp: RUN_START + 400 }) + ]) + expect(texts(panel, TIME)).toEqual(['0.0s', '0.4s']) + + // What @lit/context's consume callback does on a new value; `logs` is not + // a reactive property, so the re-render has to be asked for explicitly. + panel.logs = [ + consoleLog({ timestamp: RUN_START + 60_000 }), + consoleLog({ timestamp: RUN_START + 61_200 }) + ] + panel.requestUpdate() + await settle(panel) + + expect(texts(panel, TIME)).toEqual(['0.0s', '1.2s']) + }) }) describe('filtering', () => { From c718b52d276b785b8f42714b1b05e251a2dd2206 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 03:01:04 +0530 Subject: [PATCH 04/14] fix(app): judge a run-all against canRunAll, and say why it is refused --- .../app/src/components/sidebar/explorer.ts | 32 ++++---- .../components/sidebar/runnerCapabilities.ts | 41 ++++++++-- .../test-ui/sidebar/explorer/explorer.test.ts | 82 ++++++++++++++++--- 3 files changed, 124 insertions(+), 31 deletions(-) diff --git a/packages/app/src/components/sidebar/explorer.ts b/packages/app/src/components/sidebar/explorer.ts index f23d1c5a..31fbbd8b 100644 --- a/packages/app/src/components/sidebar/explorer.ts +++ b/packages/app/src/components/sidebar/explorer.ts @@ -19,10 +19,12 @@ import { getFramework, getLaunchCommand, getRerunCommand, - getRunCapabilities, + getRunAllDisabledReason, getRunDisabledReason, + isRunAll, isRunDisabled, - isRunDisabledDetail + isRunDisabledDetail, + RUN_ALL_UID } from './runnerCapabilities.js' import { BASELINE_API, @@ -180,7 +182,7 @@ export class DevtoolsSidebarExplorer extends CollapseableEntry { // Forward preserveBaseline so the backend knows whether to drop baselines. const payload: RunnerRequestBody = { ...detail, - runAll: detail.uid === '*', + runAll: isRunAll(detail), framework: this.#getFramework(), specFile: detail.specFile || this.#deriveSpecFile(detail), configFile: this.#getConfigPath(), @@ -293,26 +295,25 @@ export class DevtoolsSidebarExplorer extends CollapseableEntry { } #runAllSuites() { - if (!this.#getRunCapabilities().canRunSuites) { - this.#surfaceCapabilityWarning({ - entryType: 'suite', - uid: '*' - } as TestRunDetail) + // Judged against the same capability the control is rendered from, so a + // dispatch that never went through the disabled button is refused too. + const detail: TestRunDetail = { uid: RUN_ALL_UID, entryType: 'suite' } + if (this.#isRunDisabledDetail(detail)) { + this.#surfaceCapabilityWarning(detail) return } // Clear execution data and mark all tests as running this.dispatchEvent( new CustomEvent('clear-execution-data', { - detail: { uid: '*', entryType: 'suite' }, + detail, bubbles: true, composed: true }) ) const payload: RunnerRequestBody = { - uid: '*', - entryType: 'suite', + ...detail, runAll: true, framework: this.#getFramework(), configFile: this.#getConfigPath(), @@ -331,8 +332,8 @@ export class DevtoolsSidebarExplorer extends CollapseableEntry { #getFramework() { return getFramework(this.metadata) } - #getRunCapabilities() { - return getRunCapabilities(this.metadata) + #getRunAllDisabledReason() { + return getRunAllDisabledReason(this.metadata) } #isRunDisabled(entry: TestEntry) { return isRunDisabled(this.metadata, entry) @@ -403,7 +404,8 @@ export class DevtoolsSidebarExplorer extends CollapseableEntry { } #renderHeaderToolbar() { - const canRunAll = this.#getRunCapabilities().canRunAll + const runAllRefusal = this.#getRunAllDisabledReason() + const canRunAll = !runAllRefusal const runBtnCls = canRunAll ? 'hover:bg-toolbarHoverBackground' : 'opacity-30 cursor-not-allowed' @@ -412,7 +414,7 @@ export class DevtoolsSidebarExplorer extends CollapseableEntry {
diff --git a/packages/app/test-ui/workbench/panels/network.test.ts b/packages/app/test-ui/workbench/panels/network.test.ts index eedf2001..766805c6 100644 --- a/packages/app/test-ui/workbench/panels/network.test.ts +++ b/packages/app/test-ui/workbench/panels/network.test.ts @@ -8,6 +8,7 @@ import { getFileName, statusKind } from '@/utils/network-helpers.js' +import { FAILED_STATUS_LABEL } from '@/utils/network-constants.js' import '@components/workbench/network.js' import type { DevtoolsNetwork } from '@components/workbench/network.js' import { @@ -70,9 +71,7 @@ const expectedTypes = (requests: NetworkRequest[]) => requests.map(contentType) const expectedDurations = (requests: NetworkRequest[]) => requests.map((request) => - typeof request.time === 'number' && request.time > 0 - ? formatTime(request.time) - : NO_VALUE + typeof request.time === 'number' ? formatTime(request.time) : NO_VALUE ) const expectedSizes = (requests: NetworkRequest[]) => @@ -98,6 +97,20 @@ const expectedBarWidths = (inView: NetworkRequest[]) => { .map((request) => `${waterfallBar(request, range).width}%`) } +/** A 204: an answered request that transferred nothing and took no measurable + * time. Not in `fixtures.ts` because every fixture request there carries a + * transferred body, and 0 is exactly the value these columns used to drop. */ +const emptyBody = networkRequest({ + id: 'req-204', + url: 'https://the-internet.herokuapp.com/api/session', + method: 'DELETE', + type: 'fetch', + status: 204, + statusText: 'No Content', + time: 0, + size: 0 +}) + async function mountNetwork( requests: NetworkRequest[] ): Promise { @@ -266,6 +279,32 @@ describe('wdio-devtools-network', () => { ]) }) + // A 204 transfers 0 bytes and a cached or same-tick response measures 0 ms; + // both are measurements the producers send (the page collector's + // `#estimateSize` returns 0 for a body it could not read, nightwatch's + // perf-log parser clamps a duration at 0). Both columns used to render them + // as if nothing had been captured, which the in-flight row beside them — + // where nothing really was — is here to keep apart. + it('reports a zero duration and a zero size as measured values', async () => { + const panel = await mountNetwork([emptyBody, loginNetwork.pending]) + + expect(texts(panel, DURATION)).toEqual(['0.00ms', NO_VALUE]) + expect(texts(panel, SIZE)).toEqual(['0B', '-']) + }) + + it('draws no bar for a zero duration without dashing its cell', async () => { + const panel = await mountNetwork([emptyBody, loginNetwork.pending]) + + // A zero-width bar would read as a stray sliver, so the track stays empty + // for both rows — the difference is what the duration cell reports. + expect(shadowAll(panel, BAR)).toHaveLength(0) + expect( + shadowAll(panel, DURATION).map((cell) => + cell.classList.contains('req-dur-empty') + ) + ).toEqual([false, true]) + }) + // Literals only, deliberately: routing the expectation through the same // helper the panel calls made this list agree with itself while it // contradicted the Type column above — a document row dotted `type-other`. @@ -324,6 +363,45 @@ describe('wdio-devtools-network', () => { expect(text(shadow(panel, STATUS))).toBe('—') expect(kindClassOf(shadowAll(panel, STATUS)[0])).toBe('kind-pending') }) + + // A transport failure reaches this panel as status 0 carrying the failure + // text, with no `error` field — that is what the WDIO service's + // `handleNetworkFetchError` sends on `network.fetchError`. Read on + // truthiness, the row claimed the request had no status yet: dashed and + // coloured pending, indistinguishable from the in-flight row beside it. + it('renders a request whose status is 0 as failed, not as still pending', async () => { + const failed = networkRequest({ + id: 'req-dns', + url: 'https://the-internet.herokuapp.com/api/absent', + type: 'fetch', + status: 0, + statusText: 'net::ERR_NAME_NOT_RESOLVED', + time: 30 + }) + const panel = await mountNetwork([failed, loginNetwork.pending]) + + expect(texts(panel, STATUS)).toEqual(['ERR', '—']) + expect(shadowAll(panel, STATUS).map(kindClassOf)).toEqual([ + 'kind-error', + 'kind-pending' + ]) + }) + + it('summarises a status of 0 in the detail panel as the failure it was', async () => { + const failed = networkRequest({ + id: 'req-dns', + status: 0, + statusText: 'net::ERR_NAME_NOT_RESOLVED' + }) + const panel = await mountNetwork([failed]) + await clickRow(panel, 0) + + const [general] = detailSections(panel) + expect(general.values[2]).toBe('ERR net::ERR_NAME_NOT_RESOLVED') + expect(texts(panel, ERROR_VALUE)).toEqual([ + 'ERR net::ERR_NAME_NOT_RESOLVED' + ]) + }) }) describe('waterfall', () => { @@ -501,6 +579,44 @@ describe('wdio-devtools-network', () => { ]) }) + // The user-visible half of the same defect: expanding a 204 showed a General + // card with no Time and no Size row at all, so a request that answered with + // an empty body read as one whose timing and size were never captured. + it('reports the zero timing and zero size of a 204 it expands', async () => { + const panel = await mountNetwork([emptyBody]) + await clickRow(panel, 0) + + const [general] = detailSections(panel) + expect(general.keys).toEqual([ + 'Request URL', + 'Method', + 'Status', + 'Type', + 'Time', + 'Size' + ]) + expect(general.values).toEqual([ + emptyBody.url, + 'DELETE', + '204 No Content', + contentType(emptyBody), + '0.00ms', + '0B' + ]) + }) + + it('leaves out the timing and size rows a request never carried', async () => { + const panel = await mountNetwork([ + networkRequest({ id: 'req-untimed', time: undefined, size: undefined }) + ]) + await clickRow(panel, 0) + + // Read as the absence of the rows, not as empty cells: `texts()` answers + // '' for both, which is how the dropped 204 rows above went unnoticed. + expect(detailSections(panel)[0].keys).not.toContain('Time') + expect(detailSections(panel)[0].keys).not.toContain('Size') + }) + it('reports the transport error of a request that never got a status', async () => { const panel = await mountNetwork([loginNetwork.failedFont]) await clickRow(panel, 0) @@ -514,9 +630,10 @@ describe('wdio-devtools-network', () => { 'Time', 'Error' ]) - // The missing status and the error message are both flagged as errors. + // A request that reported an error before any status reads ERR in the card + // just as it does in the list column, never the dash meaning "still going". expect(texts(panel, ERROR_VALUE)).toEqual([ - '—', + FAILED_STATUS_LABEL, 'net::ERR_CONNECTION_REFUSED' ]) }) diff --git a/packages/app/tests/request-detail.test.ts b/packages/app/tests/request-detail.test.ts index 5b922b6c..7a7d32b4 100644 --- a/packages/app/tests/request-detail.test.ts +++ b/packages/app/tests/request-detail.test.ts @@ -15,6 +15,7 @@ import { render } from 'lit' import type { NetworkRequest } from '@wdio/devtools-shared' import { renderNetworkRequestDetail } from '../src/components/workbench/network/request-detail.js' +import { FAILED_STATUS_LABEL } from '../src/utils/network-constants.js' import { contentType, formatBytes, @@ -79,6 +80,22 @@ const sectionNamed = (root: Element, title: string): Section => { const sectionTitles = (root: Element) => sections(root).map((section) => section.title) +/** The value text of the General row labelled `key`, or `null` when the row was + * not rendered at all. Reading the row as an element is what separates "the row + * is missing" from "the row is there and reads empty": both answer `''` to a + * text query, which is how a dropped row hides. */ +const generalValue = (root: Element, key: string): string | null => { + const row = [...root.querySelectorAll('.kv')].find( + (kv) => (kv.querySelector('.k')?.textContent ?? '').trim() === key + ) + if (!row) { + return null + } + return (row.querySelector('.v')?.textContent ?? '') + .replace(/\s+/g, ' ') + .trim() +} + /** Class of the Status value cell — the renderer stamps `kind-`. */ const kindClassOf = (root: Element, index = 0): string | undefined => [...root.querySelectorAll('.v')[index].classList].find((name) => @@ -209,18 +226,80 @@ describe('renderNetworkRequestDetail', () => { expect(sectionNamed(root, 'General').values[3]).toBe('-') }) - it('leaves out a zero timing rather than rendering it as 0ms', () => { - // `req.time ? …` is falsy for 0, so the row is dropped — the renderer - // never reaches `formatTime(0)`. - const root = detail(req({ status: 200, time: 0, size: 0 })) + // `time` and `size` are optional, but 0 is a value a producer measures and + // sends: a 204 transfers 0 bytes (the page collector's `#estimateSize` + // returns 0 for a body it could not read), and a same-tick or cached + // response is 0 ms (nightwatch's perf-log parser clamps its duration at 0). + // Both rows used to be guarded on truthiness, so the fact was rendered as if + // it had never been captured. + it('reports the zero timing and zero size of a 204 as measured values', () => { + const root = detail( + req({ + method: 'DELETE', + status: 204, + statusText: 'No Content', + time: 0, + size: 0 + }) + ) expect(sectionNamed(root, 'General').keys).toEqual([ 'Request URL', 'Method', 'Status', - 'Type' + 'Type', + 'Time', + 'Size' ]) - expect(formatTime(0)).toBe('0.00ms') + expect(generalValue(root, 'Time')).toBe(formatTime(0)) + expect(generalValue(root, 'Time')).toBe('0.00ms') + // Not `formatBytes(0)`: that is the same dash the helper gives a size that + // was never captured, so reusing it here would report the fact as unknown. + expect(generalValue(root, 'Size')).not.toBe(formatBytes(0)) + expect(generalValue(root, 'Size')).toBe('0B') + }) + + // Each row answers for its own field: a request timed at 0 whose size was + // never captured shows the timing and drops only the size. + it('reports a zero timing while still dropping an absent size', () => { + const root = detail(req({ status: 200, time: 0, size: undefined })) + + expect(generalValue(root, 'Time')).toBe('0.00ms') + expect(generalValue(root, 'Size')).toBeNull() + }) + + // The counterpart to the test above: absent stays absent. Asserted as a + // missing row rather than as empty text — a query that answers the same for + // both is what let the zero rows disappear unnoticed. + it('renders no timing or size row for a request that carries neither', () => { + const root = detail( + req({ status: 200, time: undefined, size: undefined }) + ) + + expect(generalValue(root, 'Time')).toBeNull() + expect(generalValue(root, 'Size')).toBeNull() + }) + + // `handleNetworkFetchError` in `service/src/session.ts` reports a transport + // failure as status 0 plus the failure text, and sets no `error` field, so a + // truthiness read files a request that demonstrably failed under "no status + // yet" — dashed and coloured as pending, like a request still in flight. + it('reads a status of 0 as a failure, not as a status that never arrived', () => { + const failed = detail( + req({ status: 0, statusText: 'net::ERR_NAME_NOT_RESOLVED' }) + ) + const failedStatus = generalValue(failed, 'Status') + + expect(failedStatus).toBe('ERR net::ERR_NAME_NOT_RESOLVED') + expect(kindClassOf(failed, 2)).toBe('kind-error') + + // The same cell for a request that genuinely has no status yet, so the two + // outcomes cannot both be satisfied by one rendering. + host = document.createElement('div') + const pending = detail(req({ status: undefined, statusText: undefined })) + expect(generalValue(pending, 'Status')).not.toBe(failedStatus) + expect(generalValue(pending, 'Status')).toBe('—') + expect(kindClassOf(pending, 2)).toBe('kind-pending') }) it('renders a sub-second timing in seconds', () => { @@ -250,9 +329,10 @@ describe('renderNetworkRequestDetail', () => { 'Error' ]) expect(general.values[5]).toBe('net::ERR_CONNECTION_REFUSED') - // The missing status and the message are both flagged as errors. + // A request that reported an error before any status reads ERR, the same + // as it does in the list column — never the dash that means "still going". expect(texts(root, '.v.kind-error')).toEqual([ - '—', + FAILED_STATUS_LABEL, 'net::ERR_CONNECTION_REFUSED' ]) }) From 0f433174161c8036e6b6fda877a371906dc76f94 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 16:04:58 +0530 Subject: [PATCH 09/14] fix(app): keep the compare toolbar on one line --- .../src/components/workbench/compare/styles.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/app/src/components/workbench/compare/styles.ts b/packages/app/src/components/workbench/compare/styles.ts index 6c4085c7..a6f28d19 100644 --- a/packages/app/src/components/workbench/compare/styles.ts +++ b/packages/app/src/components/workbench/compare/styles.ts @@ -16,12 +16,15 @@ export const compareStyles = css` } /* ── Toolbar ── */ + /* One row, always. Wrapping dropped the actions onto a second line at ordinary + panel widths, which read as two unrelated toolbars. The pills shrink and + ellipsize instead, and the controls keep their size so they stay clickable. */ .topbar { flex: 0 0 auto; display: flex; align-items: center; gap: 10px; - flex-wrap: wrap; + flex-wrap: nowrap; padding: 10px 14px; border-bottom: 1px solid var(--vscode-panel-border); } @@ -29,6 +32,12 @@ export const compareStyles = css` display: inline-flex; align-items: center; gap: 7px; + /* Shrinkable, so a long baseline label narrows rather than pushing the + controls out of the row. */ + min-width: 0; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; font-family: var(--vscode-editor-font-family, monospace); font-size: 11.5px; padding: 4px 11px; @@ -60,12 +69,16 @@ export const compareStyles = css` .scope { font-size: 11px; color: var(--vscode-editorLineNumber-foreground); + white-space: nowrap; } .actions-group { margin-left: auto; display: flex; align-items: center; gap: 8px; + /* Never shrink or wrap: these are the row's controls. */ + flex: 0 0 auto; + white-space: nowrap; } .toggle-label { display: inline-flex; From eb54e1dc3131773af79a6d9c91cb160349c8dec2 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 16:05:14 +0530 Subject: [PATCH 10/14] fix(script): put an attribute removal on the wire as a removal --- .../components/browser/boolean-attribute.ts | 43 ++++++++ .../app/src/components/browser/snapshot.ts | 41 +------ .../test-ui/workbench/player/snapshot.test.ts | 102 ++++++++++++++++-- packages/app/tests/boolean-attribute.test.ts | 98 +++++++++++++++++ packages/script/src/index.ts | 3 + packages/script/src/mutations.ts | 8 +- packages/script/tests/mutations.test.ts | 59 ++++++++++ 7 files changed, 308 insertions(+), 46 deletions(-) create mode 100644 packages/app/src/components/browser/boolean-attribute.ts create mode 100644 packages/app/tests/boolean-attribute.test.ts diff --git a/packages/app/src/components/browser/boolean-attribute.ts b/packages/app/src/components/browser/boolean-attribute.ts new file mode 100644 index 00000000..5b9ea491 --- /dev/null +++ b/packages/app/src/components/browser/boolean-attribute.ts @@ -0,0 +1,43 @@ +/** HTML boolean content attributes: presence alone is the state, so a captured + * "false" has to REMOVE one — written verbatim, `checked="false"` reads as + * checked. Curated rather than probed off the element, because a probe both + * misses and misfires: `readonly` has no same-named property (it is `readOnly`), + * while `draggable`, `spellcheck` and `translate` do carry boolean properties + * yet their attributes are enumerated, where "false" is a meaningful value. */ +const BOOLEAN_ATTRIBUTES = new Set([ + 'allowfullscreen', + 'autofocus', + 'autoplay', + 'checked', + 'controls', + 'default', + 'disabled', + 'formnovalidate', + 'inert', + 'ismap', + 'itemscope', + 'loop', + 'multiple', + 'muted', + 'novalidate', + 'open', + 'playsinline', + 'readonly', + 'required', + 'reversed', + 'selected' +]) + +/** Whether a captured attribute is one whose presence IS its state. `hidden` and + * every `aria-*` are absent for the same reason `draggable` is: their "false" + * is a real value to write, not an absence to replay. */ +export const isBooleanAttribute = (name: string) => + BOOLEAN_ATTRIBUTES.has(name.toLowerCase()) + +/** State a captured boolean attribute carries. The collector emits form-field + * state as `String(el.checked)` and every other record as the attribute's own + * value, so only a literal "false" — and a record carrying no value, which + * leaves no attribute state to set — means off: an empty value is a present + * attribute (`` reaches the wire as `''`). */ +export const booleanAttributeOn = (value?: string) => + value !== undefined && value.toLowerCase() !== 'false' diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index 45756360..dd961636 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -11,6 +11,7 @@ import { import { commandPageUrl } from './url-at-timestamp.js' import { mutationForCommand } from './mutation-at-command.js' import { imageMime } from './trace-timeline-utils.js' +import { booleanAttributeOn, isBooleanAttribute } from './boolean-attribute.js' import { type ComponentChildren, h, render, type VNode } from 'preact' import { customElement, query } from 'lit/decorators.js' @@ -52,44 +53,6 @@ const textChildren = (el: Node) => (node): node is Text => node.nodeType === Node.TEXT_NODE ) -/** HTML boolean content attributes: presence alone is the state, so a captured - * "false" has to REMOVE one — written verbatim, `checked="false"` reads as - * checked. Curated rather than probed off the element, because a probe both - * misses and misfires: `readonly` has no same-named property (it is `readOnly`), - * while `draggable`, `spellcheck` and `translate` do carry boolean properties - * yet their attributes are enumerated, where "false" is a meaningful value. */ -const BOOLEAN_ATTRIBUTES = new Set([ - 'allowfullscreen', - 'autofocus', - 'autoplay', - 'checked', - 'controls', - 'default', - 'disabled', - 'formnovalidate', - 'inert', - 'ismap', - 'itemscope', - 'loop', - 'multiple', - 'muted', - 'novalidate', - 'open', - 'playsinline', - 'readonly', - 'required', - 'reversed', - 'selected' -]) - -/** State a captured boolean attribute carries. The collector emits form-field - * state as `String(el.checked)` and every other record as the attribute's own - * value, so only a literal "false" — and a record carrying no value, which - * leaves no attribute state to set — means off: an empty value is a present - * attribute (`` reaches the wire as `''`). */ -const booleanAttributeOn = (value?: string) => - value !== undefined && value.toLowerCase() !== 'false' - declare global { interface WindowEventMap { 'screencast-ready': CustomEvent<{ @@ -499,7 +462,7 @@ export class DevtoolsBrowser extends Element { return } - if (BOOLEAN_ATTRIBUTES.has(name.toLowerCase())) { + if (isBooleanAttribute(name)) { this.#applyBooleanAttribute( el, name, diff --git a/packages/app/test-ui/workbench/player/snapshot.test.ts b/packages/app/test-ui/workbench/player/snapshot.test.ts index fa0bc410..68936f75 100644 --- a/packages/app/test-ui/workbench/player/snapshot.test.ts +++ b/packages/app/test-ui/workbench/player/snapshot.test.ts @@ -10,6 +10,16 @@ import { metadataContext, mutationContext } from '@/controller/context.js' +// The collector itself, by path: `packages/app` deliberately does not depend on +// `packages/script`, so there is no alias for it. Imported here — and only here — +// so the boolean-attribute cases below replay records the CAPTURE produced +// rather than ones this spec wrote out for it. +import { + MUTATION_OBSERVER_CONFIG, + serializeMutation, + shouldCapture +} from '../../../../script/src/mutations.js' +import { REF_ATTR } from '../../../../script/src/utils.js' import { mutationForCommand } from '@components/browser/mutation-at-command.js' import '@components/browser/snapshot.js' @@ -198,6 +208,46 @@ function reparse(el: HTMLElement, selector: string): HTMLInputElement { const boxesIn = (el: Browser, selector: string) => Array.from(replayDoc(el)?.querySelectorAll(selector) ?? []) +/** The timestamp `support/builders.ts` stamps, so a record serialized here lands + * in the same replay window as the built ones beside it. */ +const BUILDER_TIMESTAMP = mutation().timestamp + +/** + * One attribute mutation as the COLLECTOR puts it on the wire: `markup` is + * mutated under `packages/script`'s own observer config, serialized by its own + * serializer, then JSON round-tripped — which is where a field the capture left + * undefined becomes an absent one, the only difference between a removed + * attribute and one present with an empty value. + */ +function capturedAttributeMutation( + ref: string, + markup: string, + mutate: (el: HTMLInputElement) => void +): TraceMutation { + const host = document.createElement('div') + host.innerHTML = markup + const el = host.firstElementChild as HTMLInputElement + el.setAttribute(REF_ATTR, ref) + document.body.append(host) + const observer = new MutationObserver(() => {}) + try { + observer.observe(el, MUTATION_OBSERVER_CONFIG) + mutate(el) + // Synchronous, so the records are here without waiting on the microtask the + // observer would otherwise deliver them in. + const records = observer.takeRecords().filter(shouldCapture) + if (records.length !== 1) { + throw new Error(`Captured ${records.length} records, expected exactly 1`) + } + return JSON.parse( + JSON.stringify(serializeMutation(records[0], BUILDER_TIMESTAMP)) + ) as TraceMutation + } finally { + observer.disconnect() + host.remove() + } +} + describe('wdio-devtools-browser', () => { describe('document replay', () => { it('rebuilds the iframe document from the captured document anchor', async () => { @@ -367,12 +417,7 @@ describe('wdio-devtools-browser', () => { * username rides along as the signal that the window landed — the attribute * under test cannot be that signal, since it is what the assertions read. */ - async function replayAttribute( - target: string, - attributeName: string, - attributeValue?: string - ): Promise { - const entry = mutation({ target, attributeName, attributeValue }) + async function replayEntry(entry: TraceMutation): Promise { const el = await mountBrowser({ commands: loginTrace.commands, mutations: [loginTrace.loginDocument, loginTrace.usernameTyped, entry] @@ -386,6 +431,12 @@ describe('wdio-devtools-browser', () => { return doc } + const replayAttribute = ( + target: string, + attributeName: string, + attributeValue?: string + ) => replayEntry(mutation({ target, attributeName, attributeValue })) + it('drops a boolean attribute the capture recorded as false', async () => { const doc = await replayAttribute(REF.username, 'disabled', 'false') @@ -418,6 +469,45 @@ describe('wdio-devtools-browser', () => { expect(username.readOnly).toBe(true) expect(reparse(username, '#username').readOnly).toBe(true) }) + + /** + * The same two states, on records `packages/script` serialized rather than + * ones written out above — the join that makes this a round trip. A capture + * coercing a removed attribute to `''` (what `getAttribute() || ''` does) + * emits the shape `` emits, so the removal case below is the + * one assertion that fails for it; the empty-value case is its control and + * has to keep passing, since a capture omitting BOTH would also "fix" the + * removal while losing the presence signal. + */ + describe('as the collector serializes them', () => { + it('removes an attribute the captured page removed', async () => { + // The captured radio arrives with `checked`, so the removal is + // observable: a cleared attribute serialized as an empty one stays + // present and keeps reading as checked. + const doc = await replayEntry( + capturedAttributeMutation(REF.planGuest, '', (el) => + el.removeAttribute('checked') + ) + ) + + const guest = input(doc, '#plan-guest') + expect(guest.getAttribute('checked')).toBeNull() + expect(guest.checked).toBe(false) + expect(reparse(guest, '#plan-guest').checked).toBe(false) + }) + + it('keeps an attribute the captured page set to an empty value', async () => { + const doc = await replayEntry( + capturedAttributeMutation(REF.username, '', (el) => + el.setAttribute('readonly', '') + ) + ) + + const username = input(doc, '#username') + expect(username.readOnly).toBe(true) + expect(reparse(username, '#username').readOnly).toBe(true) + }) + }) }) describe('command selection', () => { diff --git a/packages/app/tests/boolean-attribute.test.ts b/packages/app/tests/boolean-attribute.test.ts new file mode 100644 index 00000000..382cb348 --- /dev/null +++ b/packages/app/tests/boolean-attribute.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest' + +import { + booleanAttributeOn, + isBooleanAttribute +} from '../src/components/browser/boolean-attribute.js' + +/** + * The two pure decisions behind replaying an attribute mutation: whether the + * attribute's PRESENCE is its state, and — for those that it is — whether the + * captured record says on or off. The names below are written out literally + * rather than read back off the set: an expectation derived from the value under + * test passes whatever that value is. + */ +describe('booleanAttributeOn', () => { + it('reads the literal string "false" as off', () => { + // The collector emits form-field state as `String(el.checked)`, so this is + // the shape a cleared checkbox arrives in. + expect(booleanAttributeOn('false')).toBe(false) + }) + + it('reads a missing value as off', () => { + // Nothing to set: a record carrying no value leaves no attribute state. + expect(booleanAttributeOn(undefined)).toBe(false) + expect(booleanAttributeOn()).toBe(false) + }) + + it('reads an empty value as ON, because empty means present', () => { + // The real MutationObserver record sends `getAttribute() || ''`, so a bare + // `` reaches the wire with an empty value. Reading the + // string for truthiness would drop the attribute the page actually had. + expect(booleanAttributeOn('')).toBe(true) + }) + + it('reads the literal string "true" as on', () => { + expect(booleanAttributeOn('true')).toBe(true) + }) + + it('reads any other value as on', () => { + // Presence is the state, so the value is not a boolean to parse: + // `checked="checked"` and `disabled="disabled"` are the common spellings. + expect(booleanAttributeOn('checked')).toBe(true) + expect(booleanAttributeOn('0')).toBe(true) + }) + + it('ignores the case a producer spelled "false" in', () => { + expect(booleanAttributeOn('False')).toBe(false) + expect(booleanAttributeOn('FALSE')).toBe(false) + }) +}) + +describe('isBooleanAttribute', () => { + it('claims an attribute whose presence is its state', () => { + expect(isBooleanAttribute('disabled')).toBe(true) + expect(isBooleanAttribute('checked')).toBe(true) + }) + + it('claims an attribute regardless of the case it was captured in', () => { + expect(isBooleanAttribute('DISABLED')).toBe(true) + expect(isBooleanAttribute('Checked')).toBe(true) + }) + + it('claims `readonly`, which a property probe off the element would miss', () => { + // The motivation for curating the set instead of probing the element: the + // attribute is `readonly`, the property is `readOnly`, so a lookup keyed on + // the captured attribute name finds nothing and the attribute would be + // written verbatim — `readonly="false"` makes the field read-only. + expect(isBooleanAttribute('readonly')).toBe(true) + // Keyed on the attribute spelling, matched case-insensitively, so the + // property's spelling resolves to the same entry rather than a second one. + expect(isBooleanAttribute('readOnly')).toBe(true) + }) + + it('disclaims `hidden`, an enumerated attribute whose "false" is a value', () => { + expect(isBooleanAttribute('hidden')).toBe(false) + }) + + it('disclaims `aria-*` state, where "false" is the state and not an absence', () => { + expect(isBooleanAttribute('aria-checked')).toBe(false) + expect(isBooleanAttribute('aria-disabled')).toBe(false) + expect(isBooleanAttribute('aria-hidden')).toBe(false) + }) + + it('disclaims the enumerated attributes that do carry boolean properties', () => { + // `draggable`, `spellcheck` and `translate` are why the set cannot be built + // by probing for a same-named boolean property: they have one, yet their + // attributes are enumerated and "false" is meaningful — deleting it would + // replay a draggable element as the default the page overrode. + expect(isBooleanAttribute('draggable')).toBe(false) + expect(isBooleanAttribute('spellcheck')).toBe(false) + expect(isBooleanAttribute('translate')).toBe(false) + }) + + it('disclaims a plain value attribute', () => { + expect(isBooleanAttribute('value')).toBe(false) + expect(isBooleanAttribute('class')).toBe(false) + }) +}) diff --git a/packages/script/src/index.ts b/packages/script/src/index.ts index 10cf7016..09a8926a 100644 --- a/packages/script/src/index.ts +++ b/packages/script/src/index.ts @@ -53,6 +53,9 @@ try { type: 'attributes', target: ref, attributeName: checkable ? 'checked' : 'value', + // `String` never yields the ambiguous shape a boolean attribute reader + // has to guess at: `checked` is always an explicit "true"/"false", and + // `value` is not a boolean attribute, so its `''` is a real empty value. attributeValue: checkable ? String(el.checked) : String(el.value), addedNodes: [], removedNodes: [], diff --git a/packages/script/src/mutations.ts b/packages/script/src/mutations.ts index 4b2d8012..e8a1c941 100644 --- a/packages/script/src/mutations.ts +++ b/packages/script/src/mutations.ts @@ -85,7 +85,13 @@ export function serializeMutation( const nextSibling = m.nextSibling ? getRef(m.nextSibling) : null let attributeValue: string | undefined if (m.type === 'attributes') { - attributeValue = (m.target as Element).getAttribute(m.attributeName!) || '' + // A REMOVED attribute reads back as `null`, and the replay takes a record + // carrying no value as the removal — coerced to `''` it instead says the + // attribute is still there with an empty value, which is exactly what + // `` puts on the wire, so a boolean attribute the page just + // cleared replays as still set. + attributeValue = + (m.target as Element).getAttribute(m.attributeName!) ?? undefined } let newTextContent: string | undefined if (m.type === 'characterData') { diff --git a/packages/script/tests/mutations.test.ts b/packages/script/tests/mutations.test.ts index 2ae25b8c..cbcb0988 100644 --- a/packages/script/tests/mutations.test.ts +++ b/packages/script/tests/mutations.test.ts @@ -111,6 +111,65 @@ describe('mutation serialization', () => { expect(mutations[0].childIndex).toBeUndefined() }) + /** + * A removed attribute and one present with an empty value are different page + * states that both read back as falsy, and only the wire tells the replay + * which happened: a boolean attribute's PRESENCE is its state, so `''` means + * set (`` serializes to exactly that) and the removal has to + * arrive carrying no value at all. Asserted on the JSON the trace actually + * writes, since that is where an `undefined` field becomes an absent one. + */ + describe('a removed attribute versus one present with an empty value', () => { + const onWire = (m: TraceMutation) => + JSON.parse(JSON.stringify(m)) as Record + + it('carries no value for an attribute the page removed', async () => { + document.body.innerHTML = '' + assignRef(document.body) + const field = document.querySelector('#field')! + + const mutations = await capture(() => { + field.removeAttribute('disabled') + }) + + expect(mutations).toHaveLength(1) + expect(mutations[0].attributeName).toBe('disabled') + // Coerced to `''` this record says the field is still disabled. + expect(mutations[0].attributeValue).toBeUndefined() + expect('attributeValue' in onWire(mutations[0])).toBe(false) + }) + + it('carries the empty value of an attribute the page set to it', async () => { + document.body.innerHTML = '' + assignRef(document.body) + const field = document.querySelector('#field')! + + const mutations = await capture(() => { + field.setAttribute('readonly', '') + }) + + expect(mutations).toHaveLength(1) + expect(mutations[0].attributeValue).toBe('') + // Present and empty — indistinguishable from the removal above unless the + // field survives the trip as its own key. + expect(onWire(mutations[0]).attributeValue).toBe('') + }) + + it('keeps carrying the empty value of an emptied non-boolean attribute', async () => { + // The case the removal signal must not swallow: `class=""` is a real value + // the replay writes, and it reaches the wire the same way `disabled` does. + document.body.innerHTML = '
' + assignRef(document.body) + const flash = document.querySelector('#flash')! + + const mutations = await capture(() => { + flash.setAttribute('class', '') + }) + + expect(mutations[0].attributeValue).toBe('') + }) + }) + it('never reports the ref attribute it stamps itself', async () => { document.body.innerHTML = '
old
' assignRef(document.body) From 3d45ec9286c9c6984fcfc27ae4f4c12a1123f9c6 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 16:05:30 +0530 Subject: [PATCH 11/14] refactor: give RUN_ALL_UID and the transcript builder one home each --- .../app/src/components/sidebar/constants.ts | 13 ++ .../app/src/components/sidebar/explorer.ts | 4 +- .../components/sidebar/runnerCapabilities.ts | 21 +- .../components/workbench/console-filter.ts | 5 +- .../app/src/components/workbench/source.ts | 1 - packages/app/src/controller/DataManager.ts | 11 +- .../test-ui/workbench/panels/console.test.ts | 25 +++ .../test-ui/workbench/panels/source.test.ts | 4 - packages/app/tests/console-filter.test.ts | 28 ++- packages/app/tests/data-manager.test.ts | 25 +++ packages/app/tests/runnerCapabilities.test.ts | 61 ++++- packages/core/src/trace-exporter.ts | 67 +----- packages/core/src/trace-transcript.ts | 95 ++++++++ packages/core/tests/trace-exporter.test.ts | 19 -- packages/core/tests/trace-transcript.test.ts | 208 ++++++++++++++++++ 15 files changed, 475 insertions(+), 112 deletions(-) create mode 100644 packages/core/src/trace-transcript.ts create mode 100644 packages/core/tests/trace-transcript.test.ts diff --git a/packages/app/src/components/sidebar/constants.ts b/packages/app/src/components/sidebar/constants.ts index a346b352..46d4f5da 100644 --- a/packages/app/src/components/sidebar/constants.ts +++ b/packages/app/src/components/sidebar/constants.ts @@ -1,5 +1,18 @@ import type { RunCapabilities } from './types.js' +/** The uid the header control names the whole tree with. `RunnerRequestBody`'s + * `runAll` flag is derived from it, so it is also the signal that a refusal + * has to be judged against `canRunAll` — not against the `canRunSuites` its + * `entryType` would otherwise select. */ +export const RUN_ALL_UID = '*' + +export const SINGLE_TEST_REFUSAL = + 'Single-test execution is not supported by this framework.' +export const SUITE_REFUSAL = + 'Suite execution is not supported by this framework.' +export const RUN_ALL_REFUSAL = + 'Running every test at once is not supported by this framework.' + export const DEFAULT_CAPABILITIES: RunCapabilities = { canRunSuites: true, canRunTests: true, diff --git a/packages/app/src/components/sidebar/explorer.ts b/packages/app/src/components/sidebar/explorer.ts index 31fbbd8b..9f356dde 100644 --- a/packages/app/src/components/sidebar/explorer.ts +++ b/packages/app/src/components/sidebar/explorer.ts @@ -23,9 +23,9 @@ import { getRunDisabledReason, isRunAll, isRunDisabled, - isRunDisabledDetail, - RUN_ALL_UID + isRunDisabledDetail } from './runnerCapabilities.js' +import { RUN_ALL_UID } from './constants.js' import { BASELINE_API, TESTS_API, diff --git a/packages/app/src/components/sidebar/runnerCapabilities.ts b/packages/app/src/components/sidebar/runnerCapabilities.ts index ff9018f9..979aed58 100644 --- a/packages/app/src/components/sidebar/runnerCapabilities.ts +++ b/packages/app/src/components/sidebar/runnerCapabilities.ts @@ -17,19 +17,14 @@ import type { TestEntry, TestRunDetail } from './types.js' -import { DEFAULT_CAPABILITIES, FRAMEWORK_CAPABILITIES } from './constants.js' - -/** The uid the header control names the whole tree with. `RunnerRequestBody`'s - * `runAll` flag is derived from it, so it is also the signal that a refusal - * has to be judged against `canRunAll` — not against the `canRunSuites` its - * `entryType` would otherwise select. */ -export const RUN_ALL_UID = '*' - -const SINGLE_TEST_REFUSAL = - 'Single-test execution is not supported by this framework.' -const SUITE_REFUSAL = 'Suite execution is not supported by this framework.' -const RUN_ALL_REFUSAL = - 'Running every test at once is not supported by this framework.' +import { + DEFAULT_CAPABILITIES, + FRAMEWORK_CAPABILITIES, + RUN_ALL_REFUSAL, + RUN_ALL_UID, + SINGLE_TEST_REFUSAL, + SUITE_REFUSAL +} from './constants.js' export function isRunAll(detail: Pick): boolean { return detail.uid === RUN_ALL_UID diff --git a/packages/app/src/components/workbench/console-filter.ts b/packages/app/src/components/workbench/console-filter.ts index 2787ea85..438137c0 100644 --- a/packages/app/src/components/workbench/console-filter.ts +++ b/packages/app/src/components/workbench/console-filter.ts @@ -63,7 +63,10 @@ export function filterConsoleLogs( ): ConsoleLog[] { const needle = search.trim().toLowerCase() return logs.filter((log) => { - if (level !== 'all' && (log.type || 'log') !== level) { + // `ConsoleLog.type` is required, and the panel tags each row with it + // unguarded — a default here would file an entry under a level its own row + // does not claim. + if (level !== 'all' && log.type !== level) { return false } if (needle && !formatConsoleArgs(log.args).toLowerCase().includes(needle)) { diff --git a/packages/app/src/components/workbench/source.ts b/packages/app/src/components/workbench/source.ts index e76295ab..322c9c58 100644 --- a/packages/app/src/components/workbench/source.ts +++ b/packages/app/src/components/workbench/source.ts @@ -367,7 +367,6 @@ export class DevtoolsSource extends Element { return html`` } const hasContent = this.#contentFor(active) !== undefined diff --git a/packages/app/src/controller/DataManager.ts b/packages/app/src/controller/DataManager.ts index a199303c..42a13da2 100644 --- a/packages/app/src/controller/DataManager.ts +++ b/packages/app/src/controller/DataManager.ts @@ -29,6 +29,7 @@ import { } from './context.js' import { BASELINE_WS_SCOPE, TRACE_API, WS_SCOPE } from '@wdio/devtools-shared' import { CACHE_ID } from './constants.js' +import { RUN_ALL_UID } from '../components/sidebar/constants.js' import { rerunState } from './rerunState.js' import type { SuiteStatsFragment, SocketMessage } from './types.js' import { canonicalizeUids, mergeSuite } from './suite-merge.js' @@ -163,21 +164,21 @@ export class DataManagerController implements ReactiveController { // of the previous run's terminal state (passed/failed). if (!uid) { rerunState.activeRerunSuiteUid = undefined - this.#markTestAsRunning('*', 'suite') + this.#markTestAsRunning(RUN_ALL_UID, 'suite') return } // Track the top-level rerun suite uid so we can identify child-scenario // clears (from the Nightwatch backend) and skip their data wipes. - if (!isChildOfActiveRerun && entryType === 'suite' && uid !== '*') { + if (!isChildOfActiveRerun && entryType === 'suite' && uid !== RUN_ALL_UID) { rerunState.activeRerunSuiteUid = uid } // Track explicit single-test reruns so merge logic can keep sibling tests // stable while the backend emits suite-level "pending" snapshots. - if (entryType === 'test' && uid !== '*') { + if (entryType === 'test' && uid !== RUN_ALL_UID) { this.#activeRerunTestUid = uid - } else if (entryType === 'suite' || uid === '*') { + } else if (entryType === 'suite' || uid === RUN_ALL_UID) { this.#activeRerunTestUid = undefined } @@ -189,7 +190,7 @@ export class DataManagerController implements ReactiveController { #markTestAsRunning(uid: string, entryType?: 'suite' | 'test') { const suites = this.suitesContextProvider.value || [] const updated = - uid === '*' + uid === RUN_ALL_UID ? markAllRunning(suites) : markSpecificRunning(suites, uid, entryType) this.suitesContextProvider.setValue(updated) diff --git a/packages/app/test-ui/workbench/panels/console.test.ts b/packages/app/test-ui/workbench/panels/console.test.ts index 05e035ae..926ec1cc 100644 --- a/packages/app/test-ui/workbench/panels/console.test.ts +++ b/packages/app/test-ui/workbench/panels/console.test.ts @@ -302,6 +302,31 @@ describe('wdio-devtools-console-logs', () => { expect(texts(panel, MESSAGE)).toEqual(['plain log']) }) + // `ConsoleLog.type` is required (`shared/src/types.ts:208`), so an entry + // without one is wire data that broke the contract — and the row it renders + // says so, tagged with no level at all. The level tabs read the same field, + // so they have to agree: defaulting the filter to `log` put a row that does + // not claim to be a log under the Logs tab. + it('keeps the Logs tab in step with the level each row renders', async () => { + const untyped = { + args: ['no level'], + timestamp: RUN_START + } as unknown as ConsoleLog + const panel = await mountConsole([ + consoleLog({ args: ['plain log'] }), + untyped + ]) + + expect(shadowAll(panel, ENTRY).map(levelClassOf)).toEqual([ + 'log-type-log', + 'log-type-' + ]) + + await clickLevelTab(panel, 'Logs') + + expect(texts(panel, MESSAGE)).toEqual(['plain log']) + }) + it('narrows the list to messages containing the search text, ignoring case', async () => { const panel = await mountConsole(loginConsole.logs) await search(panel, 'SECURE') diff --git a/packages/app/test-ui/workbench/panels/source.test.ts b/packages/app/test-ui/workbench/panels/source.test.ts index 3c9d082e..22312b71 100644 --- a/packages/app/test-ui/workbench/panels/source.test.ts +++ b/packages/app/test-ui/workbench/panels/source.test.ts @@ -41,15 +41,12 @@ const NOT_CAPTURED = '.src-empty' const PLACEHOLDER = 'wdio-devtools-placeholder' const EMPTY_ICON = '.empty-state-icon' const EMPTY_HEADING = '.empty-state-text' -const EMPTY_DETAIL = '.empty-state-detail' const SKELETON = '.ph-item' /** Copy the panel hands its placeholder — a terminal state whenever the run * captured no source and no command reported a call site. */ const EMPTY_GLYPH = '📄' const EMPTY_HEADING_TEXT = 'No source to show' -const EMPTY_DETAIL_TEXT = - "A file appears here once the run captures a spec's source or a command reports the line it ran from — this run carries neither." /** Theme token the panel exposes as `--cs`, per `ActionCategory`. Mirrors * `source.ts`'s `CATEGORY_VAR`, which is module-private; `none` is the value @@ -432,7 +429,6 @@ describe('wdio-devtools-source', () => { const placeholder = shadow(panel, PLACEHOLDER)! expect(text(shadow(placeholder, EMPTY_HEADING))).toBe(EMPTY_HEADING_TEXT) - expect(text(shadow(placeholder, EMPTY_DETAIL))).toBe(EMPTY_DETAIL_TEXT) expect(text(shadow(placeholder, EMPTY_ICON))).toBe(EMPTY_GLYPH) }) diff --git a/packages/app/tests/console-filter.test.ts b/packages/app/tests/console-filter.test.ts index 25d0a340..50244f74 100644 --- a/packages/app/tests/console-filter.test.ts +++ b/packages/app/tests/console-filter.test.ts @@ -71,9 +71,33 @@ describe('filterConsoleLogs', () => { expect(errs[0].args).toEqual(['boom failed']) }) - it('treats a missing type as "log"', () => { + it('files every captured level under its own filter and no other', () => { + const levels: ConsoleLogs['type'][] = [ + 'trace', + 'debug', + 'log', + 'info', + 'warn', + 'error' + ] + const entries = levels.map((level) => log(level, [level])) + + for (const level of levels) { + expect(filterConsoleLogs(entries, level, '')).toEqual([ + log(level, [level]) + ]) + } + }) + + // `ConsoleLog.type` is required, so an entry without one is wire data that + // broke the contract — and the panel already tags such a row with the level it + // actually carries (`log-type-undefined`). Defaulting to `log` here handed the + // Logs tab a row that does not claim to be a log; the two now agree. + it('files an entry with no level under no level filter', () => { const untyped = [{ args: ['x'], timestamp: 0 } as unknown as ConsoleLogs] - expect(filterConsoleLogs(untyped, 'log', '')).toHaveLength(1) + + expect(filterConsoleLogs(untyped, 'log', '')).toEqual([]) + expect(filterConsoleLogs(untyped, 'all', '')).toHaveLength(1) }) it('matches search case-insensitively against the message', () => { diff --git a/packages/app/tests/data-manager.test.ts b/packages/app/tests/data-manager.test.ts index 8b179f2e..fe666825 100644 --- a/packages/app/tests/data-manager.test.ts +++ b/packages/app/tests/data-manager.test.ts @@ -22,6 +22,7 @@ import { type WsMessageScope } from '@wdio/devtools-shared' +import { RUN_ALL_UID } from '../src/components/sidebar/constants.js' import { CACHE_ID } from '../src/controller/constants.js' import { DataManagerController } from '../src/controller/DataManager.js' import { rerunState } from '../src/controller/rerunState.js' @@ -796,6 +797,30 @@ describe('DataManagerController', () => { ]) }) + it('marks the whole tree running for the run-all sentinel uid', async () => { + const { manager, deliver } = await boot() + deliver( + 'suites', + suitesFrame( + suite('login-suite', { state: 'passed', tests: [test('t-1')] }), + suite('checkout-suite', { state: 'failed' }) + ) + ) + + deliver(WS_SCOPE.clearExecutionData, { + uid: RUN_ALL_UID, + entryType: 'suite' + }) + + expect(publishedSuites(manager).map((entry) => entry.state)).toEqual([ + 'running', + 'running' + ]) + // The sentinel is not a suite uid — it must not be latched as the active + // rerun suite, or every child clear would be skipped as its descendant. + expect(rerunState.activeRerunSuiteUid).toBeUndefined() + }) + it('empties the tree when the backend asks for it', async () => { const { manager, deliver } = await boot() deliver('suites', suitesFrame(suite('login-suite'))) diff --git a/packages/app/tests/runnerCapabilities.test.ts b/packages/app/tests/runnerCapabilities.test.ts index 8fd525d0..c4aaedc2 100644 --- a/packages/app/tests/runnerCapabilities.test.ts +++ b/packages/app/tests/runnerCapabilities.test.ts @@ -6,11 +6,19 @@ import { getFramework, getLaunchCommand, getRerunCommand, + getRunAllDisabledReason, getRunCapabilities, getRunDisabledReason, + isRunAll, isRunDisabled, isRunDisabledDetail } from '../src/components/sidebar/runnerCapabilities.js' +import { + RUN_ALL_REFUSAL, + RUN_ALL_UID, + SINGLE_TEST_REFUSAL, + SUITE_REFUSAL +} from '../src/components/sidebar/constants.js' import type { TestEntry, TestRunDetail @@ -26,6 +34,11 @@ function entry(type: 'test' | 'suite'): TestEntry { function detail(entryType: 'test' | 'suite'): TestRunDetail { return { entryType, uid: 'u' } } +/** A run-all reaches the same helpers as a suite run — same `entryType`, only + * the uid differs. */ +function runAllDetail(): TestRunDetail { + return { entryType: 'suite', uid: RUN_ALL_UID } +} describe('getFramework', () => { it('reads options.framework', () => { @@ -63,6 +76,16 @@ describe('getRunCapabilities', () => { }) }) +describe('isRunAll', () => { + it('recognises the whole-tree sentinel uid', () => { + expect(isRunAll(runAllDetail())).toBe(true) + }) + it('is false for a normal entry uid', () => { + expect(isRunAll(detail('suite'))).toBe(false) + expect(isRunAll(detail('test'))).toBe(false) + }) +}) + describe('isRunDisabled / isRunDisabledDetail', () => { it('disables test runs when canRunTests is false', () => { const m = md({ runCapabilities: { canRunTests: false } }) @@ -77,6 +100,33 @@ describe('isRunDisabled / isRunDisabledDetail', () => { expect(isRunDisabledDetail(m, detail('suite'))).toBe(true) expect(isRunDisabled(m, entry('test'))).toBe(false) }) + + it('judges a run-all against canRunAll, not canRunSuites', () => { + const noRunAll = md({ + runCapabilities: { canRunAll: false, canRunSuites: true } + }) + expect(isRunDisabledDetail(noRunAll, runAllDetail())).toBe(true) + expect(isRunDisabledDetail(noRunAll, detail('suite'))).toBe(false) + + const suitesOnlyRefused = md({ + runCapabilities: { canRunAll: true, canRunSuites: false } + }) + expect(isRunDisabledDetail(suitesOnlyRefused, runAllDetail())).toBe(false) + expect(isRunDisabledDetail(suitesOnlyRefused, detail('suite'))).toBe(true) + }) +}) + +describe('getRunAllDisabledReason', () => { + it('undefined when the runner can run everything', () => { + expect( + getRunAllDisabledReason(md({ framework: 'cucumber' })) + ).toBeUndefined() + }) + it('names the run-all refusal when the runner cannot', () => { + expect(getRunAllDisabledReason(md({ framework: 'nightwatch' }))).toBe( + RUN_ALL_REFUSAL + ) + }) }) describe('getRunDisabledReason', () => { @@ -85,16 +135,19 @@ describe('getRunDisabledReason', () => { }) it('phrases reason per type', () => { const m = md({ runCapabilities: { canRunTests: false } }) - expect(getRunDisabledReason(m, entry('test'))).toContain('Single-test') + expect(getRunDisabledReason(m, entry('test'))).toBe(SINGLE_TEST_REFUSAL) const m2 = md({ runCapabilities: { canRunSuites: false } }) - expect(getRunDisabledReason(m2, entry('suite'))).toContain('Suite') + expect(getRunDisabledReason(m2, entry('suite'))).toBe(SUITE_REFUSAL) }) }) describe('getCapabilityWarning', () => { it('phrases warning per detail entryType', () => { - expect(getCapabilityWarning(detail('test'))).toContain('Single-test') - expect(getCapabilityWarning(detail('suite'))).toContain('Suite') + expect(getCapabilityWarning(detail('test'))).toBe(SINGLE_TEST_REFUSAL) + expect(getCapabilityWarning(detail('suite'))).toBe(SUITE_REFUSAL) + }) + it('phrases the run-all warning off the sentinel, not the entryType', () => { + expect(getCapabilityWarning(runAllDetail())).toBe(RUN_ALL_REFUSAL) }) }) diff --git a/packages/core/src/trace-exporter.ts b/packages/core/src/trace-exporter.ts index 93dce8bc..2fc1a5e9 100644 --- a/packages/core/src/trace-exporter.ts +++ b/packages/core/src/trace-exporter.ts @@ -16,12 +16,7 @@ import type { TraceLog, TraceMutation } from '@wdio/devtools-shared' -import { - formatActionTitle, - mapCommandToAction, - FILL_METHODS, - type TraceAction -} from './action-mapping.js' +import { mapCommandToAction } from './action-mapping.js' import { buildConsoleEvents, type ConsoleEvent, @@ -43,8 +38,13 @@ import { buildSourceResources } from './trace-sources.js' import { networkRequestToHar } from './trace-har.js' import { buildTraceZip, type TraceZipResource } from './trace-zip-writer.js' import { buildMutationsNdjson } from './trace-mutations.js' +import { generateTranscript } from './trace-transcript.js' import { sha1Hex } from './sha1.js' +// Transcript building moved to its own module; re-exported here because this is +// the name the package barrel and downstream adapters already import. +export { generateTranscript } + const TRACE_VERSION = 8 const LIBRARY_NAME = '@wdio/devtools-core' const LIBRARY_VERSION = '1.0.0' @@ -274,61 +274,6 @@ function compareEvents(a: TraceEvent, b: TraceEvent): number { return dt !== 0 ? dt : eventOrder(a) - eventOrder(b) } -/** - * Generate a human/LLM-readable Markdown transcript from captured commands. - */ -export function generateTranscript( - commands: CommandLog[], - startWallTime: number, - title?: string -): string { - const wallTimeISO = new Date(startWallTime).toISOString() - const lines: string[] = [`# ${title ?? 'Session'} — ${wallTimeISO}`, ''] - - // Sort by invocation time so batched commands land at their real timeline - // positions — Nightwatch buffers native asserts and emits them at test-end, - // so raw order clusters all asserts after the navigations. The Actions tree - // stays correct because buildActionEvents applies the same sort; mirror it - // here so the transcript matches execution order. Stable + a no-op for - // already-ordered WDIO/Selenium command logs. - const ordered = [...commands].sort( - (a, b) => (a.startTime ?? a.timestamp) - (b.startTime ?? b.timestamp) - ) - const captured: { entry: CommandLog; action: TraceAction }[] = [] - for (const c of ordered) { - const action = mapCommandToAction(String(c.command)) - if (action) { - captured.push({ entry: c, action }) - } - } - - captured.forEach(({ entry, action }, idx) => { - const label = formatActionTitle(action, entry.args as unknown[]) - - const rawArgs = entry.args as unknown[] - const parts: string[] = [`${idx + 1}. ${label}`] - - if (FILL_METHODS.has(action.method) && rawArgs) { - const valueIdx = rawArgs.length >= 2 ? 1 : 0 - if (rawArgs[valueIdx] !== undefined) { - parts.push(`value="${String(rawArgs[valueIdx])}"`) - } - } - - if (entry.error) { - const msg = - typeof entry.error === 'object' && 'message' in entry.error - ? (entry.error as { message: string }).message - : String(entry.error) - parts.push(`ERROR: ${msg}`) - } - - lines.push(parts.join(' ')) - }) - - return lines.join('\n') -} - interface TraceBundle { traceNdjson: string networkNdjson: Buffer diff --git a/packages/core/src/trace-transcript.ts b/packages/core/src/trace-transcript.ts new file mode 100644 index 00000000..370d97e1 --- /dev/null +++ b/packages/core/src/trace-transcript.ts @@ -0,0 +1,95 @@ +// Builds the trace's `transcript.md` — a Markdown step list read by humans and +// fed to an LLM. Runner-agnostic; the exporter writes whatever this returns. + +import type { CommandLog } from '@wdio/devtools-shared' +import { + formatActionTitle, + mapCommandToAction, + FILL_METHODS, + type TraceAction +} from './action-mapping.js' +import { stripAnsi } from './console.js' + +/** Render `text` as one numbered markdown list item, indenting every line after + * the first to the marker's content column (`'1. '` → 3, `'10. '` → 4 — an + * ordered list's continuation indent tracks the marker width, unlike a + * bullet's fixed two). A step interpolates captured strings that may hold + * newlines — a framework error routinely does (expect-webdriverio puts + * `Expected:` and `Received:` on their own lines) and a typed value can too — + * and an unindented tail leaves the list, reading as top-level prose + * unattributed to the step that produced it. */ +function asNumberedItem(marker: string, text: string): string { + const indent = ' '.repeat(marker.length) + const [head, ...tail] = text.split('\n') + return [ + `${marker}${head}`, + // A whitespace-only line stays empty: indenting it would only add trailing + // whitespace, and a blank line inside an indented item is still inside it. + ...tail.map((line) => (line.trim() ? `${indent}${line}` : '')) + ].join('\n') +} + +function errorMessage(error: NonNullable): string { + const raw = + typeof error === 'object' && error !== null && 'message' in error + ? String((error as { message: unknown }).message) + : String(error) + // Runner errors carry terminal colour (node's AssertionError diff is + // colour-coded), which is noise in a document read by a model. + return stripAnsi(raw).trim() +} + +/** Commands that map to a trace action, in invocation order. */ +function capturedSteps( + commands: CommandLog[] +): { entry: CommandLog; action: TraceAction }[] { + // Sort by invocation time so batched commands land at their real timeline + // positions — Nightwatch buffers native asserts and emits them at test-end, + // so raw order clusters all asserts after the navigations. The Actions tree + // stays correct because buildActionEvents applies the same sort; mirror it + // here so the transcript matches execution order. Stable + a no-op for + // already-ordered WDIO/Selenium command logs. + const ordered = [...commands].sort( + (a, b) => (a.startTime ?? a.timestamp) - (b.startTime ?? b.timestamp) + ) + const captured: { entry: CommandLog; action: TraceAction }[] = [] + for (const c of ordered) { + const action = mapCommandToAction(String(c.command)) + if (action) { + captured.push({ entry: c, action }) + } + } + return captured +} + +/** + * Generate a human/LLM-readable Markdown transcript from captured commands. + */ +export function generateTranscript( + commands: CommandLog[], + startWallTime: number, + title?: string +): string { + const wallTimeISO = new Date(startWallTime).toISOString() + const lines: string[] = [`# ${title ?? 'Session'} — ${wallTimeISO}`, ''] + + capturedSteps(commands).forEach(({ entry, action }, idx) => { + const rawArgs = entry.args as unknown[] + const parts: string[] = [stripAnsi(formatActionTitle(action, rawArgs))] + + if (FILL_METHODS.has(action.method) && rawArgs) { + const valueIdx = rawArgs.length >= 2 ? 1 : 0 + if (rawArgs[valueIdx] !== undefined) { + parts.push(`value="${stripAnsi(String(rawArgs[valueIdx]))}"`) + } + } + + if (entry.error) { + parts.push(`ERROR: ${errorMessage(entry.error)}`) + } + + lines.push(asNumberedItem(`${idx + 1}. `, parts.join(' '))) + }) + + return lines.join('\n') +} diff --git a/packages/core/tests/trace-exporter.test.ts b/packages/core/tests/trace-exporter.test.ts index 269ce3c6..0c4b2d1e 100644 --- a/packages/core/tests/trace-exporter.test.ts +++ b/packages/core/tests/trace-exporter.test.ts @@ -12,7 +12,6 @@ import { type TraceCapturer } from '@wdio/devtools-core' import { TraceType, type CommandLog } from '@wdio/devtools-shared' -import { generateTranscript } from '../src/trace-exporter.js' const isBefore = (event: ActionEvent): event is BeforeEvent => event.type === 'before' @@ -31,24 +30,6 @@ function cmd(command: string, overrides: Partial = {}): CommandLog { } } -describe('generateTranscript', () => { - it('orders commands by invocation time when captured out of order (Nightwatch batches asserts to test-end)', () => { - // Array order puts a later navigation before an earlier-timestamped click, - // mimicking Nightwatch buffering native asserts until test-end. - const commands = [ - cmd('url', { timestamp: 1100, startTime: 1050 }), - cmd('url', { timestamp: 1300, startTime: 1250 }), - cmd('click', { timestamp: 1200, startTime: 1150 }) - ] - const lines = generateTranscript(commands, 1000, 'Test') - .split('\n') - .filter((l) => /^\d+\./.test(l)) - expect(lines).toHaveLength(3) - // Sorted by startTime: url(1050) → click(1150) → url(1250) — click is #2. - expect(lines[1]).toMatch(/click/i) - }) -}) - describe('buildActionEvents', () => { const pageId = 'page@abc123' const wallTime = 1000 diff --git a/packages/core/tests/trace-transcript.test.ts b/packages/core/tests/trace-transcript.test.ts new file mode 100644 index 00000000..fbe1c6e4 --- /dev/null +++ b/packages/core/tests/trace-transcript.test.ts @@ -0,0 +1,208 @@ +import { describe, it, expect } from 'vitest' +import type { CommandLog } from '@wdio/devtools-shared' +import { generateTranscript } from '../src/trace-transcript.js' + +const ESC = '' +/** An expect-webdriverio failure: a headline, a blank line, then the coloured + * `Expected:`/`Received:` pair each on its own line. */ +const MULTILINE_ERROR = [ + 'Expect $(`#flash`) to have text', + '', + `Expected: "${ESC}[32mWelcome!${ESC}[39m"`, + `Received: "${ESC}[31mYour username is invalid!${ESC}[39m"` +].join('\n') + +function cmd(command: string, overrides: Partial = {}): CommandLog { + const base = (overrides.timestamp ?? 1000) + 100 + return { + command, + args: [], + timestamp: base, + startTime: overrides.startTime ?? base - 50, + ...overrides + } +} + +const transcript = (commands: CommandLog[]) => + generateTranscript(commands, 1000, 'Test').split('\n') + +const HEADING = '# Test — 1970-01-01T00:00:01.000Z' + +/** Every line must be a step marker, a heading, blank, or indented under its + * step — an unindented tail line has escaped its list item and reads as + * top-level prose no longer attributed to the step that produced it. */ +function unattributedLines(lines: string[]): string[] { + return lines.filter( + (line) => + line !== '' && + !line.startsWith('#') && + !/^\d+\. /.test(line) && + !/^ /.test(line) + ) +} + +describe('generateTranscript', () => { + it('orders commands by invocation time when captured out of order (Nightwatch batches asserts to test-end)', () => { + // Array order puts a later navigation before an earlier-timestamped click, + // mimicking Nightwatch buffering native asserts until test-end. + const lines = transcript([ + cmd('url', { timestamp: 1100, startTime: 1050 }), + cmd('url', { timestamp: 1300, startTime: 1250 }), + cmd('click', { timestamp: 1200, startTime: 1150 }) + ]) + // Sorted by startTime: url(1050) → click(1150) → url(1250) — click is #2. + expect(lines).toEqual([ + HEADING, + '', + '1. Page.navigate()', + '2. Element.click()', + '3. Page.navigate()' + ]) + }) + + it('indents a multi-line error under its numbered step and strips ANSI', () => { + const lines = transcript([ + cmd('click', { + timestamp: 1100, + startTime: 1050, + error: { name: 'Error', message: MULTILINE_ERROR } + }) + ]) + // Three spaces — the width of the `1. ` marker, which is what markdown + // needs to keep a continuation inside an ordered list item. + expect(lines).toEqual([ + HEADING, + '', + '1. Element.click() ERROR: Expect $(`#flash`) to have text', + '', + ' Expected: "Welcome!"', + ' Received: "Your username is invalid!"' + ]) + expect(unattributedLines(lines)).toEqual([]) + }) + + it('indents a multi-line typed value under its numbered step', () => { + const lines = transcript([ + cmd('setValue', { + timestamp: 1100, + startTime: 1050, + args: ['#comment', 'line one\nline two\nline three'] + }) + ]) + expect(lines).toEqual([ + HEADING, + '', + '1. Element.fill("#comment") value="line one', + ' line two', + ' line three"' + ]) + expect(unattributedLines(lines)).toEqual([]) + }) + + it('indents a multi-line label — a captured `execute` script spans lines', () => { + const lines = transcript([ + cmd('execute', { + timestamp: 1100, + startTime: 1050, + args: ['const el = document.body\nreturn el.textContent'] + }) + ]) + expect(lines).toEqual([ + HEADING, + '', + '1. Page.evaluate("const el = document.body', + ' return el.textContent")' + ]) + expect(unattributedLines(lines)).toEqual([]) + }) + + it('matches the continuation indent to a two-digit marker', () => { + // `10. ` is four columns wide; a fixed two- or three-space indent would + // leave the tail outside the tenth item. + const commands = Array.from({ length: 10 }, (_, i) => + cmd('click', { timestamp: 1100 + i * 10, startTime: 1050 + i * 10 }) + ) + commands[9] = cmd('setValue', { + timestamp: 1190, + startTime: 1140, + args: ['#comment', 'first\nsecond'] + }) + const lines = transcript(commands) + expect(lines.slice(-2)).toEqual([ + '10. Element.fill("#comment") value="first', + ' second"' + ]) + expect(unattributedLines(lines)).toEqual([]) + }) + + it('keeps every line attributed when a step carries both a multi-line value and a multi-line error', () => { + const lines = transcript([ + cmd('setValue', { + timestamp: 1100, + startTime: 1050, + args: ['#comment', 'typed\nover two lines'], + error: { name: 'Error', message: MULTILINE_ERROR } + }), + cmd('click', { timestamp: 1200, startTime: 1150 }) + ]) + expect(lines).toEqual([ + HEADING, + '', + '1. Element.fill("#comment") value="typed', + ' over two lines" ERROR: Expect $(`#flash`) to have text', + '', + ' Expected: "Welcome!"', + ' Received: "Your username is invalid!"', + '2. Element.click()' + ]) + expect(unattributedLines(lines)).toEqual([]) + }) + + it('strips ANSI from a colour-coded label and a colour-coded typed value', () => { + const lines = transcript([ + cmd('setValue', { + timestamp: 1100, + startTime: 1050, + args: [`${ESC}[36m#comment${ESC}[39m`, `${ESC}[1msecret${ESC}[22m`] + }) + ]) + expect(lines).toEqual([ + HEADING, + '', + '1. Element.fill("#comment") value="secret"' + ]) + }) + + it('drops a trailing newline in an error instead of emitting a bare blank tail', () => { + const lines = transcript([ + cmd('click', { + timestamp: 1100, + startTime: 1050, + error: { name: 'Error', message: 'boom\n' } + }) + ]) + expect(lines).toEqual([HEADING, '', '1. Element.click() ERROR: boom']) + }) + + it('renders a non-object error value', () => { + const lines = transcript([ + cmd('click', { + timestamp: 1100, + startTime: 1050, + error: 'plain failure' as unknown as CommandLog['error'] + }) + ]) + expect(lines).toEqual([ + HEADING, + '', + '1. Element.click() ERROR: plain failure' + ]) + }) + + it('emits heading only when no command maps to an action', () => { + expect(transcript([cmd('clearValue'), cmd('executeScript')])).toEqual([ + HEADING, + '' + ]) + }) +}) From 50347671c0365f0c6e40e910a5f0a4cbda8add2b Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 16:22:56 +0530 Subject: [PATCH 12/14] fix(app): replay a non-boolean attribute removal as a removal --- .../app/src/components/browser/snapshot.ts | 12 +- .../test-ui/workbench/player/snapshot.test.ts | 104 +++++++++++++----- 2 files changed, 86 insertions(+), 30 deletions(-) diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index dd961636..fd099214 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -471,7 +471,17 @@ export class DevtoolsBrowser extends Element { return } - const value = mutation.attributeValue ?? '' + // An absent value is the capture's removal signal (`mutations.ts` sends + // undefined where `getAttribute` read null), and `class=""` is not `class` + // gone: presence-based selectors and `aria-label` semantics both turn on it. + // The `value` PROPERTY is deliberately left alone — the collector re-sends + // the field's real value on every input event, so it stays authoritative. + if (mutation.attributeValue === undefined) { + el.removeAttribute(name) + return + } + + const value = mutation.attributeValue el.setAttribute(name, value) // Form-field state lives on the PROPERTY, not just the attribute — mirror it // so a replayed input shows the captured value, including a field cleared diff --git a/packages/app/test-ui/workbench/player/snapshot.test.ts b/packages/app/test-ui/workbench/player/snapshot.test.ts index 68936f75..cffef991 100644 --- a/packages/app/test-ui/workbench/player/snapshot.test.ts +++ b/packages/app/test-ui/workbench/player/snapshot.test.ts @@ -411,34 +411,34 @@ describe('wdio-devtools-browser', () => { * cases drive the general shape through `disabled` and `readonly` on a text * field and through a cleared `checked` on the captured radio. */ - describe('boolean attributes', () => { - /** - * Replays one boolean-attribute mutation on the login page. The typed - * username rides along as the signal that the window landed — the attribute - * under test cannot be that signal, since it is what the assertions read. - */ - async function replayEntry(entry: TraceMutation): Promise { - const el = await mountBrowser({ - commands: loginTrace.commands, - mutations: [loginTrace.loginDocument, loginTrace.usernameTyped, entry] - }) - await replayedPage(el) - const doc = await replayAfter(el, () => selectMutation(entry)) - await waitUntil( - () => input(doc, '#username').value === TYPED_USERNAME, - 'the replay window to be applied' - ) - return doc - } + /** + * Replays one attribute mutation on the login page. The typed username rides + * along as the signal that the window landed — the attribute under test cannot + * be that signal, since it is what the assertions read. + */ + async function replayMutation(entry: TraceMutation): Promise { + const el = await mountBrowser({ + commands: loginTrace.commands, + mutations: [loginTrace.loginDocument, loginTrace.usernameTyped, entry] + }) + await replayedPage(el) + const doc = await replayAfter(el, () => selectMutation(entry)) + await waitUntil( + () => input(doc, '#username').value === TYPED_USERNAME, + 'the replay window to be applied' + ) + return doc + } - const replayAttribute = ( - target: string, - attributeName: string, - attributeValue?: string - ) => replayEntry(mutation({ target, attributeName, attributeValue })) + const replayAttributeOn = ( + target: string, + attributeName: string, + attributeValue?: string + ) => replayMutation(mutation({ target, attributeName, attributeValue })) + describe('boolean attributes', () => { it('drops a boolean attribute the capture recorded as false', async () => { - const doc = await replayAttribute(REF.username, 'disabled', 'false') + const doc = await replayAttributeOn(REF.username, 'disabled', 'false') // Written verbatim, `disabled="false"` DISABLES the field — the state the // capture says the page was not in. @@ -452,7 +452,7 @@ describe('wdio-devtools-browser', () => { // The captured radio has `checked="checked"`, so the removal is observable: // a cleared attribute written as an empty one stays present and keeps // reading as checked. - const doc = await replayAttribute(REF.planGuest, 'checked') + const doc = await replayAttributeOn(REF.planGuest, 'checked') const guest = input(doc, '#plan-guest') expect(guest.getAttribute('checked')).toBeNull() @@ -463,7 +463,7 @@ describe('wdio-devtools-browser', () => { it('keeps a boolean attribute the capture carries with an empty value', async () => { // `` reaches the wire as an empty value, so empty is the // PRESENT state — reading the string for truthiness would drop it. - const doc = await replayAttribute(REF.username, 'readonly', '') + const doc = await replayAttributeOn(REF.username, 'readonly', '') const username = input(doc, '#username') expect(username.readOnly).toBe(true) @@ -484,7 +484,7 @@ describe('wdio-devtools-browser', () => { // The captured radio arrives with `checked`, so the removal is // observable: a cleared attribute serialized as an empty one stays // present and keeps reading as checked. - const doc = await replayEntry( + const doc = await replayMutation( capturedAttributeMutation(REF.planGuest, '', (el) => el.removeAttribute('checked') ) @@ -497,7 +497,7 @@ describe('wdio-devtools-browser', () => { }) it('keeps an attribute the captured page set to an empty value', async () => { - const doc = await replayEntry( + const doc = await replayMutation( capturedAttributeMutation(REF.username, '', (el) => el.setAttribute('readonly', '') ) @@ -510,6 +510,52 @@ describe('wdio-devtools-browser', () => { }) }) + /** + * Removal has to survive for attributes that are NOT boolean too. `aria-label` + * is the case with a consequence a reader can see: `#cancel` is captured with + * one, and the element overlay names a box by `aria-label` BEFORE its visible + * text — so an `aria-label=""` left behind by a coerced removal names the + * button `''` instead of letting `Cancel` name it. + */ + describe('non-boolean attributes', () => { + it('removes a non-boolean attribute a mutation carries no value for', async () => { + const doc = await replayAttributeOn(REF.cancel, 'aria-label') + + const cancel = doc.querySelector('#cancel')! + expect(cancel.getAttribute('aria-label')).toBeNull() + expect(cancel.outerHTML).not.toContain('aria-label') + }) + + it('removes a non-boolean attribute the captured page removed', async () => { + const doc = await replayMutation( + capturedAttributeMutation( + REF.cancel, + ``, + (el) => el.removeAttribute('aria-label') + ) + ) + + const cancel = doc.querySelector('#cancel')! + expect(cancel.getAttribute('aria-label')).toBeNull() + }) + + it('keeps a non-boolean attribute the captured page set to an empty value', async () => { + const doc = await replayAttributeOn(REF.cancel, 'aria-label', '') + + // The control: an empty value is a value. A fix that removed on `''` too + // would pass the two cases above while losing this distinction. + expect(doc.querySelector('#cancel')!.getAttribute('aria-label')).toBe('') + }) + + it('writes a non-boolean attribute the captured page changed', async () => { + const doc = await replayAttributeOn(REF.cancel, 'aria-label', 'Go back') + + expect(doc.querySelector('#cancel')!.getAttribute('aria-label')).toBe( + 'Go back' + ) + }) + }) + describe('command selection', () => { it('shows the page a navigating click produced, not the one it left', async () => { const el = await mountBrowser(loginTrace) From 33efd44bd61c4e6030b9105c313e22775b0ef373 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 16:37:03 +0530 Subject: [PATCH 13/14] fix(app): keep a boolean attribute the page set to false --- .../components/browser/boolean-attribute.ts | 33 +++++++++--- .../app/src/components/browser/snapshot.ts | 2 +- .../test-ui/workbench/player/snapshot.test.ts | 39 ++++++++++++--- packages/app/tests/boolean-attribute.test.ts | 50 ++++++++++++------- 4 files changed, 93 insertions(+), 31 deletions(-) diff --git a/packages/app/src/components/browser/boolean-attribute.ts b/packages/app/src/components/browser/boolean-attribute.ts index 5b9ea491..44c5d3d8 100644 --- a/packages/app/src/components/browser/boolean-attribute.ts +++ b/packages/app/src/components/browser/boolean-attribute.ts @@ -34,10 +34,29 @@ const BOOLEAN_ATTRIBUTES = new Set([ export const isBooleanAttribute = (name: string) => BOOLEAN_ATTRIBUTES.has(name.toLowerCase()) -/** State a captured boolean attribute carries. The collector emits form-field - * state as `String(el.checked)` and every other record as the attribute's own - * value, so only a literal "false" — and a record carrying no value, which - * leaves no attribute state to set — means off: an empty value is a present - * attribute (`` reaches the wire as `''`). */ -export const booleanAttributeOn = (value?: string) => - value !== undefined && value.toLowerCase() !== 'false' +/** The only boolean attribute the collector reports as a PROPERTY state rather + * than as the attribute's own value: `packages/script` emits `String(el.checked)` + * on every input and change, so a cleared checkbox arrives as "false". Every + * other boolean attribute reaches the wire only through a real mutation record, + * which carries whatever the page set. */ +const PROPERTY_STATE_ATTRIBUTES = new Set(['checked']) + +/** State a captured boolean attribute carries. A record with no value is off — + * there is no attribute state to set. Otherwise presence IS the state, so any + * value means on, including `''` (`` reaches the wire empty). + * + * A literal "false" is the one value that depends on which attribute it is: + * `checked="false"` is the collector reporting an unchecked box, but + * `disabled="false"` can only be the page having SET that attribute, and a + * boolean attribute is active whenever present — so it replays as disabled. + * The residual ambiguity is a page literally writing `checked="false"`; the + * collector's own signal wins there, since it fires on every field edit. */ +export const booleanAttributeOn = (name: string, value?: string) => { + if (value === undefined) { + return false + } + if (!PROPERTY_STATE_ATTRIBUTES.has(name.toLowerCase())) { + return true + } + return value.toLowerCase() !== 'false' +} diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index fd099214..de702af2 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -466,7 +466,7 @@ export class DevtoolsBrowser extends Element { this.#applyBooleanAttribute( el, name, - booleanAttributeOn(mutation.attributeValue) + booleanAttributeOn(name, mutation.attributeValue) ) return } diff --git a/packages/app/test-ui/workbench/player/snapshot.test.ts b/packages/app/test-ui/workbench/player/snapshot.test.ts index cffef991..1195534f 100644 --- a/packages/app/test-ui/workbench/player/snapshot.test.ts +++ b/packages/app/test-ui/workbench/player/snapshot.test.ts @@ -437,15 +437,42 @@ describe('wdio-devtools-browser', () => { ) => replayMutation(mutation({ target, attributeName, attributeValue })) describe('boolean attributes', () => { - it('drops a boolean attribute the capture recorded as false', async () => { + /** + * `disabled="false"` is invalid markup that browsers still honour, because a + * boolean attribute is active whenever PRESENT. Asserted here on a plain + * element rather than taken on trust, since the replay rule below is only + * right if this is — if a browser read the value instead, the captured page + * would have been enabled and keeping the attribute would be the bug. + */ + it('is the browser, not this replay, that reads a present `disabled=false` as disabled', () => { + const probe = document.createElement('input') + probe.setAttribute('disabled', 'false') + + expect(probe.disabled).toBe(true) + }) + + it('keeps a non-checked boolean attribute the page set to "false"', async () => { const doc = await replayAttributeOn(REF.username, 'disabled', 'false') - // Written verbatim, `disabled="false"` DISABLES the field — the state the - // capture says the page was not in. + // Only the page can have put `disabled="false"` on the wire, and that field + // IS disabled — so the replay keeps it. Dropping it replayed a control the + // capture recorded as disabled as an enabled one. Presence is asserted, not + // the value: the replay normalizes it to `''`, which reads identically. const username = input(doc, '#username') - expect(username.getAttribute('disabled')).toBeNull() - expect(username.disabled).toBe(false) - expect(reparse(username, '#username').disabled).toBe(false) + expect(username.hasAttribute('disabled')).toBe(true) + expect(username.disabled).toBe(true) + expect(reparse(username, '#username').disabled).toBe(true) + }) + + it('drops the checked state the capture recorded as false', async () => { + // `checked` is the one boolean attribute the collector reports as a + // property state (`String(el.checked)`), so here "false" means unchecked. + const doc = await replayAttributeOn(REF.planGuest, 'checked', 'false') + + const guest = input(doc, '#plan-guest') + expect(guest.getAttribute('checked')).toBeNull() + expect(guest.checked).toBe(false) + expect(reparse(guest, '#plan-guest').checked).toBe(false) }) it('removes a boolean attribute a mutation carries no value for', async () => { diff --git a/packages/app/tests/boolean-attribute.test.ts b/packages/app/tests/boolean-attribute.test.ts index 382cb348..37ecb794 100644 --- a/packages/app/tests/boolean-attribute.test.ts +++ b/packages/app/tests/boolean-attribute.test.ts @@ -13,39 +13,55 @@ import { * test passes whatever that value is. */ describe('booleanAttributeOn', () => { - it('reads the literal string "false" as off', () => { - // The collector emits form-field state as `String(el.checked)`, so this is - // the shape a cleared checkbox arrives in. - expect(booleanAttributeOn('false')).toBe(false) - }) - - it('reads a missing value as off', () => { + it('reads a missing value as off, for any attribute', () => { // Nothing to set: a record carrying no value leaves no attribute state. - expect(booleanAttributeOn(undefined)).toBe(false) - expect(booleanAttributeOn()).toBe(false) + expect(booleanAttributeOn('checked', undefined)).toBe(false) + expect(booleanAttributeOn('checked')).toBe(false) + expect(booleanAttributeOn('disabled', undefined)).toBe(false) }) it('reads an empty value as ON, because empty means present', () => { - // The real MutationObserver record sends `getAttribute() || ''`, so a bare + // A real MutationObserver record sends the attribute's own value, so a bare // `` reaches the wire with an empty value. Reading the // string for truthiness would drop the attribute the page actually had. - expect(booleanAttributeOn('')).toBe(true) + expect(booleanAttributeOn('disabled', '')).toBe(true) + expect(booleanAttributeOn('checked', '')).toBe(true) }) it('reads the literal string "true" as on', () => { - expect(booleanAttributeOn('true')).toBe(true) + expect(booleanAttributeOn('checked', 'true')).toBe(true) + expect(booleanAttributeOn('disabled', 'true')).toBe(true) }) it('reads any other value as on', () => { // Presence is the state, so the value is not a boolean to parse: // `checked="checked"` and `disabled="disabled"` are the common spellings. - expect(booleanAttributeOn('checked')).toBe(true) - expect(booleanAttributeOn('0')).toBe(true) + expect(booleanAttributeOn('checked', 'checked')).toBe(true) + expect(booleanAttributeOn('disabled', '0')).toBe(true) }) - it('ignores the case a producer spelled "false" in', () => { - expect(booleanAttributeOn('False')).toBe(false) - expect(booleanAttributeOn('FALSE')).toBe(false) + describe('a literal "false"', () => { + it('is off for `checked`, the one attribute reported as a property state', () => { + // The collector emits form-field state as `String(el.checked)` on every + // input and change, so this is the shape a CLEARED checkbox arrives in — + // the only boolean attribute for which "false" is a state and not a value. + expect(booleanAttributeOn('checked', 'false')).toBe(false) + }) + + it("ignores the case `checked`'s state was spelled in", () => { + expect(booleanAttributeOn('checked', 'False')).toBe(false) + expect(booleanAttributeOn('checked', 'FALSE')).toBe(false) + }) + + it('is ON for every other boolean attribute, where it is a present value', () => { + // `disabled="false"` can only have come from the page setting it, and a + // boolean attribute is active whenever PRESENT — so the captured control + // was disabled. Removing it would replay it as enabled. + expect(booleanAttributeOn('disabled', 'false')).toBe(true) + expect(booleanAttributeOn('readonly', 'false')).toBe(true) + expect(booleanAttributeOn('required', 'false')).toBe(true) + expect(booleanAttributeOn('open', 'false')).toBe(true) + }) }) }) From d56ce578d7956a23e07a54fe2a5f13d5cb1ff1e1 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Mon, 3 Aug 2026 17:03:08 +0530 Subject: [PATCH 14/14] fix(app): empty a replayed field whose value attribute the page removed --- .../app/src/components/browser/snapshot.ts | 25 +++++- .../test-ui/workbench/player/snapshot.test.ts | 82 +++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index de702af2..be4d8ff2 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -67,6 +67,10 @@ const COMPONENT = 'wdio-devtools-browser' @customElement(COMPONENT) export class DevtoolsBrowser extends Element { #vdom = document.createDocumentFragment() + /** Fields a field-state record has written the `value` PROPERTY of, which is + * what separates a dirty replayed field from a pristine one. Weak, and every + * replay rebuilds the document, so entries die with the elements they key. */ + #fieldStateApplied = new WeakSet() #activeUrl?: string /** Base64 PNG of the screenshot for the currently selected command, or null. */ #screenshotData: string | null = null @@ -474,10 +478,9 @@ export class DevtoolsBrowser extends Element { // An absent value is the capture's removal signal (`mutations.ts` sends // undefined where `getAttribute` read null), and `class=""` is not `class` // gone: presence-based selectors and `aria-label` semantics both turn on it. - // The `value` PROPERTY is deliberately left alone — the collector re-sends - // the field's real value on every input event, so it stays authoritative. if (mutation.attributeValue === undefined) { el.removeAttribute(name) + this.#clearRemovedFieldValue(el, name) return } @@ -488,9 +491,27 @@ export class DevtoolsBrowser extends Element { // back to empty. if (name === 'value' && 'value' in el) { ;(el as HTMLInputElement).value = value + this.#fieldStateApplied.add(el) } } + /** A pristine field's text IS its `value` attribute, so removing the attribute + * empties the field — but the property stops tracking it once assigned, and + * the snapshot render assigns it. Mirroring the clear restores that coupling, + * EXCEPT where a field-state record already set the property: the captured + * field was dirty then, and a dirty field keeps its text when the attribute + * goes. Only the collector's per-edit records carry that text, so clearing + * there would lose what the user actually typed. */ + #clearRemovedFieldValue(el: HTMLElement, name: string) { + if (name !== 'value' || !('value' in el)) { + return + } + if (this.#fieldStateApplied.has(el)) { + return + } + ;(el as HTMLInputElement).value = '' + } + /** Presence IS the state of a boolean attribute, so the captured state is * toggled rather than written — the markup a re-serialization reads then says * what the replayed page shows. `checked` is mirrored onto the property as diff --git a/packages/app/test-ui/workbench/player/snapshot.test.ts b/packages/app/test-ui/workbench/player/snapshot.test.ts index 1195534f..32ffe2b7 100644 --- a/packages/app/test-ui/workbench/player/snapshot.test.ts +++ b/packages/app/test-ui/workbench/player/snapshot.test.ts @@ -583,6 +583,88 @@ describe('wdio-devtools-browser', () => { }) }) + /** + * `value` is the one non-boolean attribute with a live PROPERTY behind it, and + * the two are only coupled while the field is pristine — once anything assigns + * the property, removing the attribute no longer changes what the field shows. + * That is a browser rule, not ours (probed in the first case below), and it is + * why removal leaves the property alone: the replay assigns it exactly when the + * collector reported field state, which is exactly when the captured field had + * been typed into and was therefore dirty itself. + */ + describe('a removed `value` attribute', () => { + /** Replays `entry` with a trailing checkbox tick as the landing signal, so + * the field under test is never also the signal that the window arrived. */ + async function replayThenTick( + entry: TraceMutation, + ...before: TraceMutation[] + ) { + const el = await mountBrowser({ + commands: loginTrace.commands, + mutations: [ + loginTrace.loginDocument, + ...before, + entry, + loginTrace.rememberChecked + ] + }) + await replayedPage(el) + const doc = await replayAfter(el, () => + selectMutation(loginTrace.rememberChecked) + ) + await waitUntil( + () => input(doc, '#remember').checked, + 'the replay window to be applied' + ) + return doc + } + + // `attributeValue` is passed explicitly: the builder defaults it to a real + // value, so omitting the key would write one rather than remove the attribute. + const valueRemoval = () => + mutation({ + target: REF.username, + attributeName: 'value', + attributeValue: undefined + }) + + it('is the browser that couples a pristine field to its value attribute', () => { + const pristine = document.createElement('input') + pristine.setAttribute('value', STALE_USERNAME) + expect(pristine.value).toBe(STALE_USERNAME) + pristine.removeAttribute('value') + expect(pristine.value).toBe('') + + // ...and that decouples it once the property has been assigned. + const dirty = document.createElement('input') + dirty.setAttribute('value', STALE_USERNAME) + dirty.value = TYPED_USERNAME + dirty.removeAttribute('value') + expect(dirty.value).toBe(TYPED_USERNAME) + }) + + it('clears a field the capture never reported typing into', async () => { + const doc = await replayThenTick(valueRemoval()) + + // No property assignment has happened, so the replayed field is pristine + // and the removal clears it on its own — no mirror needed. + const username = input(doc, '#username') + expect(username.getAttribute('value')).toBeNull() + expect(username.value).toBe('') + }) + + it('keeps the text of a field the capture reported typing into', async () => { + const doc = await replayThenTick(valueRemoval(), loginTrace.usernameTyped) + + // The captured field was dirty when the page removed the attribute, so it + // kept showing the typed text. Clearing the property here — the obvious + // reading of "a removal should clear the field" — would lose it. + const username = input(doc, '#username') + expect(username.getAttribute('value')).toBeNull() + expect(username.value).toBe(TYPED_USERNAME) + }) + }) + describe('command selection', () => { it('shows the page a navigating click produced, not the one it left', async () => { const el = await mountBrowser(loginTrace)