Repository navigation
Cross-adapter conformance suite, and the bugs it found - #428
vishnuv688 wants to merge 14 commits into
Conversation
A runnerless script that never called DevTools.startTest dropped every command: only a start call created the TestManager, so the "Selenium Session" row the README promises never appeared. One is now created for a script whose runner is undetected. Such a script also never exited. Teardown runs on beforeExit, which the open worker socket stopped from firing; the socket is now unref'd once connected and re-ref'd while closing, so the close still completes.
The app and the backend's baseline store each carried their own copy of how a replaceCommand payload finds its row: by stable id, else the last row carrying the old timestamp. It now lives once in shared, with a test pinning the case that matters: two rows completing in one millisecond.
They describe what an adapter puts in Metadata.options for the run controls, which crosses the wire, so they belong beside Metadata. The app keeps its names through a re-export.
A WebDriverException's str() prefixes "Message: " and appends the driver's stacktrace, so the dashboard's Errors tab showed both around the browser's text. The row now carries `msg`, the browser's own message, as the JavaScript adapters do; other exceptions keep their str().
Each adapter runs one scenario as its own process against a fake W3C WebDriver that records what really happened in the browser, reporting into a recording backend through the rerun reuse handshake, so no adapter gains a test-only path. One checker judges every recorded frame stream against behavioural rules, with and without BiDi. The fake recognises the collector by its own source rather than any adapter's idiom, and the recording backend serves the real collector bundle. Known gaps need a reason and fail once they stop occurring. Every run writes its stream and verdict to out/, and an HTML report places each violation on the event it concerns, naming the adapter responsibility the broken rule tests. Covers selenium-devtools and selenium-devtools-py. Refs #300
…suite # Conflicts: # pnpm-lock.yaml
|
| if (COLLECTOR_SIGNATURE.test(script) || COLLECTOR_SIGNATURE.test(args)) { | ||
| doc.collector = true | ||
| return true |
There was a problem hiding this comment.
Duplicate capture goes unnoticed
Reinjecting the collector leaves doc.anchored unchanged. After an adapter drains the first anchor, reinjects, and drains again, the fake emits no second anchor. The real source creates a new DataCollector whose #anchored starts false, so the same sequence can emit a duplicate.
Model a new collector on injection and test that sequence so one-anchor-per-document can catch this adapter bug.
| await writeAtomically( | ||
| path.join(outDir, REPORT_INDEX), | ||
| renderReport(await readRunReports(outDir)) | ||
| ) |
There was a problem hiding this comment.
Each parallel test rebuilds the same index.html independently. One writer can read the directory before another publishes its JSON, then finish its HTML write last and remove that newer run from the index. The uploaded HTML can omit a failing run even though its JSON exists.
Renaming each file prevents partial reads, not this stale overwrite. Build the index once after all runs finish, or lock the full read-and-write step across workers.
…e rules The rules only catch what someone thought to write a rule for. Each run is now reduced to what a dashboard user sees (each step's shared action name, target, outcome and the page it replays, the pages snapshotted, the run controls) and compared across adapters within a mode. Any difference fails as an adapters-agree-on-* rule unless a declared gap explains it, and a gap explains it only while its holder's own run shows the gap, so one adapter's known limitation cannot mask another adapter regressing. To compare runs they have to exist side by side, so adapters are now registered once in src/adapters.ts and one test runs every registered adapter in both modes, sequentially, writing the report once at the end. The two per-adapter test files are gone; they also rebuilt index.html independently, so one could drop the other's newer run. A scenario that cannot run becomes a failed report rather than a thrown error.
A table per mode, a column per adapter and a row per step, differing rows highlighted, headed AGREE, KNOWN DIFFERENCES (n) or DIVERGE.
…lock A row stamped at invocation passed as long as startTime did not exceed timestamp, and only navigation rows were held to the page they showed, so a click or a fill stamped early slipped through. The fake now records when it completed each action, and the nth row of a kind must end no earlier than the nth completion of that kind; an equal millisecond is fine, since a fast command legitimately finishes in the one it started. An anchor was accepted anywhere between two births; it must now carry its document's exact birth time, which is what the collector stamps. A second injection into a document now starts a fresh, unanchored collector, as the real bundle does, so a duplicate anchor can be modelled and one-anchor-per-document can actually fire.
…al collector preload A scenario timeout rejected, so a hung adapter, the bug class this suite has already caught, lost the frames that showed where it hung. The run is now killed and reported as failed with everything recorded up to then. The fake BiDi endpoint counted any script.addPreloadScript as the collector; it now reads the body and registers only the collector's own source, so an empty preload no longer produces a passing stream.
| return { | ||
| action: `${action.class}.${action.method}`, | ||
| target: targetOf(row), | ||
| outcome: row.error ? `error: ${row.error.message.trim()}` : 'ok', | ||
| page: shown?.url ? pathOf(shown.url) : '(none)' | ||
| } |
There was a problem hiding this comment.
normalizeRun drops the text typed by fill commands. Changing one adapter's reported args from ['tom'] to [] or ['wrong'] leaves the same action, selector, outcome and page. compareRuns then reports agreement even though the dashboard shows different parameters.
Compare the fill value after removing differences in how the bindings package it, and add a differing-value test.
Knowledge Base Used: Shared contracts and transport
| ? gapsFor(id, run.bidi).filter((g) => | ||
| run.violations.some((v) => g.rules.includes(v.rule)) | ||
| ) | ||
| : [] | ||
| } | ||
| const explainedBy = (d: Divergence) => | ||
| d.adapters.some((id) => occurring(id).some((g) => g.rules.includes(d.rule))) |
There was a problem hiding this comment.
judgeDivergences excuses every page or snapshot difference once the JS run has any matching missing-anchor violation. For example, JS can miss the first login anchor while Python independently stamps a fill row after the submit navigation. Python's row then shows the wrong page, but its timestamp is not early and its anchors are valid, so its own rules pass. The unrelated page difference is also excused by the JS gap.
Limit each exception to the documents and steps affected by that gap, and test both failures occurring together.
| exitCode: null, | ||
| output: String(err), | ||
| frames: [], | ||
| truth: { | ||
| documents: [], | ||
| failedNavigations: [], | ||
| actions: { navigate: [], click: [], fill: [] } |
There was a problem hiding this comment.
Bad frames lose their evidence
runAdapter discards the captured run if checkConformance throws. A malformed commands payload, such as an object instead of an array, throws in readStream. This catch then replaces the recorded frames, ground truth and scenario output with empty values. The adapter fails the test, but the report loses the payload needed to diagnose it.
Keep the completed run when checking fails and record the checker error separately.
Refs #300
Every adapter is supposed to tell the dashboard the same story about the same test, and nothing checked that. Each adapter's own tests consider its own output correct, so a capture fix that lands in one adapter and not another, or a field one adapter formats differently, passes CI and surfaces after release. This adds a suite that runs one scenario through each adapter and judges what it actually sends, two ways.
How it works
packages/conformance/src/fake-webdriver.ts) models two pages and records ground truth: which documents were born and when, which navigations failed and with what message, and when it completed each navigation, click and keystroke. A minimal BiDi endpoint lets a registered collector preload put the collector in every later document from birth, as in Chrome; it recognises the collector by its own source, not by any adapter's way of injecting it, and an unrelated preload registers nothing.src/adapters.ts; one test runs every registered adapter with and without BiDi. A scenario that hangs is killed and reported with the frames recorded up to then.src/checker.ts, 11 of them, each traced to a bug the repo has hit): a row ends no earlier than the browser completed its action, one DOM anchor per document carrying its exact birth time, a navigation row replays its destination, one row per browser action, a failed command carries the browser's own message, replacements target a sent row, advertised run controls have a command, and so on. Replacements are applied with the same rule the dashboard uses.src/normalize.ts,src/agreement.ts): within each mode, every run is reduced to what a dashboard user sees (each step's shared action name, target, outcome and the page it replays, the pages snapshotted, the run controls) and compared with the others. Any difference fails as anadapters-agree-on-*rule, with no per-field rule needed. The rules catch every adapter being wrong in the same way; the comparison catches a difference no rule anticipated.<script>fallback cannot anchor a page left within tens of ms.packages/conformance/out/, plus an HTML page with an agreement table per mode (a column per adapter, a row per step, differing rows highlighted) and, per run, a timeline with each violation on the event it concerns and the adapter responsibility the broken rule tests, linked to the guide. CI uploads it when the job fails.Covers
selenium-devtoolsandselenium-devtools-py; the WDIO service and Nightwatch have no scenario yet. Console, network and assertion capture are not checked yet.Bugs it found, fixed here
DevTools.startTestcaptured nothing: only a start call created the TestManager.beforeExit, where teardown runs, from firing. This includes every rerun of one.str(WebDriverException), so the Errors tab showedMessage:and the driver's stacktrace around the browser's text. It now carriesmsg, as the JS adapters do.Also in this PR
replaceCommand(find a row by id, else the last row with the old timestamp) moves intoshared; the app, the backend and the checker use one copy.RunCapabilitiesandRunnerOptionsmove intoshared, since they describeMetadata.optionson the wire.docs/ADAPTERS.md: a guide to writing an adapter in another language, written around the suite.ARCHITECTURE.mdgains a section on the suite and corrects its claim that the Python adapter writes no trace archive.pnpm test, so both languages run there. The adapter tests skip locally when a prerequisite is missing, and fail rather than skip in CI.webdriverio@9.32.0, which was already in the lockfile; no installed version changes.Review feedback
All seven review comments are addressed in the last commits: rows are held to the browser's recorded completion times (not only
startTime <= timestamp), anchors must match the exact birth time, an empty preload no longer counts as the collector, a second injection starts an unanchored collector so a duplicate anchor can be modelled, the report is written once after all runs, a hung scenario keeps its frames, and the guide now states that replacements match byidfirst.Verification
pnpm build,pnpm test(2564 tests),pnpm lint,pnpm typecheck: green. Python unit tests (709): green.launchCommandgetter, and storing Python'sstr()error. The cross-adapter comparison was checked the same way with thestr()error, which it flags with no error-specific rule.