Repository navigation
Cross-adapter conformance suite, and the bugs it found - #428
Open
vishnuv688 wants to merge 19 commits into
Open
vishnuv688 wants to merge 19 commits into
vishnuv688 wants to merge 19 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
Contributor
|
…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.
A fill's text is read whatever shape the binding packaged it in (bare, or the W3C text and value payload), so two adapters typing different text no longer compare equal.
…ry row to its page A declared gap excused any matching difference once it was occurring, so one adapter's known limitation could hide the other adapter being wrong at the same step. A gap now explains a difference only where the holder's own run shows the gap on that same step or document and the other adapter's run is clean there. For that to be checkable, the page rule covers every action row, not only navigations: row-replays-its-page requires a navigate, click or fill row to replay the document the browser was on when that action completed, so a fill stamped after a later navigation is a finding of its own.
A checker that threw on a malformed frame replaced the recorded frames, ground truth and output with empty values. The run is kept and the checker's error recorded beside it.
…aded Generalising the page rule to every action row dropped the check that a navigation's reported url is the document the browser loaded, so two adapters both reporting the wrong destination agreed with each other and with the anchors. It is back as navigation-row-names-its-destination, read off the same row-to-completion mapping.
A fill row with args that were not an array passed the checker and then crashed normalisation, which took the whole HTML report down. The checker now reports such a row as a clear error where it reads the stream; the harness keeps the frames beside it and the report renders every other run.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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, 12 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, every navigate, click or fill row replays the document the browser was on when that action completed, a navigation row names the page the browser loaded, 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, typed text, 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
The two latest comments are addressed: a navigation row must again name the page the browser loaded (its own rule, beside the generalised page rule), and a row without
command,args[]andtimestampis a clear checker error that keeps the frames and leaves every other run's report intact.The three comments before those are addressed too: a fill's typed text is compared across adapters; a gap is scoped to the step or document it occurs on, with the page rule generalised to every action row so a fill stamped after a later navigation is a finding in its own right; and a checker that cannot read a stream keeps the recorded run and records its error beside it.
The first seven review comments are addressed in earlier 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.