test: end-to-end pipeline integration test, and the compiler bug it found - #70
Conversation
|
Follow-up filed: #71 — Capture real visibility: fix That is the root cause this PR deliberately did not fix. It is marked blocked by this PR (same branch of Filing it surfaced a consequence I had not written down here: |
…arget
Adds tests/integration/pipeline.test.ts, the first test that crosses package
boundaries: record -> compile -> cache-write -> replay -> gate report, against
the bundled fixture. Wired into `npm run ci` and a new CI step. Runs in ~2s
with no Docker, no model, and no egress.
It found a real compiler bug on its first run, so the fix ships with it.
synthesizeAssertion always preferred the acted-on control as the
element-visible target, including for clicks. That post-condition is not
merely weak, it is usually false: a click that transitions the page hides the
surface the control lived on. Replaying the compiled login step timed out
waiting for the submit button to stay visible after it had been clicked.
The post_state landmark fallback does not rescue it either. visible_landmarks
is collected by walking the DOM with no visibility filter, so a landmark that
was just hidden is still listed - the name overpromises.
Fix: a click whose url_template changed skips the element-visible branch and
is asserted on its destination via url-matches, which that branch already
labels strong. Narrow by construction - fills, non-navigating clicks, and
navigate steps are untouched, and the example trajectory has no click steps,
so neither the committed bundle nor existing expectations move. Pinned by two
new unit tests.
The test serves the fixture over loopback instead of opening it as file://.
A file://{fixture_root}/... template makes the parameter a whole filesystem
path, and a path-valued hole spans / separators, which the compiler's
templateToRegex deliberately rejects (holes compile to [^/?#]+ so a url-matches
assertion cannot skip path segments). Loosening that to fit the test would
weaken every URL assertion the product emits. http://{host}:{port}/... is also
what a real recording looks like.
Refs #52
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
736e071 to
c3a8a93
Compare
|
Rebased onto One file conflicted: Resolved by keeping both, in the order each intended: Nothing from #69 was dropped — its job body, Re-verified against the new
Force-pushed with |
ADR-0007. #70 added the first end-to-end test and it caught the compiler asserting "the button I clicked is still visible" as the post-condition for logging in. That PR fixed only the navigating case by asserting the destination. This closes the root cause underneath it, which was two separate defects. visible_landmarks did not mean visible. Both capture sites collected landmark roles with no visibility filter - the recorder walked every element under document.body, the runner used querySelector - so a landmark that had just been hidden was still listed. On the fixture's login page that reported the hidden app view's banner and navigation as visible, which is why the landmark fallback could not rescue a mis-asserted click. Now filtered in both, with the same in-page checkVisibility() predicate so the recorder and the repair context cannot describe the same page differently. That second site matters: capturePageState feeds RepairContext.page_state, so once #27 wires a real model it would have been handed a page the recorder never saw. Nothing recorded whether the acted-on control survived. Added optional post_action_target_visible to trajectory.schema.json, observed with Playwright's Locator.isVisible() - deliberately Playwright and not the DOM predicate, because src/runner/assertions.ts later checks that same target with waitFor state hidden/visible, so the recorder now claims exactly what the runner will check. The compiler consumes it: a click-like step whose target went visible -> hidden is asserted element-visible with expected.visible false. Labelled strong because for a dismiss-shaped control the disappearance is the purpose of the step and the assertion fails on a no-op. A click that both navigates and hides its control keeps url-matches - where it landed is better evidence than what vanished. Reproduced first with a self-hiding, non-navigating control added to the fixture, which failed exactly as predicted before the fix: step 5: REPAIR_EXHAUSTED - locator.waitFor: Timeout 5000ms exceeded. 14 x locator resolved to hidden <button data-testid="dismiss-notice"> Artifacts regenerated by command, never hand-edited. dom_digest shifts because it is derived from the same structural signals. The example trajectory has no clicks, so its committed bundle is byte-identical. Role and element counts stay DOM-wide on purpose - they are structural, not visibility claims. Closes #71 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ADR-0007. #70 added the first end-to-end test and it caught the compiler asserting "the button I clicked is still visible" as the post-condition for logging in. That PR fixed only the navigating case by asserting the destination. This closes the root cause underneath it, which was two separate defects. visible_landmarks did not mean visible. Both capture sites collected landmark roles with no visibility filter - the recorder walked every element under document.body, the runner used querySelector - so a landmark that had just been hidden was still listed. On the fixture's login page that reported the hidden app view's banner and navigation as visible, which is why the landmark fallback could not rescue a mis-asserted click. Now filtered in both, with the same in-page checkVisibility() predicate so the recorder and the repair context cannot describe the same page differently. That second site matters: capturePageState feeds RepairContext.page_state, so once #27 wires a real model it would have been handed a page the recorder never saw. Nothing recorded whether the acted-on control survived. Added optional post_action_target_visible to trajectory.schema.json, observed with Playwright's Locator.isVisible() - deliberately Playwright and not the DOM predicate, because src/runner/assertions.ts later checks that same target with waitFor state hidden/visible, so the recorder now claims exactly what the runner will check. The compiler consumes it: a click-like step whose target went visible -> hidden is asserted element-visible with expected.visible false. Labelled strong because for a dismiss-shaped control the disappearance is the purpose of the step and the assertion fails on a no-op. A click that both navigates and hides its control keeps url-matches - where it landed is better evidence than what vanished. Reproduced first with a self-hiding, non-navigating control added to the fixture, which failed exactly as predicted before the fix: step 5: REPAIR_EXHAUSTED - locator.waitFor: Timeout 5000ms exceeded. 14 x locator resolved to hidden <button data-testid="dismiss-notice"> Artifacts regenerated by command, never hand-edited. dom_digest shifts because it is derived from the same structural signals. The example trajectory has no clicks, so its committed bundle is byte-identical. Role and element counts stay DOM-wide on purpose - they are structural, not visibility claims. Closes #71 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
architecture.md was written against 6ad7151, before #70 merged, so it still claimed the bundle -> cache -> runner seam was untested. It is now exercised end to end by tests/integration/pipeline.test.ts. What remains true, and is the useful statement, is narrower: no *product* path walks that seam - only a test does, and the adapter lives in the test on purpose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…#73) * feat: capture real visibility and post-action target state (ADR-0007) ADR-0007. #70 added the first end-to-end test and it caught the compiler asserting "the button I clicked is still visible" as the post-condition for logging in. That PR fixed only the navigating case by asserting the destination. This closes the root cause underneath it, which was two separate defects. visible_landmarks did not mean visible. Both capture sites collected landmark roles with no visibility filter - the recorder walked every element under document.body, the runner used querySelector - so a landmark that had just been hidden was still listed. On the fixture's login page that reported the hidden app view's banner and navigation as visible, which is why the landmark fallback could not rescue a mis-asserted click. Now filtered in both, with the same in-page checkVisibility() predicate so the recorder and the repair context cannot describe the same page differently. That second site matters: capturePageState feeds RepairContext.page_state, so once #27 wires a real model it would have been handed a page the recorder never saw. Nothing recorded whether the acted-on control survived. Added optional post_action_target_visible to trajectory.schema.json, observed with Playwright's Locator.isVisible() - deliberately Playwright and not the DOM predicate, because src/runner/assertions.ts later checks that same target with waitFor state hidden/visible, so the recorder now claims exactly what the runner will check. The compiler consumes it: a click-like step whose target went visible -> hidden is asserted element-visible with expected.visible false. Labelled strong because for a dismiss-shaped control the disappearance is the purpose of the step and the assertion fails on a no-op. A click that both navigates and hides its control keeps url-matches - where it landed is better evidence than what vanished. Reproduced first with a self-hiding, non-navigating control added to the fixture, which failed exactly as predicted before the fix: step 5: REPAIR_EXHAUSTED - locator.waitFor: Timeout 5000ms exceeded. 14 x locator resolved to hidden <button data-testid="dismiss-notice"> Artifacts regenerated by command, never hand-edited. dom_digest shifts because it is derived from the same structural signals. The example trajectory has no clicks, so its committed bundle is byte-identical. Role and element counts stay DOM-wide on purpose - they are structural, not visibility claims. Closes #71 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(architecture): narrow the unwired-seam row after #52 architecture.md was written against 6ad7151, before #70 merged, so it still claimed the bundle -> cache -> runner seam was untested. It is now exercised end to end by tests/integration/pipeline.test.ts. What remains true, and is the useful statement, is narrower: no *product* path walks that seam - only a test does, and the adapter lives in the test on purpose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes #52.
Adds
tests/integration/pipeline.test.ts— the first test that crosses package boundaries. Every other suite tests one package in isolation, so the seams where the gate actually lives were verified only by humans reading four schemas.The test records the fixture task, compiles it, pushes every row through the fail-closed cache write, replays the compiled program live in Chromium, and feeds the emitted metric rows to
buildGateReport(). ~2s, no Docker, no model, no egress.It found a real bug on its first run
synthesizeAssertionalways preferred the acted-on control as theelement-visibletarget, including for clicks. That post-condition isn't merely weak — it's usually false. A click that transitions the page hides the surface the control lived on:The compiler was asserting "the button I clicked is still visible" as the post-condition for logging in — which the login transition immediately falsifies.
The
post_statelandmark fallback doesn't rescue it.visible_landmarksis collected by walking the DOM without a visibility filter, so a landmark that was just hidden is still listed. The name overpromises; it's landmark roles present.Fix: a
clickwhoseurl_templatechanged skips theelement-visiblebranch and is asserted on its destination viaurl-matches— which that branch already labelsstrong. So the assertion gets stronger, not weaker.Narrow by construction: fills, non-navigating clicks, and navigate steps are untouched, and
contracts/examples/trajectory.example.jsonhas no click steps — so neither the committed bundle nor any existing expectation moves. Pinned by two new unit tests (navigating click →url-matches; non-navigating click → stillelement-visible).This is scope beyond "add a test", and I want that visible rather than buried: the alternative was a skipped test documenting a bug, which protects nothing and can't gate CI.
Why loopback instead of
file://The issue said to open the fixture as a
file://URL. That turns out to be the wrong call and I'd rather say so than quietly comply.file://{fixture_root}/...makes the parameter a whole filesystem path, and a path-valued hole spans/separators. The compiler'stemplateToRegexdeliberately compiles holes to[^/?#]+so aurl-matchesassertion can't skip across path segments. Widening that to fit the test would weaken every URL assertion the product emits — for afile://quirk that no real recording has.Serving the fixture over loopback gives
http://{host}:{port}/..., which is single-segment holes and is exactly whatnpm run recorder -- --base-urlproduces against the Grafana testbed. The seam under test is now the representative one. Still hermetic — loopback, ~15 lines ofnode:http, no dependency.Deliberate-breakage check
Required by the issue. Two rounds, and the first one is worth reporting:
Round 1 — corrupted
testid/name/labelon every compiled locator. Test still passed. Not a weak test: the fallback chain fell through to thestructuralandcss_vocabcandidates and resolved correctly. That's the chain doing its job, demonstrated accidentally.Round 2 — replaced each chain wholesale with a single dead locator:
Test fails loudly, naming the step and the cause. Both edits reverted; working tree verified clean before commit.
Verification
npm run cigreen — now includestest:integrationnpm run test:canarygreenciwell inside budgetnpm run typecheckcaught an indexed-access widening on the params object, fixed with a typedFixtureParamsNotes for the reviewer
CompiledProgramadapter is local to the test, per the issue's design note. The runtime doesn't need it, and promoting it would be a contract change (ADR territory).docs/gate/compiler.mdsynthesis priority table updated, plus a new blind spot chore(deps-dev): bump typescript from 5.9.3 to 7.0.2 #8: the trajectory records no post-action visibility for the acted-on control, so a non-navigating click that hides its own control (modal close, no route change) is still mis-asserted. Fixing that properly means the recorder capturing post-action visibility — atrajectory.schema.jsonchange, so an ADR.🤖 Generated with Claude Code