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..44c5d3d8 --- /dev/null +++ b/packages/app/src/components/browser/boolean-attribute.ts @@ -0,0 +1,62 @@ +/** 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()) + +/** 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/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..be4d8ff2 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' @@ -66,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 @@ -451,7 +456,8 @@ export class DevtoolsBrowser extends Element { } #handleAttributeMutation(mutation: TraceMutation) { - if (!mutation.attributeName) { + const name = mutation.attributeName + if (!name) { return } @@ -460,15 +466,62 @@ export class DevtoolsBrowser extends Element { return } - const value = mutation.attributeValue ?? '' - el.setAttribute(mutation.attributeName, value) + if (isBooleanAttribute(name)) { + this.#applyBooleanAttribute( + el, + name, + booleanAttributeOn(name, mutation.attributeValue) + ) + return + } + + // 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. + if (mutation.attributeValue === undefined) { + el.removeAttribute(name) + this.#clearRemovedFieldValue(el, 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 / 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' + 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 + * 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/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 f23d1c5a..9f356dde 100644 --- a/packages/app/src/components/sidebar/explorer.ts +++ b/packages/app/src/components/sidebar/explorer.ts @@ -19,11 +19,13 @@ import { getFramework, getLaunchCommand, getRerunCommand, - getRunCapabilities, + getRunAllDisabledReason, getRunDisabledReason, + isRunAll, isRunDisabled, isRunDisabledDetail } from './runnerCapabilities.js' +import { RUN_ALL_UID } from './constants.js' import { BASELINE_API, TESTS_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 { `, + (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' + ) + }) + }) + + /** + * `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) @@ -399,6 +717,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/boolean-attribute.test.ts b/packages/app/tests/boolean-attribute.test.ts new file mode 100644 index 00000000..37ecb794 --- /dev/null +++ b/packages/app/tests/boolean-attribute.test.ts @@ -0,0 +1,114 @@ +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 a missing value as off, for any attribute', () => { + // Nothing to set: a record carrying no value leaves no attribute state. + 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', () => { + // 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('disabled', '')).toBe(true) + expect(booleanAttributeOn('checked', '')).toBe(true) + }) + + it('reads the literal string "true" as on', () => { + 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', 'checked')).toBe(true) + expect(booleanAttributeOn('disabled', '0')).toBe(true) + }) + + 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) + }) + }) +}) + +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/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/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() }) diff --git a/packages/app/tests/network-helpers.test.ts b/packages/app/tests/network-helpers.test.ts index 8872d679..27f8493d 100644 --- a/packages/app/tests/network-helpers.test.ts +++ b/packages/app/tests/network-helpers.test.ts @@ -87,6 +87,30 @@ describe('getResourceType', () => { expect(getResourceType(request({ type: 'fetch' }))).toBe('Fetch') expect(getResourceType(request({ type: 'other' }))).toBe('Other') }) + // A reconstructed trace carries an empty HAR `content.mimeType`, so the + // backend reports every request as `other`. The response header is then the + // only thing left that identifies the resource — without sniffing it, every + // row in the Network tab renders the same neutral dot. + it('sniffs a trace-shaped request whose captured type is other', () => { + expect( + getResourceType( + request({ + type: 'other', + url: 'https://the-internet.herokuapp.com/login', + responseHeaders: { 'content-type': 'text/html; charset=utf-8' } + }) + ) + ).toBe('HTML') + expect( + getResourceType( + request({ + type: 'other', + url: 'https://the-internet.herokuapp.com/js/foundation.js', + responseHeaders: {} + }) + ) + ).toBe('JS') + }) it('names a bucket for every word in the shared vocabulary', () => { // The table is `Record`, so a new word breaks the 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' ]) }) 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/spec-trace-helpers.ts b/packages/core/src/spec-trace-helpers.ts index 8271d341..01cca374 100644 --- a/packages/core/src/spec-trace-helpers.ts +++ b/packages/core/src/spec-trace-helpers.ts @@ -17,7 +17,7 @@ import type { } from '@wdio/devtools-shared' import type { TraceCapturer } from './trace-exporter.js' import { writeTraceZip } from './trace-exporter.js' -import { deterministicUid } from './uid.js' +import { deterministicUid, isStepUidOf } from './uid.js' import { trimChar } from './artifact-naming.js' // ─── SpecRange ──────────────────────────────────────────────────────────────── @@ -222,9 +222,11 @@ export function filterTestMetadataBySpec( } /** - * Filter a full `testUid → metadata` map down to a single test's entry. The - * per-test analog of {@link filterTestMetadataBySpec}: a test slice's metadata - * is just that one test's entry, attached as its tracingGroup name. + * Filter a full `testUid → metadata` map down to a single test's entry and its + * step entries. The per-test analog of {@link filterTestMetadataBySpec}. The + * steps have to come along: `buildGroupPath` names each step group from this + * map and falls back to the raw uid when the entry is missing, so dropping them + * renders a scenario's steps as `stable-…:step:1` instead of their Gherkin text. */ export function filterTestMetadataByUid( allMetadata: TestMetadataMap, @@ -235,6 +237,11 @@ export function filterTestMetadataByUid( if (entry) { filtered.set(testUid, entry) } + for (const [uid, meta] of allMetadata) { + if (isStepUidOf(uid, testUid)) { + filtered.set(uid, meta) + } + } return filtered } 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/src/uid.ts b/packages/core/src/uid.ts index 40ee085f..d0e81d72 100644 --- a/packages/core/src/uid.ts +++ b/packages/core/src/uid.ts @@ -16,6 +16,25 @@ export function deterministicUid(...parts: string[]): string { return `stable-${Math.abs(hash).toString(36)}` } +const STEP_UID_SEPARATOR = ':step:' + +/** + * Key for one step (a Cucumber `Given`/`When`/`Then`) inside a test, derived + * from the test's own uid plus a per-test index — repeated step text can't + * collide. Derived rather than hashed so a step's owning test is recoverable + * from the key alone, which is what {@link isStepUidOf} relies on. + */ +export function stepMetadataUid(testUid: string, index: number): string { + return `${testUid}${STEP_UID_SEPARATOR}${index}` +} + +/** True when `uid` is a step of `testUid`. Anchored on the full test uid plus + * the separator, so a sibling test whose uid merely starts with the same + * characters doesn't match. */ +export function isStepUidOf(uid: string, testUid: string): boolean { + return uid.startsWith(`${testUid}${STEP_UID_SEPARATOR}`) +} + // Counter for disambiguating repeated (file, name) signatures within a single // test run. Cleared by resetSignatureCounters() between runs. const signatureCounters = new Map() diff --git a/packages/core/tests/spec-trace-helpers.test.ts b/packages/core/tests/spec-trace-helpers.test.ts index c0e4ea91..52f3d92e 100644 --- a/packages/core/tests/spec-trace-helpers.test.ts +++ b/packages/core/tests/spec-trace-helpers.test.ts @@ -13,6 +13,7 @@ import { recordSliceBoundary, recordSpecBoundary, sanitizeSpecName, + stepMetadataUid, writeSpecTrace, writeTestSliceTrace, type SpecBoundaryContext, @@ -131,6 +132,39 @@ describe('filterTestMetadataByUid', () => { expect([...filterTestMetadataByUid(all, 'u1').keys()]).toEqual(['u1']) expect(filterTestMetadataByUid(all, 'missing').size).toBe(0) }) + + it("keeps the test's own step entries so their titles survive the slice", () => { + const all: TestMetadataMap = new Map([ + ['u1', { title: 'A', specFile: '/a.js' }], + [ + stepMetadataUid('u1', 1), + { title: 'Given I log in', specFile: '/a.js' } + ], + [stepMetadataUid('u1', 2), { title: 'Then I see it', specFile: '/a.js' }], + ['u2', { title: 'B', specFile: '/b.js' }], + [stepMetadataUid('u2', 1), { title: 'Given other', specFile: '/b.js' }] + ]) + const filtered = filterTestMetadataByUid(all, 'u1') + expect([...filtered.keys()]).toEqual([ + 'u1', + stepMetadataUid('u1', 1), + stepMetadataUid('u1', 2) + ]) + expect(filtered.get(stepMetadataUid('u1', 1))?.title).toBe('Given I log in') + }) + + it('keeps step entries of the requested test only, never a sibling test whose uid shares a prefix', () => { + const all: TestMetadataMap = new Map([ + ['u1', { title: 'A', specFile: '/a.js' }], + [stepMetadataUid('u1', 1), { title: 'step of A', specFile: '/a.js' }], + ['u12', { title: 'B', specFile: '/a.js' }], + [stepMetadataUid('u12', 1), { title: 'step of B', specFile: '/a.js' }] + ]) + expect([...filterTestMetadataByUid(all, 'u1').keys()]).toEqual([ + 'u1', + stepMetadataUid('u1', 1) + ]) + }) }) describe('buildTestSliceSessionId', () => { 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-hierarchy.test.ts b/packages/core/tests/trace-hierarchy.test.ts index fc10cd3f..aa3eabc6 100644 --- a/packages/core/tests/trace-hierarchy.test.ts +++ b/packages/core/tests/trace-hierarchy.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect } from 'vitest' -import { buildGroupPath } from '@wdio/devtools-core' +import { + buildGroupPath, + filterTestMetadataByUid, + stepMetadataUid +} from '@wdio/devtools-core' import type { CommandLog, TestMetadataMap } from '@wdio/devtools-shared' function cmd(overrides: Partial = {}): CommandLog { @@ -60,4 +64,29 @@ describe('buildGroupPath', () => { { uid: 'st1', title: 'st1' } ]) }) + + // The chain a per-test trace slice actually goes through. Asserted end to end + // because the two halves were individually defensible — the filter narrowed to + // one test, the path fell back to the uid — and only their composition showed + // the defect: every Gherkin step rendered as `stable-…:step:1` in the viewer. + it('names steps from a per-test-filtered metadata map', () => { + const all: TestMetadataMap = new Map([ + ['sc1', { title: 'Scenario', specFile: '/login.feature' }], + [ + stepMetadataUid('sc1', 1), + { title: 'When I log in', specFile: '/login.feature' } + ], + ['sc2', { title: 'Other', specFile: '/login.feature' }] + ]) + const sliceMeta = filterTestMetadataByUid(all, 'sc1') + expect( + buildGroupPath( + cmd({ testUid: 'sc1', stepUid: stepMetadataUid('sc1', 1) }), + sliceMeta + ) + ).toEqual([ + { uid: 'sc1', title: 'Scenario' }, + { uid: stepMetadataUid('sc1', 1), title: 'When I log in' } + ]) + }) }) 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, + '' + ]) + }) +}) diff --git a/packages/core/tests/uid.test.ts b/packages/core/tests/uid.test.ts new file mode 100644 index 00000000..06d46e8e --- /dev/null +++ b/packages/core/tests/uid.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from 'vitest' +import { + deterministicUid, + generateStableUid, + isStepUidOf, + resetSignatureCounters, + stepMetadataUid +} from '@wdio/devtools-core' + +describe('deterministicUid', () => { + it('is stable across calls and distinct per input', () => { + expect(deterministicUid('/a.js', 'logs in')).toBe( + deterministicUid('/a.js', 'logs in') + ) + expect(deterministicUid('/a.js', 'logs in')).not.toBe( + deterministicUid('/a.js', 'logs out') + ) + }) + + it('separates parts so a shifted split hashes differently', () => { + expect(deterministicUid('ab', 'c')).not.toBe(deterministicUid('a', 'bc')) + }) +}) + +describe('generateStableUid', () => { + it('disambiguates repeated (file, name) pairs within one run', () => { + resetSignatureCounters() + const first = generateStableUid('/a.js', 'logs in') + const second = generateStableUid('/a.js', 'logs in') + expect(second).not.toBe(first) + resetSignatureCounters() + expect(generateStableUid('/a.js', 'logs in')).toBe(first) + }) +}) + +describe('stepMetadataUid / isStepUidOf', () => { + it('derives a key that reports its owning test', () => { + const uid = stepMetadataUid('stable-abc', 2) + expect(uid).toBe('stable-abc:step:2') + expect(isStepUidOf(uid, 'stable-abc')).toBe(true) + }) + + it('gives each index its own key', () => { + expect(stepMetadataUid('stable-abc', 1)).not.toBe( + stepMetadataUid('stable-abc', 2) + ) + }) + + it('does not claim a step of a test whose uid merely shares a prefix', () => { + expect(isStepUidOf(stepMetadataUid('stable-abcd', 1), 'stable-abc')).toBe( + false + ) + }) + + it('does not treat the test uid itself as one of its steps', () => { + expect(isStepUidOf('stable-abc', 'stable-abc')).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) diff --git a/packages/service/src/index.ts b/packages/service/src/index.ts index 2fa2f08d..67515d90 100644 --- a/packages/service/src/index.ts +++ b/packages/service/src/index.ts @@ -11,6 +11,7 @@ import { mapCommandToAction, recordSliceBoundary, resolveAdapterOutputDir, + stepMetadataUid, TestAttemptTracker, tracePolicyModeWarning, type SpecRange, @@ -514,7 +515,7 @@ export default class DevToolsHookService implements Services.ServiceInstance { return } this.#currentStepIndex += 1 - const uid = `${this.#currentTestUid}:step:${this.#currentStepIndex}` + const uid = stepMetadataUid(this.#currentTestUid, this.#currentStepIndex) const title = [step?.keyword, step?.text].filter(Boolean).join('').trim() || `Step ${this.#currentStepIndex}`