Skip to content

Cross-adapter conformance suite, and the bugs it found - #428

Open
vishnuv688 wants to merge 14 commits into
mainfrom
feat/300-conformance-suite
Open

vishnuv688 wants to merge 14 commits into
mainfrom
feat/300-conformance-suite

Conversation

@vishnuv688

@vishnuv688 vishnuv688 commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

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

  • A fake W3C WebDriver (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.
  • A recording backend stands in for the dashboard, records every worker-socket frame, and serves the real collector bundle as the real backend does.
  • The harness runs each adapter's scenario as its own process, reached through the existing rerun reuse handshake, so no adapter gains a test-only code path. Adapters are registered once in 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.
  • The rules (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.
  • Cross-adapter agreement (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 an adapters-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.
  • Known gaps live in an allowlist with a mandatory reason. An entry that stops matching fails the run, and a gap excuses a cross-adapter difference only while its holder's own run shows it, so one adapter's limitation cannot mask another adapter regressing. One gap exists: the Selenium JS adapter without BiDi, whose documented <script> fallback cannot anchor a page left within tens of ms.
  • A report: every run writes its stream and verdict to 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-devtools and selenium-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

Adapter Bug
selenium-devtools A plain script that never called DevTools.startTest captured nothing: only a start call created the TestManager.
selenium-devtools A plain script never exited: the open worker socket stopped beforeExit, where teardown runs, from firing. This includes every rerun of one.
selenium-devtools-py A failed command's row carried str(WebDriverException), so the Errors tab showed Message: and the driver's stacktrace around the browser's text. It now carries msg, as the JS adapters do.

Also in this PR

  • replaceCommand (find a row by id, else the last row with the old timestamp) moves into shared; the app, the backend and the checker use one copy.
  • RunCapabilities and RunnerOptions move into shared, since they describe Metadata.options on the wire.
  • docs/ADAPTERS.md: a guide to writing an adapter in another language, written around the suite. ARCHITECTURE.md gains a section on the suite and corrects its claim that the Python adapter writes no trace archive.
  • CI's build job installs the Python adapter before pnpm test, so both languages run there. The adapter tests skip locally when a prerequisite is missing, and fail rather than skip in CI.
  • The lockfile merge with main consolidates some peer-dependency contexts onto 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 by id first.

Verification

  • pnpm build, pnpm test (2564 tests), pnpm lint, pnpm typecheck: green. Python unit tests (709): green.
  • Each rule was checked by breaking real adapter code and confirming the suite fails with that rule, then restoring it: removing Python's forced anchor in the drain script, stamping JS rows at invocation, removing the JS launchCommand getter, and storing Python's str() error. The cross-adapter comparison was checked the same way with the str() error, which it flags with no error-specific rule.

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
@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge, with three non-blocking gaps in the new comparisons and failure reports.

Findings

  1. P2 Wrong typed text passes ▶
  2. P2 A gap hides another bug ▶
  3. P2 Bad frames lose their evidence ▶

Summary

Adds a shared conformance suite for the JavaScript and Python Selenium adapters, plus fixes for plain-script capture, process exit and browser error text.

  • JavaScript and Python adapters now run the same scenario through a shared set of checks.
  • Plain Selenium scripts now capture commands and exit when finished.
  • Python command errors now show the browser’s message without exception formatting.
  • The app and backend now use shared command replacement and run-option types.
  • The architecture guide now explains how Python writes trace archives.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Registry[Adapter registry] --> Scenario[Scenario process]
  Scenario --> Driver[Fake WebDriver]
  Scenario --> Backend[Recording backend]
  Driver --> Truth[Browser ground truth]
  Backend --> Frames[Recorded frames]
  Truth --> Rules[Shared rules]
  Frames --> Rules
  Frames --> Compare[Cross-adapter comparison]
  Rules --> Report[JSON and HTML reports]
  Compare --> Report
Loading

Reviews (2) · Last reviewed commit: "docs(adapters): replaceCommand matches b..." · Reviewed by Greptile

Comment thread packages/conformance/src/checker.ts Outdated
Comment thread packages/conformance/src/checker.ts Outdated
Comment thread packages/conformance/src/fake-bidi.ts Outdated
Comment on lines +269 to +271
if (COLLECTOR_SIGNATURE.test(script) || COLLECTOR_SIGNATURE.test(args)) {
doc.collector = true
return true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Comment on lines +20 to +23
await writeAtomically(
path.join(outDir, REPORT_INDEX),
renderReport(await readRunReports(outDir))
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The report drops a run

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.

Comment thread packages/conformance/src/harness.ts Outdated
Comment thread docs/ADAPTERS.md Outdated
…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.
@vishnuv688 vishnuv688 linked an issue Oct 13, 2026 that may be closed by this pull request
Comment on lines +18 to +23
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)'
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Wrong typed text passes

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

Comment on lines +103 to +109
? 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)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 A gap hides another bug

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.

Comment on lines +80 to +86
exitCode: null,
output: String(err),
frames: [],
truth: {
documents: [],
failedNavigations: [],
actions: { navigate: [], click: [], fill: [] }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cross-adapter conformance suite for capture invariants

1 participant