diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index ad6fb778..4bb9a40a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -23,9 +23,11 @@ - [ ] `service` (WebdriverIO adapter) - [ ] `nightwatch-devtools` (Nightwatch adapter) - [ ] `selenium-devtools` (Selenium adapter) +- [ ] `selenium-devtools-py` (Selenium Python adapter) - [ ] `backend` (server) - [ ] `app` (UI) - [ ] `script` (page-injected runtime) +- [ ] `trace` (Trace mode) ## Notes for reviewers diff --git a/.gitignore b/.gitignore index 1208c692..a36f454e 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,10 @@ packages/nightwatch-devtools/nightwatch-video-*.webm # trace output (mode: 'trace') trace-*.zip examples/**/trace-*/ +# ...but this one is a committed example, not output. The rule above exists for +# unpacked archives (`trace-/`), and a folder named for what it +# demonstrates collides with it. +!examples/selenium/python-test/trace-py-test/ # test results examples/**/test-results*/ diff --git a/examples/selenium/python-test/trace-py-test/pytest.ini b/examples/selenium/python-test/trace-py-test/pytest.ini new file mode 100644 index 00000000..819030c9 --- /dev/null +++ b/examples/selenium/python-test/trace-py-test/pytest.ini @@ -0,0 +1,123 @@ +; DevTools capture, committed with the project. +; +; Why here and not on the command line: +; - a contributor who clones this gets the same capture, without being told +; - a flag has to be retyped every run; an exported variable is per-shell +; +; Run it (no flags — pytest reads this file because it sits beside the test): +; pytest examples/selenium/python-test/trace-py-test/ +; pnpm show-trace examples/selenium/python-test/trace-py-test/test-results/*.zip +; +; Override one setting for one run, without editing this file: +; pytest -o devtools_trace_policy=on examples/.../trace-py-test/ +; pytest -o devtools_trace_granularity=session examples/.../trace-py-test/ +; pytest --devtools examples/.../trace-py-test/ ; live dashboard +; pytest -o devtools=false examples/.../trace-py-test/ ; capture nothing + +[pytest] + +; ───────────────────────────────────────────────────────────────────────────── +; devtools — opt in +; ───────────────────────────────────────────────────────────────────────────── +; - capture is ALWAYS opt-in: the plugin auto-loads when the package is +; installed, so it must never change an existing suite's behaviour +; - precedence, highest first: CLI flag > this file > environment +; - environment fallback: DEVTOOLS_ENABLE=1, or DEVTOOLS_PORT= +; - `-o devtools=false` turns it off for one run +; (which is why there is no --no-devtools flag) +; - not set below: devtools_trace already implies it + +; ───────────────────────────────────────────────────────────────────────────── +; devtools_trace — a dashboard, or a file +; ───────────────────────────────────────────────────────────────────────────── +; false live mode: streams to a dashboard window (the default) +; true trace mode: writes an archive to test-results/, opens no window +; +; - trace mode is what you want in CI, and when the run already happened +; - open an archive with `pnpm show-trace ` +devtools_trace = true + +; ───────────────────────────────────────────────────────────────────────────── +; devtools_trace_granularity — how many archives +; ───────────────────────────────────────────────────────────────────────────── +; session one archive for the whole run (the default) +; test one per test +; +; - a per-test archive holds only that test's own commands, console, network, +; DOM mutations, a11y trees and screencast frames +; - `spec` is not offered: this adapter's spec IS its test file, so it could +; only silently mean one of the two above +devtools_trace_granularity = test + +; ───────────────────────────────────────────────────────────────────────────── +; devtools_trace_policy — which archives to keep +; ───────────────────────────────────────────────────────────────────────────── +; Values: +; on keep everything (the default) +; retain-on-failure keep only what failed +; retain-on-first-failure ) +; on-first-retry ) accepted, but see below +; on-all-retries ) +; retain-on-failure-and-retries ) +; +; Those last four currently behave EXACTLY like retain-on-failure: +; - nothing this adapter puts on the wire carries an attempt number +; - so a retried test overwrites its own earlier outcome +; - so the retry-aware question cannot be asked at all +; - the backend logs the degradation rather than pretending otherwise +; - pick one only if you want retain-on-failure under a name that will mean +; more later +; +; What it combines with: +; granularity=test + retain-on-failure -> only the tests that failed +; granularity=session + retain-on-failure -> the whole run, if any failed +; either granularity + on -> everything +; +; As set here, a green run writes nothing at all — which is the point: the +; archives you have are the ones worth opening. +devtools_trace_policy = retain-on-failure + +; ───────────────────────────────────────────────────────────────────────────── +; Environment-only settings +; ───────────────────────────────────────────────────────────────────────────── +; No ini option yet. Export them, or pass the matching keyword to +; devtools.enable() in a plain script (see ../login.py). +; +; Connecting: +; DEVTOOLS_ENABLE=1 opt in (what `devtools = true` does) +; DEVTOOLS_HOST= backend host (default: localhost) +; DEVTOOLS_PORT= attach to a backend already listening +; DEVTOOLS_BACKEND_CMD= launch the backend your own way +; +; Capture: +; DEVTOOLS_BIDI=0 skip BiDi; console and network go quiet +; DEVTOOLS_OPEN=0 never open a dashboard window (CI) +; +; Trace mode: +; DEVTOOLS_TRACE=1 selects trace mode, but does NOT opt a pytest +; run in on its own — it is a mode fallback you +; may have exported for your own scripts +; DEVTOOLS_TRACE_GRANULARITY=test ) ambient, so neither turns trace mode +; DEVTOOLS_TRACE_POLICY=... ) on by itself — pair with DEVTOOLS_TRACE=1. +; ) The CLI flag, the ini option and the +; ) enable() argument DO imply it. A run +; ) that ignores one of these says so. +; DEVTOOLS_FILMSTRIP=0 drop the dense screencast from the archive +; DEVTOOLS_A11Y=0 drop the per-action a11y tree and element +; rects (loses the A11y tab and the overlay) +; +; Set for you, not by you — listed so an unexpected value is recognisable: +; DEVTOOLS_RUN_ID one id shared by every process of a run +; DEVTOOLS_RUNNER_CWD where a Rerun spawns (pytest's rootdir) +; DEVTOOLS_APP_REUSE ) +; DEVTOOLS_APP_HOST ) point a spawned rerun back at the +; DEVTOOLS_APP_PORT ) backend that asked for it +; +; The same choices in a plain script: +; devtools.enable( +; trace=True, +; trace_granularity="test", +; trace_policy="retain-on-failure", +; filmstrip=True, +; a11y=True, +; ) diff --git a/examples/selenium/python-test/test_login_pytest.py b/examples/selenium/python-test/trace-py-test/test_login_pytest.py similarity index 75% rename from examples/selenium/python-test/test_login_pytest.py rename to examples/selenium/python-test/trace-py-test/test_login_pytest.py index 158fb15b..5ffdfe1b 100644 --- a/examples/selenium/python-test/test_login_pytest.py +++ b/examples/selenium/python-test/trace-py-test/test_login_pytest.py @@ -13,10 +13,31 @@ pytest has no ``describe``/``it`` blocks; a class IS the grouping construct, so this is the closest equivalent to a nested ``describe`` in the JS examples. -Run it (the plugin is inert unless a devtools env var opts the run in): +This folder is the "committed config" shape: `pytest.ini` beside it already +says how to capture, so running it needs no flags and no environment. Read that +file — it documents every DevTools setting the adapter has, including the ones +that are environment-only. pip install -e packages/selenium-devtools-py - python -m pytest --devtools examples/selenium/python-test/test_login_pytest.py + pytest examples/selenium/python-test/trace-py-test/ + pnpm show-trace examples/selenium/python-test/trace-py-test/test-results/*.zip + +As committed it writes ONE archive PER FAILING TEST — `devtools_trace = true`, +`devtools_trace_granularity = test`, `devtools_trace_policy = retain-on-failure`. +So a green run writes nothing at all, which is the point: the archives you have +are the ones worth opening. + +To see the difference, break an assertion below — the flash-message one in +`test_rejects_invalid_credentials` is the easiest — and compare: + + pytest -o devtools_trace_policy=on examples/.../trace-py-test/ # 3 archives + pytest examples/.../trace-py-test/ # 1, the broken test + pytest -o devtools_trace_granularity=session examples/.../trace-py-test/ + # 1, the whole run + +`-o` overrides one setting for one run without editing the file. A sibling +example, `../login.py`, shows the same choices passed to `devtools.enable()` +instead, for a script with no test runner. The driver fixture is function-scoped, so each test gets its own browser session — which also exercises the adapter's per-driver capture state. diff --git a/package.json b/package.json index 996f9480..0fd9355a 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "demo:selenium": "pnpm --filter @wdio/selenium-devtools example", "demo:python": "python3 examples/selenium/python-test/web_form.py", "demo:python:login": "python3 examples/selenium/python-test/login.py", - "demo:python:pytest": "python3 -m pytest --devtools examples/selenium/python-test/test_login_pytest.py", + "demo:python:pytest": "python3 -m pytest examples/selenium/python-test/trace-py-test/", "dev": "pnpm --parallel dev", "preview": "pnpm --parallel preview", "test": "vitest run", diff --git a/packages/backend/src/trace-export-message.ts b/packages/backend/src/trace-export-message.ts index 5be36407..da587c60 100644 --- a/packages/backend/src/trace-export-message.ts +++ b/packages/backend/src/trace-export-message.ts @@ -11,7 +11,11 @@ import { type TraceExportResult } from '@wdio/devtools-shared' import type { ActiveRun } from './baseline/types.js' -import { exportActiveRunTrace } from './trace-export.js' +import { + exportActiveRunTrace, + exportPerTestTraces, + retentionDecision +} from './trace-export.js' const log = logger('@wdio/devtools-backend') @@ -56,11 +60,45 @@ export async function runTraceExport( JSON.stringify({ scope: TRACE_EXPORT_SCOPE.result, data: result }) ) try { - const path = await exportActiveRunTrace(deps.activeRun(), { + const run = deps.activeRun() + if (request.traceGranularity === 'test') { + // Retention is per slice here, so the run-wide decision below would be + // the wrong question: one failing test must not keep the passing ones. + const paths = await exportPerTestTraces(run, request) + log.info( + paths.length + ? `Trace exported for session ${request.sessionId}: ${paths.length} test slice(s)` + : `No test slice for session ${request.sessionId} was retained by ` + + `policy '${request.tracePolicy}'` + ) + reply({ + requestId: request.requestId, + paths, + ...(paths.length ? {} : { declinedByPolicy: true }) + }) + return + } + // Asked before exporting rather than reported after: a decline is the + // policy working, and routing it through the catch below would log a + // passing run's own success as an export failure. + const decision = retentionDecision(run, request.tracePolicy) + if (!decision.retain) { + log.info( + `Trace for session ${request.sessionId} not retained by policy ` + + `'${request.tracePolicy}'` + + (decision.degradedToFailure + ? ' (no attempt info; degraded to retain-on-failure)' + : '') + ) + reply({ requestId: request.requestId, declinedByPolicy: true }) + return + } + const path = await exportActiveRunTrace(run, { outputDir: request.outputDir, sessionId: request.sessionId, format: request.format, - fileStem: request.fileStem + fileStem: request.fileStem, + tracePolicy: request.tracePolicy }) log.info(`Trace exported for session ${request.sessionId}: ${path}`) reply({ requestId: request.requestId, path }) diff --git a/packages/backend/src/trace-export.ts b/packages/backend/src/trace-export.ts index 17c98c13..2e044b01 100644 --- a/packages/backend/src/trace-export.ts +++ b/packages/backend/src/trace-export.ts @@ -10,14 +10,20 @@ import type { ActionSnapshot, TestMetadataMap, - TraceExportRequest + TraceExportRequest, + TraceRetentionPolicy } from '@wdio/devtools-shared' import { serializeWebSnapshot } from '@wdio/devtools-trace/a11y-snapshot' import { writeTraceZip, type TraceCapturer } from '@wdio/devtools-trace/trace-exporter' +import { + shouldRetainTrace, + type RetentionDecision +} from '@wdio/devtools-trace/trace-retention' import type { ActiveRun, TimeWindowNode } from './baseline/types.js' +import { sliceRunByTest } from './trace-slice.js' /** * Test titles for `Tracing.tracingGroup` events, derived from the suite tree @@ -94,16 +100,42 @@ export function hasExportableData(run: Readonly): boolean { ) } +/** + * Whether this run is worth an archive, by the same rule and the same function + * `core/trace-finalizer.ts` `writeSessionTrace` applies for an in-process + * adapter — one implementation, not one per language. + * + * `attemptInfoAvailable` is false because nothing on the wire carries an + * attempt number: the accumulator's node tree keeps one state per uid, so a + * retried test overwrites its own earlier outcome. The retry-aware policies + * therefore degrade to `retain-on-failure`, which `shouldRetainTrace` reports + * through `degradedToFailure` rather than silently. + */ +export function retentionDecision( + run: Readonly, + policy: TraceRetentionPolicy | undefined +): RetentionDecision { + const outcomes = Array.from(run.nodes.values()) + .filter((node) => node.kind === 'test') + .map((node) => ({ state: node.state })) + return shouldRetainTrace(policy, { outcomes, attemptInfoAvailable: false }) +} + export async function exportActiveRunTrace( run: Readonly, request: Pick< TraceExportRequest, - 'outputDir' | 'sessionId' | 'format' | 'fileStem' + 'outputDir' | 'sessionId' | 'format' | 'fileStem' | 'tracePolicy' > ): Promise { if (!hasExportableData(run)) { throw new Error('nothing captured for this run') } + if (!retentionDecision(run, request.tracePolicy).retain) { + // Distinct from the error above: that one means the run captured nothing, + // this one means it captured a run nobody asked to keep. + throw new Error(`trace not retained by policy '${request.tracePolicy}'`) + } // `capabilities` is not passed separately: writeTraceZip spreads the // capturer's own metadata, which already carries it. return writeTraceZip(toCapturer(run), { @@ -126,3 +158,53 @@ export async function exportActiveRunTrace( testMetadata: testMetadataFromNodes(run.nodes) }) } + +/** Filesystem-safe fragment of a test's identity, for its artifact name. */ +function slugForTest(uid: string, title: string): string { + const base = (title || uid) + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, '') + .slice(0, 60) + // A hash of the UID, not of the title: two tests can share a title (a + // parametrised case), and colliding names would have one overwrite the other. + let hash = 0 + for (let i = 0; i < uid.length; i++) { + hash = (hash * 31 + uid.charCodeAt(i)) | 0 + } + return `${base || 'test'}-${(hash >>> 0).toString(36).slice(0, 6)}` +} + +/** + * One archive per test that the policy retains. + * + * A test whose slice captured nothing is skipped rather than written empty — + * the same rule `hasExportableData` applies to a whole run, for the same + * reason: an empty archive reads in the viewer as a run that captured nothing. + */ +export async function exportPerTestTraces( + run: Readonly, + request: Pick< + TraceExportRequest, + 'outputDir' | 'sessionId' | 'format' | 'tracePolicy' + > +): Promise { + const paths: string[] = [] + for (const slice of sliceRunByTest(run)) { + if (!hasExportableData(slice.run)) { + continue + } + if (!retentionDecision(slice.run, request.tracePolicy).retain) { + continue + } + paths.push( + await exportActiveRunTrace(slice.run, { + outputDir: request.outputDir, + sessionId: request.sessionId, + ...(request.format ? { format: request.format } : {}), + fileStem: `trace-${slugForTest(slice.uid, slice.title)}` + }) + ) + } + return paths +} diff --git a/packages/backend/src/trace-slice.ts b/packages/backend/src/trace-slice.ts new file mode 100644 index 00000000..872895a4 --- /dev/null +++ b/packages/backend/src/trace-slice.ts @@ -0,0 +1,87 @@ +/** + * Split an accumulated run into one slice per test. + * + * `core/spec-trace-helpers.ts` cannot be reused here. It slices by index ranges + * the adapter snapshots at each test boundary — possible only in-process, where + * the code that knows a test started is the same code holding the arrays. The + * backend accumulates asynchronously and learns about boundaries from the suite + * tree, after the fact. + * + * So the split is by content instead: commands carry `testUid`, and every other + * stream is timestamped, so a test's window bounds them. That is the more robust + * half of the trade — an index snapshot taken a moment late silently attributes + * rows to the wrong test, whereas a window is wrong only where tests genuinely + * overlap, which one driver in one process cannot do. + */ + +import type { ActiveRun, TimeWindowNode } from './baseline/types.js' + +/** One test's own view of the run. */ +export interface RunSlice { + uid: string + title: string + /** The node this came from, so retention reads the same state the tree has. */ + node: TimeWindowNode + run: ActiveRun +} + +/** Inclusive at both ends: a command stamped exactly at its test's start or end + * belongs to it, and the boundaries come from the same clock the rows do. */ +function within( + timestamp: number | undefined, + start: number, + end: number +): boolean { + return timestamp !== undefined && timestamp >= start && timestamp <= end +} + +/** + * Every test that reported a usable window, in the order the tree lists them. + * + * A node missing `start` or `end` is skipped rather than given an open window: + * an unbounded slice would swallow the whole run and be written once per test. + */ +export function sliceRunByTest(run: Readonly): RunSlice[] { + const slices: RunSlice[] = [] + for (const node of run.nodes.values()) { + if (node.kind !== 'test') { + continue + } + const { start, end } = node + if (start === undefined || end === undefined || end < start) { + continue + } + // Commands are attributed rather than windowed: the adapter stamped which + // test each one belongs to, which beats inferring it from a clock. + const commands = run.commands.filter((cmd) => cmd.testUid === node.uid) + slices.push({ + uid: node.uid, + title: node.title ?? node.fullTitle ?? node.uid, + node, + run: { + ...run, + commands, + consoleLogs: run.consoleLogs.filter((e) => + within(e.timestamp, start, end) + ), + networkRequests: run.networkRequests.filter((e) => + within(e.startTime ?? e.timestamp, start, end) + ), + mutations: run.mutations.filter((e) => within(e.timestamp, start, end)), + actionSnapshots: run.actionSnapshots.filter((e) => + within(e.timestamp, start, end) + ), + screencastFrames: run.screencastFrames.filter((e) => + within(e.timestamp, start, end) + ), + // Shared, not sliced: the Source tab needs the file a command points at, + // and a run's sources are small next to its frames. + sources: run.sources, + // Only this test, so the slice's own trace carries one group rather + // than opening groups for tests whose rows are not in it. + nodes: new Map([[node.uid, node]]) + } + }) + } + return slices +} diff --git a/packages/backend/tests/trace-export-message.test.ts b/packages/backend/tests/trace-export-message.test.ts index b55bba34..d24b098c 100644 --- a/packages/backend/tests/trace-export-message.test.ts +++ b/packages/backend/tests/trace-export-message.test.ts @@ -178,3 +178,81 @@ describe('runTraceExport', () => { expect(await fs.readdir(outputDir)).toEqual(['trace-sess-4.zip']) }) }) + +describe('a run the policy declines', () => { + function passingRun() { + return run({ + nodes: new Map([ + [ + 't1', + { uid: 't1', kind: 'test' as const, childUids: [], state: 'passed' } + ] + ]) + }) + } + + it('reports a decline, not an error', async () => { + // The distinction is the whole point: a passing run under + // retain-on-failure did exactly what was asked, and an `error` here makes + // the adapter log a broken export for a run that succeeded. + const { replyToWorker, deps: d } = deps(passingRun()) + + await runTraceExport( + { + requestId: 'r1', + outputDir: await tmpDir(), + sessionId: 's1', + tracePolicy: 'retain-on-failure' + }, + d + ) + + const answer = reply(replyToWorker) + expect(answer.data.declinedByPolicy).toBe(true) + expect(answer.data.error).toBeUndefined() + expect(answer.data.path).toBeUndefined() + }) + + it('writes nothing to the output directory', async () => { + const dir = await tmpDir() + const { deps: d } = deps(passingRun()) + + await runTraceExport( + { + requestId: 'r1', + outputDir: dir, + sessionId: 's1', + tracePolicy: 'retain-on-failure' + }, + d + ) + + expect(await fs.readdir(dir)).toEqual([]) + }) + + it('still writes when the policy retains', async () => { + const failing = run({ + nodes: new Map([ + [ + 't1', + { uid: 't1', kind: 'test' as const, childUids: [], state: 'failed' } + ] + ]) + }) + const { replyToWorker, deps: d } = deps(failing) + + await runTraceExport( + { + requestId: 'r1', + outputDir: await tmpDir(), + sessionId: 's1', + tracePolicy: 'retain-on-failure' + }, + d + ) + + const answer = reply(replyToWorker) + expect(answer.data.path).toMatch(/trace-s1\.zip$/) + expect(answer.data.declinedByPolicy).toBeUndefined() + }) +}) diff --git a/packages/backend/tests/trace-export.test.ts b/packages/backend/tests/trace-export.test.ts index 09920095..4926cf50 100644 --- a/packages/backend/tests/trace-export.test.ts +++ b/packages/backend/tests/trace-export.test.ts @@ -20,6 +20,7 @@ import { } from '@wdio/devtools-shared' import { exportActiveRunTrace, + retentionDecision, hasExportableData, testMetadataFromNodes } from '../src/trace-export.js' @@ -168,6 +169,60 @@ describe('hasExportableData', () => { }) }) +describe('retentionDecision', () => { + const failing = run({ + nodes: new Map([ + ['t1', node({ uid: 't1', state: 'passed' })], + ['t2', node({ uid: 't2', state: 'failed' })] + ]) + }) + const passing = run({ + nodes: new Map([['t1', node({ uid: 't1', state: 'passed' })]]) + }) + + it('keeps everything when no policy was asked for', () => { + expect(retentionDecision(passing, undefined).retain).toBe(true) + expect(retentionDecision(passing, 'on').retain).toBe(true) + }) + + it('drops a passing run under retain-on-failure', () => { + expect(retentionDecision(passing, 'retain-on-failure').retain).toBe(false) + expect(retentionDecision(failing, 'retain-on-failure').retain).toBe(true) + }) + + it('reads outcomes from test nodes, not suites', () => { + // A suite rolls its children up, so counting it too would let one failure + // vote twice — harmless for retain-on-failure, wrong for anything counting. + const suiteOnly = run({ + nodes: new Map([ + ['s1', node({ uid: 's1', kind: 'suite', state: 'failed' })] + ]) + }) + + expect(retentionDecision(suiteOnly, 'retain-on-failure').retain).toBe(true) + expect(retentionDecision(suiteOnly, 'retain-on-failure').failOpen).toBe( + true + ) + }) + + it('fails open when the run reported no test outcomes', () => { + // A plain script has no test tree; losing its only artifact to a policy it + // cannot express is worse than keeping one that was not wanted. + const decision = retentionDecision(run(), 'retain-on-failure') + + expect(decision.retain).toBe(true) + expect(decision.failOpen).toBe(true) + }) + + it('degrades the retry-aware policies, and says so', () => { + // Nothing on the wire carries an attempt number. + const decision = retentionDecision(passing, 'retain-on-first-failure') + + expect(decision.retain).toBe(false) + expect(decision.degradedToFailure).toBe(true) + }) +}) + describe('exportActiveRunTrace', () => { it('writes a zip whose actions are the accumulated commands', async () => { const outputDir = await tmpDir() diff --git a/packages/backend/tests/trace-slice.test.ts b/packages/backend/tests/trace-slice.test.ts new file mode 100644 index 00000000..210765d2 --- /dev/null +++ b/packages/backend/tests/trace-slice.test.ts @@ -0,0 +1,118 @@ +/** + * Splitting an accumulated run into per-test slices. + * + * Commands are attributed by `testUid`; every other stream is bounded by the + * test's own window. The cases that matter are the ones where those two + * disagree, and the ones where a window is unusable. + */ + +import { describe, it, expect } from 'vitest' +import { sliceRunByTest } from '../src/trace-slice.js' +import { freshRun } from '../src/baseline/utils.js' +import type { ActiveRun, TimeWindowNode } from '../src/baseline/types.js' + +function node(o: Partial = {}): TimeWindowNode { + return { uid: 't1', kind: 'test', childUids: [], start: 100, end: 200, ...o } +} + +function run(o: Partial = {}): ActiveRun { + return { ...freshRun(), ...o } +} + +const twoTests = new Map([ + ['t1', node({ uid: 't1', title: 'first', start: 100, end: 200 })], + ['t2', node({ uid: 't2', title: 'second', start: 200, end: 300 })] +]) + +describe('sliceRunByTest', () => { + it('gives each test its own commands, by uid rather than by clock', () => { + // The uid is what the adapter stamped; trusting it beats re-deriving + // attribution from timestamps that can tie at a boundary. + const sliced = sliceRunByTest( + run({ + nodes: twoTests, + commands: [ + { command: 'a', args: [], timestamp: 150, testUid: 't1' }, + { command: 'b', args: [], timestamp: 250, testUid: 't2' }, + { command: 'c', args: [], timestamp: 150, testUid: 't2' } + ] + }) + ) + + expect(sliced.map((s) => s.uid)).toEqual(['t1', 't2']) + expect(sliced[0]!.run.commands.map((c) => c.command)).toEqual(['a']) + // `c` is timestamped inside t1's window but belongs to t2. + expect(sliced[1]!.run.commands.map((c) => c.command)).toEqual(['b', 'c']) + }) + + it('drops a command no test claimed', () => { + const sliced = sliceRunByTest( + run({ + nodes: twoTests, + commands: [{ command: 'orphan', args: [], timestamp: 150 }] + }) + ) + + expect(sliced.every((s) => s.run.commands.length === 0)).toBe(true) + }) + + it('bounds the timestamped streams by the test window, inclusively', () => { + const sliced = sliceRunByTest( + run({ + nodes: twoTests, + consoleLogs: [ + { type: 'log', args: [], timestamp: 100, source: 'browser' }, + { type: 'log', args: [], timestamp: 201, source: 'browser' } + ], + mutations: [{ timestamp: 200 } as never, { timestamp: 99 } as never] + }) + ) + + // A row stamped exactly on a boundary belongs to that test. + expect(sliced[0]!.run.consoleLogs.map((c) => c.timestamp)).toEqual([100]) + expect(sliced[0]!.run.mutations).toHaveLength(1) + }) + + it('skips a test with no usable window rather than giving it the whole run', () => { + const sliced = sliceRunByTest( + run({ + nodes: new Map([ + ['t1', node({ uid: 't1', start: undefined })], + ['t2', node({ uid: 't2', end: undefined })], + ['t3', node({ uid: 't3', start: 300, end: 200 })] + ]), + commands: [{ command: 'a', args: [], timestamp: 1, testUid: 't1' }] + }) + ) + + expect(sliced).toEqual([]) + }) + + it('ignores suites', () => { + const sliced = sliceRunByTest( + run({ + nodes: new Map([ + ['s1', node({ uid: 's1', kind: 'suite' })], + ['t1', node({ uid: 't1' })] + ]) + }) + ) + + expect(sliced.map((s) => s.uid)).toEqual(['t1']) + }) + + it('carries one node so the slice opens one group, not the run’s', () => { + const sliced = sliceRunByTest(run({ nodes: twoTests })) + + expect([...sliced[0]!.run.nodes.keys()]).toEqual(['t1']) + }) + + it('shares sources rather than slicing them', () => { + // The Source tab needs the file a command points at; a run's sources are + // small next to its frames. + const sources = { '/a.py': 'print(1)' } + const sliced = sliceRunByTest(run({ nodes: twoTests, sources })) + + expect(sliced[0]!.run.sources).toEqual(sources) + }) +}) diff --git a/packages/core/src/allure-artifacts.ts b/packages/core/src/allure-artifacts.ts index a07d2595..88c80c34 100644 --- a/packages/core/src/allure-artifacts.ts +++ b/packages/core/src/allure-artifacts.ts @@ -23,7 +23,10 @@ import { writeScreenshotArtifact } from './screenshot-artifact.js' import { encodePerTestVideo, sliceFramesFrom } from './video-slice.js' -import { shouldRetainTrace, type TestOutcome } from './trace-retention.js' +import { + shouldRetainTrace, + type TestOutcome +} from '@wdio/devtools-trace/trace-retention' import type { TraceArtifact } from './trace-finalizer.js' /** Adapter-supplied binding to the active Allure reporter. Attaches one file to diff --git a/packages/core/src/attempt-tracker.ts b/packages/core/src/attempt-tracker.ts index 6f6f8bc1..ba7c3e2f 100644 --- a/packages/core/src/attempt-tracker.ts +++ b/packages/core/src/attempt-tracker.ts @@ -1,5 +1,5 @@ import type { TestStatus } from '@wdio/devtools-shared' -import type { TestOutcome } from './trace-retention.js' +import type { TestOutcome } from '@wdio/devtools-trace/trace-retention' /** * Framework-agnostic per-test attempt ledger. Every supported runner re-enters diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index f27c20bc..4c8ecce6 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -35,7 +35,7 @@ export * from '@wdio/devtools-trace/trace-hierarchy' export * from '@wdio/devtools-trace/trace-exporter' export * from './trace-finalizer.js' export * from '@wdio/devtools-trace/trace-frame-snapshots' -export * from './trace-retention.js' +export * from '@wdio/devtools-trace/trace-retention' export * from '@wdio/devtools-trace/trace-sources' export * from '@wdio/devtools-trace/trace-har' export * from '@wdio/devtools-trace/trace-mutations' diff --git a/packages/core/src/trace-finalizer.ts b/packages/core/src/trace-finalizer.ts index 4630fbba..ce79f0ff 100644 --- a/packages/core/src/trace-finalizer.ts +++ b/packages/core/src/trace-finalizer.ts @@ -27,7 +27,10 @@ import { writeTestSliceTrace, type SpecRange } from './spec-trace-helpers.js' -import { shouldRetainTrace, type TestOutcome } from './trace-retention.js' +import { + shouldRetainTrace, + type TestOutcome +} from '@wdio/devtools-trace/trace-retention' import type { RetryOutcomeView } from './attempt-tracker.js' import { writeTraceZip, diff --git a/packages/selenium-devtools-py/README.md b/packages/selenium-devtools-py/README.md index 23b8e768..f6512777 100644 --- a/packages/selenium-devtools-py/README.md +++ b/packages/selenium-devtools-py/README.md @@ -74,6 +74,81 @@ a project default off for one run, which is why there is no `--no-devtools`. itself — it is a mode fallback you may have exported for your own scripts, and reading it as an opt-in would capture pytest runs you never asked for. +### How many archives, and which ones to keep + +Two settings shape the output. `traceGranularity` decides how many archives a +run writes; `tracePolicy` decides which of them survive. + +| `traceGranularity` | | +|---|---| +| `session` | one archive for the whole run (the default) | +| `test` | one per test, holding only that test's own commands, console, network, DOM mutations, a11y trees and frames | + +`spec` is not offered: this adapter's spec **is** its test file, so it could only +silently mean one of the two above. + +| `tracePolicy` | | +|---|---| +| `on` | keep everything (the default) | +| `retain-on-failure` | keep only what failed | + +Together: + +| | | +|---|---| +| `test` + `retain-on-failure` | only the tests that failed | +| `session` + `retain-on-failure` | the whole run, if anything in it failed | +| either + `on` | everything | + +```bash +pytest --devtools-trace-granularity test --devtools-trace-policy retain-on-failure tests/ +``` + +```toml +[tool.pytest.ini_options] +devtools_trace_granularity = "test" +devtools_trace_policy = "retain-on-failure" +``` + +A plain script passes +`devtools.enable(trace_granularity="test", trace_policy="retain-on-failure")`. +A committed example of all of this is in +[`examples/selenium/python-test/trace-py-test/`](../../examples/selenium/python-test/trace-py-test/), +whose `pytest.ini` documents every setting the adapter has. + +Naming a policy or a granularity **explicitly** selects trace mode — the CLI flag, the ini option +and the `enable()` argument all imply it, since a policy means nothing in live +mode. `DEVTOOLS_TRACE_POLICY` and `DEVTOOLS_TRACE_GRANULARITY` deliberately do +**not**: an exported variable is +ambient, and may have been set for a different script in the same shell, so +flipping a live run to trace mode on that basis would take away the dashboard +nobody asked to lose. Pair it with `DEVTOOLS_TRACE=1`. A run that ignores it +says so rather than leaving you to notice a missing archive. + +The values are shared's `TraceRetentionPolicy`: `on` (the default — keep +everything), `retain-on-failure`, `retain-on-first-failure`, `on-first-retry`, +`on-all-retries`, `retain-on-failure-and-retries`. A value outside that set +warns and keeps everything, rather than being discovered as a missing file. + +Two limits: + +- At `session` granularity the decision covers **the whole run** — one failing + test keeps everything, because there is only one archive to keep. Use + `test` granularity if you want only the failure. +- The retry-aware policies — `retain-on-first-failure`, `on-first-retry`, + `on-all-retries`, `retain-on-failure-and-retries` — **behave exactly like + `retain-on-failure`**. Nothing on the wire carries an attempt number, so a + retried test overwrites its own earlier outcome and the retry-aware question + cannot be asked; the backend logs the degradation rather than pretending + otherwise. + +Per-test `screenshot`, `video` and inline Allure attachment are still +Node.js-only — it is the trace **archive** that is now per-test, not the other +artifacts. + +A declined run is reported as the policy working, not as a failed export. + + The bundled plugin auto-captures the run, opens the dashboard in a dedicated window, and — after the run — **keeps it open so you can inspect it**; close the window (or Ctrl-C) to finish. Nothing devtools-specific goes in your test files. diff --git a/packages/selenium-devtools-py/src/selenium_devtools/__init__.py b/packages/selenium-devtools-py/src/selenium_devtools/__init__.py index b36cda99..49b63c87 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/__init__.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/__init__.py @@ -34,6 +34,8 @@ from .output_dir import resolve_adapter_output_dir from .run_id import reset_run_id, resolve_run_id from .constants import ( + TRACE_RETENTION_POLICIES, + TRACE_GRANULARITIES, DEFAULT_HOST, DEFAULT_PORT, ENV_HOST, @@ -41,6 +43,8 @@ ENV_FILMSTRIP, ENV_PORT, ENV_TRACE, + ENV_TRACE_POLICY, + ENV_TRACE_GRANULARITY, LOGGER_NAME, ) from .logcapture import LogCapturer @@ -85,6 +89,7 @@ def _restore_excepthook() -> None: "capturer": None, "transport": None, "process": None, "url": None, "handle": None, "terminal": None, "logs": None, "excepthook": None, "trace": False, "traced": False, "filmstrip_mark": None, + "trace_policy": None, "trace_granularity": None, } @@ -100,6 +105,98 @@ def _filmstrip_enabled(filmstrip: Optional[bool]) -> bool: return value.lower() not in ("0", "false", "no", "off", "") +def _resolve_trace_settings( + trace: Optional[bool], + filmstrip: Optional[bool], + a11y: Optional[bool], + trace_policy: Optional[str], + trace_granularity: Optional[str], +) -> tuple: + """Every trace decision in one place: ``(mode, filmstrip, a11y, policy, + granularity)``. + + Extracted from `enable()` so the wiring is testable on its own. The rules it + encodes — that an explicit policy or granularity implies the mode, that an + exported one does not, and that nothing is dropped silently — were each + reachable only through a live `enable()` before, so a mutation to any of + them left every test green. + """ + mode = _trace_enabled(trace, implied_by=(trace_policy, trace_granularity)) + if not mode: + _warn_if_ignored() + return False, False, False, None, None + return ( + True, + _filmstrip_enabled(filmstrip), + _a11y_enabled(a11y), + _trace_policy(trace_policy), + _trace_granularity(trace_granularity), + ) + + +def _warn_if_ignored() -> None: + """Say so when a trace setting was exported but this run is not tracing. + + An ARGUMENT naming a policy turns trace mode on; an exported variable does + not, because it is ambient — it may have been set for a different script in + the same shell, and flipping a live run to trace mode on that basis would + take away the dashboard nobody asked to lose. But it must not be silent: + the symptom is otherwise an archive that never appears. + """ + for name in (ENV_TRACE_POLICY, ENV_TRACE_GRANULARITY): + if os.environ.get(name): + _log.warning( + "%s is set but this run is not in trace mode, so it does " + "nothing. Add %s=1, or pass trace=True to enable().", + name, + ENV_TRACE, + ) + + +def _trace_granularity(granularity: Optional[str]) -> Optional[str]: + """One archive per run (`session`, the default) or one per test (`test`). + + Validated here for the same reason the policy is: the backend falls back to + session granularity for anything it does not recognise, which is the right + runtime behaviour and an invisible way to lose per-test archives. + """ + value = granularity if granularity is not None else os.environ.get( + ENV_TRACE_GRANULARITY + ) + if not value: + return None + if value not in TRACE_GRANULARITIES: + _log.warning( + "unknown trace granularity %r; writing one archive for the run. " + "Expected one of: %s", + value, + ", ".join(sorted(TRACE_GRANULARITIES)), + ) + return None + return value + + +def _trace_policy(policy: Optional[str]) -> Optional[str]: + """Which runs are worth an archive. Default: keep every one. + + Validated here rather than at the backend, so a typo says so on the machine + that made it instead of silently keeping everything — `shouldRetainTrace` + treats an unknown policy as `on`, which is the right runtime behaviour and + the wrong thing to discover from a missing artifact. + """ + value = policy if policy is not None else os.environ.get(ENV_TRACE_POLICY) + if not value: + return None + if value not in TRACE_RETENTION_POLICIES: + _log.warning( + "unknown trace policy %r; keeping every run's archive. Expected one of: %s", + value, + ", ".join(sorted(TRACE_RETENTION_POLICIES)), + ) + return None + return value + + def _a11y_enabled(a11y: Optional[bool]) -> bool: """Whether trace mode captures the per-action element tree. Default ON — the A11y tab is empty without it — opt out with DEVTOOLS_A11Y=0. Two extra @@ -112,11 +209,19 @@ def _a11y_enabled(a11y: Optional[bool]) -> bool: return value.lower() not in ("0", "false", "no", "off", "") -def _trace_enabled(trace: Optional[bool]) -> bool: +def _trace_enabled( + trace: Optional[bool], *, implied_by: tuple = () +) -> bool: """Whether this run writes a trace archive. The argument wins over the environment so a script can opt out of an exported default.""" if trace is not None: return trace + # An explicit policy or granularity ARGUMENT is a request for trace mode: + # neither means anything in live mode, so honouring one without it would + # silently drop what the caller asked for. Same rule as the CLI flags, and + # deliberately not extended to the environment — see `_warn_if_ignored`. + if any(implied_by): + return True return os.environ.get(ENV_TRACE, "").lower() in ("1", "true", "yes") @@ -224,6 +329,8 @@ def _export_trace( _active["transport"], output_dir=output_dir or resolve_adapter_output_dir(), session_id=session_id, + trace_policy=_active["trace_policy"], + trace_granularity=_active["trace_granularity"], ) except Exception as exc: # noqa: BLE001 _log.warning("trace export skipped (%s)", exc) @@ -238,6 +345,8 @@ def enable( trace: Optional[bool] = None, filmstrip: Optional[bool] = None, a11y: Optional[bool] = None, + trace_policy: Optional[str] = None, + trace_granularity: Optional[str] = None, ) -> Optional[SessionCapturer]: """Connect to the backend and instrument Selenium. Idempotent. @@ -251,9 +360,11 @@ def enable( # Decided before anything reads it: the screencast recorder, the dashboard # window and the teardown export all branch on this. - trace_mode = _trace_enabled(trace) - filmstrip_mode = trace_mode and _filmstrip_enabled(filmstrip) - a11y_mode = trace_mode and _a11y_enabled(a11y) + trace_mode, filmstrip_mode, a11y_mode, policy, granularity = ( + _resolve_trace_settings( + trace, filmstrip, a11y, trace_policy, trace_granularity + ) + ) # Before the backend is launched: the directory a rerun spawns in travels # through the environment the backend process inherits. A framework plugin @@ -314,6 +425,7 @@ def enable( _active.update( capturer=capturer, transport=transport, process=process, url=url, terminal=term, logs=logs, trace=trace_mode, traced=False, filmstrip_mark=None, + trace_policy=policy, trace_granularity=granularity, ) # Open the dashboard window and wire exit/signal + control-frame teardown so @@ -377,6 +489,7 @@ def disable() -> None: _active.update( capturer=None, transport=None, process=None, url=None, handle=None, terminal=None, logs=None, excepthook=None, trace=False, traced=False, filmstrip_mark=None, + trace_policy=None, trace_granularity=None, ) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/capturer.py b/packages/selenium-devtools-py/src/selenium_devtools/capturer.py index e705156d..4fe0d9d8 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/capturer.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/capturer.py @@ -41,6 +41,10 @@ def __init__(self, transport: Transport) -> None: self._lock = threading.Lock() self._metadata_sent: set = set() # session ids already announced self.session_id: Optional[str] = None + #: The test every command captured from now on belongs to. Set by + #: whichever runner integration knows about tests; None for a plain + #: script, whose commands belong to the run rather than to a test. + self.test_uid: Optional[str] = None # ── metadata ─────────────────────────────────────────────────────────────── @@ -99,6 +103,7 @@ def capture_command( command_id=command_id, screenshot=screenshot, selector=selector, + test_uid=self.test_uid, ) self._tx.send_json(SCOPE_COMMANDS, [entry]) # Returned so a caller that learns more about the command AFTER it was diff --git a/packages/selenium-devtools-py/src/selenium_devtools/constants.py b/packages/selenium-devtools-py/src/selenium_devtools/constants.py index 4ee79f21..925704b2 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/constants.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/constants.py @@ -96,6 +96,32 @@ #: Opt OUT of per-action element capture in trace mode (on by default). ENV_A11Y = "DEVTOOLS_A11Y" +#: Which runs are worth an archive. Unset keeps every one. +ENV_TRACE_POLICY = "DEVTOOLS_TRACE_POLICY" + +#: One archive per run, or one per test. Unset means per run. +ENV_TRACE_GRANULARITY = "DEVTOOLS_TRACE_GRANULARITY" + +# `spec` is deliberately absent: this adapter's spec IS its test file, so it +# would have to behave as one of the other two, and a granularity that silently +# means something else is worse than one that is not offered. +TRACE_GRANULARITIES = frozenset({"session", "test"}) + +# Mirrors shared's `TraceRetentionPolicy`. Validated adapter-side so a typo is +# reported where it was made: `shouldRetainTrace` treats an unknown policy as +# "keep everything", which is the right runtime behaviour and the wrong thing +# to learn about from a missing file. +TRACE_RETENTION_POLICIES = frozenset( + { + "on", + "retain-on-failure", + "retain-on-first-failure", + "on-first-retry", + "on-all-retries", + "retain-on-failure-and-retries", + } +) + #: Filmstrip frames per websocket message. The buffer can hold #: SCREENCAST_MAX_BUFFER_FRAMES JPEGs, which in one message would approach the #: socket's payload limit; the transport also masks payloads in a per-byte diff --git a/packages/selenium-devtools-py/src/selenium_devtools/frames.py b/packages/selenium-devtools-py/src/selenium_devtools/frames.py index 99f1ce00..c0f2cd65 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/frames.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/frames.py @@ -69,6 +69,7 @@ def command_log( command_id: int, screenshot: Optional[str] = None, selector: Optional[str] = None, + test_uid: Optional[str] = None, ) -> CommandLog: entry: CommandLog = { "command": command, @@ -81,6 +82,10 @@ def command_log( } if selector: entry["selector"] = selector + # What the exporter groups a trace by: absent, `buildGroupPath` returns an + # empty path and the archive carries no test boundaries at all. + if test_uid: + entry["testUid"] = test_uid if error is not None: entry["error"] = { "name": type(error).__name__, diff --git a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py index 54d04e3b..805cdf80 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py @@ -171,8 +171,13 @@ def _send_default_suite(capturer: SessionCapturer, state: str) -> None: end = start if state == "running" else now_ms() # Named the way the pytest plugin names a real test: the file is the suite, # the test carries its own name, and fullTitle joins the two. + test_uid = f"{entry or suite_title}::{DEFAULT_TEST_TITLE}" + # The synthetic test is what a script's commands belong to, so the trace + # gets one group rather than none — the same uid the tree reports, or the + # exporter would open a group the viewer cannot name. + capturer.test_uid = test_uid test = frames.test_stats( - uid=f"{entry or suite_title}::{DEFAULT_TEST_TITLE}", + uid=test_uid, title=DEFAULT_TEST_TITLE, full_title=f"{suite_title} › {DEFAULT_TEST_TITLE}", parent=suite_title, state=state, file=entry, start_ms=start, end_ms=end, diff --git a/packages/selenium-devtools-py/src/selenium_devtools/pytest_plugin.py b/packages/selenium-devtools-py/src/selenium_devtools/pytest_plugin.py index aa3d0043..dbbbb94d 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/pytest_plugin.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/pytest_plugin.py @@ -56,6 +56,17 @@ def pytest_addoption(parser) -> None: # noqa: ANN001 default=None, help="Write a trace archive instead of opening a dashboard. Implies --devtools.", ) + group.addoption( + "--devtools-trace-granularity", + default=None, + choices=("session", "test"), + help="One trace archive per run (default) or one per test. Implies --devtools-trace.", + ) + group.addoption( + "--devtools-trace-policy", + default=None, + help="Which runs keep their trace archive. Implies --devtools-trace.", + ) parser.addini( "devtools", "Capture pytest runs for the DevTools dashboard.", @@ -68,15 +79,57 @@ def pytest_addoption(parser) -> None: # noqa: ANN001 type="bool", default=None, ) + parser.addini( + "devtools_trace_policy", + "Which runs keep their trace archive. Implies devtools_trace.", + default=None, + ) + parser.addini( + "devtools_trace_granularity", + "One trace archive per run, or one per test. Implies devtools_trace.", + default=None, + ) -def _ini(config, name: str) -> Optional[bool]: # noqa: ANN001 - """An ini option's value, or None when the project did not set it.""" +def _ini_raw(config, name: str): # noqa: ANN001, ANN201 + """An ini option exactly as the project wrote it, or None when unset.""" try: value = config.getini(name) except (ValueError, KeyError): # option not registered (another plugin's parser) return None - return None if value is None or value == "" else bool(value) + return None if value is None or value == "" else value + + +def _ini(config, name: str) -> Optional[bool]: # noqa: ANN001 + """A BOOLEAN ini option's value, or None when the project did not set it. + + Only for options registered `type="bool"`. A string option read through here + would come back as `True` rather than its value. + """ + value = _ini_raw(config, name) + return None if value is None else bool(value) + + +def _resolve_trace_granularity(config) -> Optional[str]: # noqa: ANN001 + """One archive per run or per test: CLI, else ini, else undecided.""" + cli = config.getoption("--devtools-trace-granularity", None) + if cli: + return str(cli) + value = _ini_raw(config, "devtools_trace_granularity") + return str(value) if value else None + + +def _resolve_trace_policy(config) -> Optional[str]: # noqa: ANN001 + """Which runs keep their archive: CLI, else ini, else undecided. + + None rather than a default, so `enable()` still reads DEVTOOLS_TRACE_POLICY + and validates the value in one place instead of two. + """ + cli = config.getoption("--devtools-trace-policy", None) + if cli: + return str(cli) + value = _ini_raw(config, "devtools_trace_policy") + return str(value) if value else None def _resolve_trace(config) -> Optional[bool]: # noqa: ANN001 @@ -85,7 +138,13 @@ def _resolve_trace(config) -> Optional[bool]: # noqa: ANN001 None rather than False when nothing said, so `enable()` still reads DEVTOOLS_TRACE — the env layer lives there and is not duplicated here. """ - if config.getoption("--devtools-trace", None): + if ( + config.getoption("--devtools-trace", None) + or _resolve_trace_policy(config) + or _resolve_trace_granularity(config) + ): + # A policy or a granularity only means anything in trace mode, so naming + # either selects it rather than being silently ignored. return True return _ini(config, "devtools_trace") @@ -103,8 +162,15 @@ def _resolve_enabled(config) -> bool: # noqa: ANN001 # sitting empty for a run that never happened. if config.getoption("--collect-only", False): return False - if config.getoption("--devtools", None) or config.getoption( - "--devtools-trace", None + if ( + config.getoption("--devtools", None) + or config.getoption("--devtools-trace", None) + or config.getoption("--devtools-trace-policy", None) + or config.getoption("--devtools-trace-granularity", None) + ): + return True + if _ini_raw(config, "devtools_trace_policy") or _ini_raw( + config, "devtools_trace_granularity" ): return True for name in ("devtools", "devtools_trace"): @@ -366,7 +432,11 @@ def pytest_configure(config) -> None: # noqa: ANN001 root = getattr(config, "rootpath", None) or getattr(config, "rootdir", None) _rootdir = str(root) if root else None _configure_rerun(config, _rootdir) - capturer = devtools.enable(trace=_resolve_trace(config)) + capturer = devtools.enable( + trace=_resolve_trace(config), + trace_policy=_resolve_trace_policy(config), + trace_granularity=_resolve_trace_granularity(config), + ) # pytest owns the suite tree — suppress the adapter's default script suite. from . import instrumentation @@ -464,6 +534,10 @@ def pytest_runtest_logstart(nodeid, location) -> None: # noqa: ANN001 capturer = devtools.get_capturer() if capturer is None: return + # Every command captured from here belongs to this test. Without it the + # exporter's `buildGroupPath` returns an empty path and the archive carries + # no test boundaries — 22 rows and no way to tell which test failed. + capturer.test_uid = nodeid # Flip this one to running so the tree shows WHICH test is executing, not # just that something is. file, line, name = location diff --git a/packages/selenium-devtools-py/src/selenium_devtools/trace_export.py b/packages/selenium-devtools-py/src/selenium_devtools/trace_export.py index 7b5ff744..44b0aeba 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/trace_export.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/trace_export.py @@ -25,7 +25,7 @@ import logging import threading import uuid -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any, Optional from ._contract import ( @@ -53,6 +53,10 @@ class _Pending: done: threading.Event path: Optional[str] = None error: Optional[str] = None + declined: bool = False + #: Per-test archives, at `test` granularity. The run writes several files, + #: so the single `path` cannot carry the answer. + paths: list = field(default_factory=list) _pending: Optional[_Pending] = None @@ -76,6 +80,9 @@ def on_result(data: Any) -> None: error = data.get("error") pending.path = path if isinstance(path, str) else None pending.error = error if isinstance(error, str) else None + pending.declined = bool(data.get("declinedByPolicy")) + many = data.get("paths") + pending.paths = [p for p in many if isinstance(p, str)] if isinstance(many, list) else [] pending.done.set() @@ -146,6 +153,8 @@ def export( *, output_dir: str, session_id: str, + trace_policy: Optional[str] = None, + trace_granularity: Optional[str] = None, timeout: float = TRACE_EXPORT_TIMEOUT_S, ) -> Optional[str]: """Request the archive and block until the backend answers. @@ -172,6 +181,14 @@ def export( "requestId": pending.request_id, "outputDir": output_dir, "sessionId": session_id, + # Omitted when unset so the backend's own default ('on', keep + # everything) applies rather than a null it has to interpret. + **({"tracePolicy": trace_policy} if trace_policy else {}), + **( + {"traceGranularity": trace_granularity} + if trace_granularity + else {} + ), }, ) except Exception as exc: # noqa: BLE001 — a failed export is not a failed run @@ -190,9 +207,20 @@ def export( return None reset() + if pending.declined: + # The run captured fine and the policy decided against keeping it. + _log.info("trace not retained by policy '%s'", trace_policy) + return None if pending.error: _log.warning("trace export failed: %s", pending.error) return None + if pending.paths: + _log.info( + "%d per-test trace(s) written to %s", + len(pending.paths), + output_dir, + ) + return pending.paths[0] if pending.path: _log.info("trace written to %s", pending.path) return pending.path diff --git a/packages/selenium-devtools-py/src/selenium_devtools/types.py b/packages/selenium-devtools-py/src/selenium_devtools/types.py index 0d18ebcc..82a421a3 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/types.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/types.py @@ -34,6 +34,7 @@ class CommandLog(TypedDict, total=False): id: int screenshot: str selector: str + testUid: str class Viewport(TypedDict): diff --git a/packages/selenium-devtools-py/tests/test_pytest_config.py b/packages/selenium-devtools-py/tests/test_pytest_config.py index 528ec096..c0198b29 100644 --- a/packages/selenium-devtools-py/tests/test_pytest_config.py +++ b/packages/selenium-devtools-py/tests/test_pytest_config.py @@ -132,6 +132,58 @@ def test_an_ini_that_says_no_is_an_answer_not_a_shrug(self): self.assertIs(plugin._resolve_trace(_Config(ini={"devtools_trace": False})), False) +class ResolveTracePolicyTest(unittest.TestCase): + """A retention policy is a string, so it must not go through the boolean + ini reader — that would hand `enable()` the word "True".""" + + def test_undecided_is_none_so_enable_reads_the_environment(self): + self.assertIsNone(plugin._resolve_trace_policy(_Config())) + + def test_the_cli_flag_carries_the_value(self): + config = _Config({"--devtools-trace-policy": "retain-on-failure"}) + self.assertEqual( + plugin._resolve_trace_policy(config), "retain-on-failure" + ) + + def test_the_ini_option_carries_the_value_not_a_bool(self): + config = _Config(ini={"devtools_trace_policy": "retain-on-failure"}) + self.assertEqual( + plugin._resolve_trace_policy(config), "retain-on-failure" + ) + + def test_naming_a_policy_selects_trace_mode(self): + # It means nothing in live mode, so it selects the mode rather than + # being silently ignored. + for source in ( + _Config({"--devtools-trace-policy": "retain-on-failure"}), + _Config(ini={"devtools_trace_policy": "retain-on-failure"}), + ): + with self.subTest(source=source): + self.assertTrue(plugin._resolve_trace(source)) + self.assertTrue(plugin._resolve_enabled(source)) + + def test_granularity_also_selects_trace_mode_and_opts_in(self): + for source in ( + _Config({"--devtools-trace-granularity": "test"}), + _Config(ini={"devtools_trace_granularity": "test"}), + ): + with self.subTest(source=source): + self.assertEqual( + plugin._resolve_trace_granularity(source), "test" + ) + self.assertTrue(plugin._resolve_trace(source)) + self.assertTrue(plugin._resolve_enabled(source)) + + def test_an_unset_granularity_is_none(self): + self.assertIsNone(plugin._resolve_trace_granularity(_Config())) + + def test_collect_only_still_wins(self): + config = _Config( + {"--devtools-trace-policy": "retain-on-failure", "--collect-only": True} + ) + self.assertFalse(plugin._resolve_enabled(config)) + + class ConfigureResolvesOnceTest(unittest.TestCase): """Most hooks are handed no `config`, so the answer has to be cached.""" @@ -150,6 +202,24 @@ def test_opted_in_reports_what_configure_resolved(self): self.assertTrue(plugin._opted_in()) + def test_the_policy_reaches_enable(self): + with mock.patch.object(plugin, "_resolve_enabled", return_value=True), \ + mock.patch.object(plugin, "_resolve_trace", return_value=True), \ + mock.patch.object( + plugin, "_resolve_trace_policy", return_value="retain-on-failure" + ), \ + mock.patch.object(plugin, "_enable_assertion_pass_hook"), \ + mock.patch.object(plugin, "_configure_rerun"), \ + mock.patch.object(plugin.devtools, "enable", return_value=None) as enable, \ + mock.patch.object(plugin.devtools, "dashboard_url", return_value=None): + plugin.pytest_configure(_Config()) + + enable.assert_called_once_with( + trace=True, + trace_policy="retain-on-failure", + trace_granularity=None, + ) + def test_a_run_that_did_not_opt_in_leaves_every_hook_inert(self): plugin._enabled = True with mock.patch.object(plugin, "_resolve_enabled", return_value=False), \ @@ -168,7 +238,9 @@ def test_the_resolved_mode_is_what_enable_is_asked_for(self): mock.patch.object(plugin.devtools, "dashboard_url", return_value=None): plugin.pytest_configure(_Config()) - enable.assert_called_once_with(trace=True) + enable.assert_called_once_with( + trace=True, trace_policy=None, trace_granularity=None + ) class EmptyRunTest(unittest.TestCase): diff --git a/packages/selenium-devtools-py/tests/test_trace_export.py b/packages/selenium-devtools-py/tests/test_trace_export.py index 68259388..60030684 100644 --- a/packages/selenium-devtools-py/tests/test_trace_export.py +++ b/packages/selenium-devtools-py/tests/test_trace_export.py @@ -6,12 +6,15 @@ export, the backend can refuse, and none of it may take the run down. """ +import os import threading import time import unittest from unittest import mock +import selenium_devtools as devtools from selenium_devtools import lifecycle, trace_export +from selenium_devtools.constants import TRACE_RETENTION_POLICIES from selenium_devtools._contract import SCOPE_TRACE_EXPORT, SCOPE_TRACE_EXPORTED @@ -588,3 +591,213 @@ def off_thread_teardown(): self.assertTrue( returned, "off-thread export blocked on the in-flight one" ) + + +class TracePolicyTest(unittest.TestCase): + """Validated adapter-side: `shouldRetainTrace` treats an unknown policy as + "keep everything", which is right at runtime and the wrong way to learn you + made a typo.""" + + def setUp(self): + patcher = mock.patch.dict(os.environ, {}, clear=False) + patcher.start() + self.addCleanup(patcher.stop) + os.environ.pop("DEVTOOLS_TRACE_POLICY", None) + + def test_unset_keeps_every_run(self): + self.assertIsNone(devtools._trace_policy(None)) + + def test_an_argument_is_taken_as_given(self): + self.assertEqual( + devtools._trace_policy("retain-on-failure"), "retain-on-failure" + ) + + def test_the_environment_is_the_fallback(self): + os.environ["DEVTOOLS_TRACE_POLICY"] = "retain-on-first-failure" + self.assertEqual(devtools._trace_policy(None), "retain-on-first-failure") + + def test_the_argument_beats_the_environment(self): + os.environ["DEVTOOLS_TRACE_POLICY"] = "retain-on-failure" + self.assertEqual(devtools._trace_policy("on"), "on") + + def test_every_shared_policy_is_accepted(self): + for policy in TRACE_RETENTION_POLICIES: + with self.subTest(policy=policy): + self.assertEqual(devtools._trace_policy(policy), policy) + + def test_a_typo_warns_and_keeps_everything(self): + with self.assertLogs("selenium_devtools", level="WARNING") as logs: + self.assertIsNone(devtools._trace_policy("retain-on-failures")) + + self.assertIn("retain-on-failures", "\n".join(logs.output)) + + +class DeclinedExportTest(unittest.TestCase): + """A policy decline is the feature working. Reported as an error it reads + as a broken export on a run that passed.""" + + def tearDown(self): + trace_export.reset() + + def test_a_decline_is_not_an_error(self): + tx = FakeTransport(reply={"declinedByPolicy": True}, delay=0.05) + + with self.assertLogs("selenium_devtools", level="INFO") as logs: + path = trace_export.export( + tx, output_dir="/out", session_id="s", + trace_policy="retain-on-failure", + ) + + self.assertIsNone(path) + joined = "\n".join(logs.output) + self.assertIn("not retained", joined) + self.assertNotIn("export failed", joined) + + def test_the_policy_travels_with_the_request(self): + tx = FakeTransport(reply={"path": "/out/t.zip"}) + + trace_export.export( + tx, output_dir="/out", session_id="s", + trace_policy="retain-on-failure", + ) + + self.assertEqual(tx.sent[0][1]["tracePolicy"], "retain-on-failure") + + def test_no_policy_sends_no_field(self): + # Absent rather than null: the backend's own default applies. + tx = FakeTransport(reply={"path": "/out/t.zip"}) + + trace_export.export(tx, output_dir="/out", session_id="s") + + self.assertNotIn("tracePolicy", tx.sent[0][1]) + + +class TraceModeGatingTest(unittest.TestCase): + """A policy or a granularity means nothing in live mode. Which of the four + ways of naming one turns trace mode ON is the question this pins.""" + + def setUp(self): + patcher = mock.patch.dict(os.environ, {}, clear=False) + patcher.start() + self.addCleanup(patcher.stop) + for name in ("DEVTOOLS_TRACE", "DEVTOOLS_TRACE_POLICY", + "DEVTOOLS_TRACE_GRANULARITY"): + os.environ.pop(name, None) + + def test_an_explicit_argument_selects_trace_mode(self): + # Same rule as --devtools-trace-policy: honouring a policy without the + # mode would silently drop what the caller asked for. + self.assertTrue( + devtools._trace_enabled(None, implied_by=("retain-on-failure", None)) + ) + self.assertTrue(devtools._trace_enabled(None, implied_by=(None, "test"))) + + def test_an_explicit_trace_false_still_wins(self): + self.assertFalse( + devtools._trace_enabled(False, implied_by=("retain-on-failure", "test")) + ) + + def test_the_environment_stays_ambient(self): + # It may have been exported for a different script in the same shell; + # flipping a live run to trace mode would take away the dashboard. + os.environ["DEVTOOLS_TRACE_POLICY"] = "retain-on-failure" + os.environ["DEVTOOLS_TRACE_GRANULARITY"] = "test" + + self.assertFalse(devtools._trace_enabled(None)) + + def test_but_an_ignored_environment_setting_is_never_silent(self): + # The symptom is otherwise an archive that never appears. + for name in ("DEVTOOLS_TRACE_POLICY", "DEVTOOLS_TRACE_GRANULARITY"): + with self.subTest(env=name): + os.environ.pop("DEVTOOLS_TRACE_POLICY", None) + os.environ.pop("DEVTOOLS_TRACE_GRANULARITY", None) + os.environ[name] = "retain-on-failure" + with self.assertLogs("selenium_devtools", level="WARNING") as logs: + devtools._warn_if_ignored() + self.assertIn(name, "\n".join(logs.output)) + + def test_nothing_is_said_when_nothing_was_set(self): + with self.assertNoLogs("selenium_devtools", level="WARNING"): + devtools._warn_if_ignored() + + def test_the_environment_works_alongside_the_mode(self): + os.environ["DEVTOOLS_TRACE"] = "1" + os.environ["DEVTOOLS_TRACE_POLICY"] = "retain-on-failure" + + self.assertTrue(devtools._trace_enabled(None)) + self.assertEqual(devtools._trace_policy(None), "retain-on-failure") + + +class ResolveTraceSettingsTest(unittest.TestCase): + """The wiring, not the helpers. Testing the helpers alone left mutations to + `enable()`'s own decisions completely undetected.""" + + def setUp(self): + patcher = mock.patch.dict(os.environ, {}, clear=False) + patcher.start() + self.addCleanup(patcher.stop) + for name in ("DEVTOOLS_TRACE", "DEVTOOLS_TRACE_POLICY", + "DEVTOOLS_TRACE_GRANULARITY", "DEVTOOLS_FILMSTRIP", + "DEVTOOLS_A11Y"): + os.environ.pop(name, None) + + def resolve(self, **kw): + args = {"trace": None, "filmstrip": None, "a11y": None, + "trace_policy": None, "trace_granularity": None} + args.update(kw) + return devtools._resolve_trace_settings(**args) + + def test_live_mode_carries_no_trace_settings_at_all(self): + # Every trace-only feature is off, not merely unused. + self.assertEqual(self.resolve(), (False, False, False, None, None)) + + def test_trace_mode_defaults_the_captures_on_and_the_policy_off(self): + mode, filmstrip, a11y, policy, gran = self.resolve(trace=True) + + self.assertEqual((mode, filmstrip, a11y), (True, True, True)) + self.assertEqual((policy, gran), (None, None)) + + def test_a_policy_argument_turns_the_mode_on_and_is_kept(self): + mode, _, _, policy, _ = self.resolve(trace_policy="retain-on-failure") + + self.assertTrue(mode) + self.assertEqual(policy, "retain-on-failure") + + def test_a_granularity_argument_does_the_same(self): + mode, _, _, _, gran = self.resolve(trace_granularity="test") + + self.assertTrue(mode) + self.assertEqual(gran, "test") + + def test_trace_false_beats_a_policy_and_drops_it(self): + self.assertEqual( + self.resolve(trace=False, trace_policy="retain-on-failure"), + (False, False, False, None, None), + ) + + def test_an_exported_policy_does_not_turn_the_mode_on(self): + os.environ["DEVTOOLS_TRACE_POLICY"] = "retain-on-failure" + + mode, _, _, policy, _ = self.resolve() + + self.assertFalse(mode) + self.assertIsNone(policy) + + def test_and_says_so_rather_than_dropping_it_silently(self): + os.environ["DEVTOOLS_TRACE_GRANULARITY"] = "test" + + with self.assertLogs("selenium_devtools", level="WARNING") as logs: + self.resolve() + + self.assertIn("DEVTOOLS_TRACE_GRANULARITY", "\n".join(logs.output)) + + def test_an_exported_policy_applies_once_the_mode_is_on(self): + os.environ["DEVTOOLS_TRACE_POLICY"] = "retain-on-failure" + + _, _, _, policy, _ = self.resolve(trace=True) + + self.assertEqual(policy, "retain-on-failure") + + def test_a_live_run_says_nothing_when_nothing_was_exported(self): + with self.assertNoLogs("selenium_devtools", level="WARNING"): + self.resolve() diff --git a/packages/shared/src/trace-export.ts b/packages/shared/src/trace-export.ts index 6a12622a..5c341125 100644 --- a/packages/shared/src/trace-export.ts +++ b/packages/shared/src/trace-export.ts @@ -15,7 +15,7 @@ * reachable from any page the browser has open. */ -import type { TraceFormat } from './types.js' +import type { TraceFormat, TraceRetentionPolicy } from './types.js' export const TRACE_EXPORT_SCOPE = { /** Worker → backend: build an artifact from the accumulated run. */ @@ -40,15 +40,33 @@ export interface TraceExportRequest { format?: TraceFormat /** Artifact base name. Defaults to `trace-`. */ fileStem?: string + /** Which runs are worth keeping. Evaluated against the run's own test + * outcomes, exactly as `writeSessionTrace` does for an in-process adapter — + * an unretained run writes nothing rather than writing and deleting. */ + tracePolicy?: TraceRetentionPolicy + /** `session` (default) writes one archive for the run; `test` writes one per + * test, and `tracePolicy` is then evaluated per test rather than run-wide. + * `spec` is deliberately absent: this adapter's spec IS its test file, and a + * granularity that silently behaved like one of the other two would be worse + * than not offering it. */ + traceGranularity?: 'session' | 'test' } /** Payload sent under {@link TRACE_EXPORT_SCOPE.result}. Exactly one of * `path` / `error` is set. */ export interface TraceExportResult { requestId: string - /** Absolute path of the artifact written. */ + /** Absolute path of the artifact written. Set for a session-granularity + * export; per-test exports report `paths` instead. */ path?: string + /** Absolute paths written at `test` granularity, one per RETAINED test. + * Empty when the policy declined every one. */ + paths?: string[] /** Why nothing was written. The adapter logs this; a failed export must not * fail the user's test run. */ error?: string + /** Set when the run captured fine and `tracePolicy` decided against keeping + * it. Distinct from `error`: nothing went wrong, so an adapter reports it as + * the policy working rather than as a failure. */ + declinedByPolicy?: boolean } diff --git a/packages/trace/src/index.ts b/packages/trace/src/index.ts index b3361747..c3aa025f 100644 --- a/packages/trace/src/index.ts +++ b/packages/trace/src/index.ts @@ -13,6 +13,7 @@ export * from './a11y-snapshot.js' export * from './sha1.js' export * from './screencast-trace.js' export * from './trace-action-events.js' +export * from './trace-retention.js' export * from './trace-console.js' export * from './trace-exporter.js' export * from './trace-frame-snapshots.js' diff --git a/packages/core/src/trace-retention.ts b/packages/trace/src/trace-retention.ts similarity index 100% rename from packages/core/src/trace-retention.ts rename to packages/trace/src/trace-retention.ts diff --git a/packages/core/tests/trace-retention.test.ts b/packages/trace/tests/trace-retention.test.ts similarity index 100% rename from packages/core/tests/trace-retention.test.ts rename to packages/trace/tests/trace-retention.test.ts