Complete the e2e data-test contract and guard ratchet - #5650
Merged
norman-abramovitz merged 7 commits intoJul 18, 2026
Conversation
A file on the e2e legacy list that stops violating the drift guards previously went unnoticed: the guards stayed off and its entry could linger after modernization. lint-e2e-ratchet.mjs re-runs the two guard rules over the listed files and fails when an entry is clean or its file no longer exists, forcing list removal on the PR that cleans it. Chained into bun run lint after the data-test drift check. (cloudfoundry#5619)
Extend the data-test contract (cloudfoundry#5619 section 2) to tabs: every tab surface now exposes '<prefix>-' + label — page-side-nav (page-tab-), signal-detail (signal-detail-tab-, previously one shared static value), nested-tabs (nested-tab-) and custom-tabs (custom-tab-). The e2e pageTab() helper binds to the hook via exact attribute match instead of CSS class + label-span regex.
Kebab-menu entries in signal-list previously had no test hook — e2e had to match them by text, the exact failure mode from cloudfoundry#5619's unmap- route example. Both card and table menus now render act.dataTest ?? ('row-action-' + act.label), matching the existing header- and bulk-action convention, and SignalListRowAction grows the same optional dataTest field. (cloudfoundry#5619 section 2)
Dynamic bindings like 'header-action-' + act.label used to make any e2e ref under a live prefix pass, so a renamed action label left the old ref silently green. The drift check now harvests every label/ textLabel string literal (and explicit dataTest config values) from src and requires a prefixed ref's suffix to be one of them. Closes the known hole recorded at the check's introduction. (cloudfoundry#5619 section 2 follow-on)
Close cloudfoundry#5619 section 3 in its feasible form: scripts/e2e-impact.mjs derives which e2e specs cover a set of changed files — component data-test hooks and element selectors matched against e2e sources, with hits in page objects walked up the import graph to the specs that use them — and prints a scoped playwright command. A pr.yml job posts the report to the job summary on every PR; it never gates, because the e2e suite needs a live CF endpoint that CI does not have. TESTING.md gains the matching contributor rule: sync or file a follow-up for covering specs when changing component DOM.
Ten verified findings from review of the branch, all in the new tooling: - e2e-impact: hook tokens now match only quote-adjacent (generic hooks like 'row'/'empty' matched identifiers and prose, reporting ~80 specs for signal-list; now 27 with no unrelated specs), barrel index.ts re-exports are pierced to the declaring module so one hit no longer fans out to every spec, and a bare invocation on a TTY prints usage instead of hanging on stdin. - drift check: label/dataTest harvest excludes .spec.ts fixtures (fixture literals kept renamed-label drift green), the too-broad '<' placeholder skip is removed (docs reworded instead), a ref is valid under ANY live prefix rather than only the longest, and unmatched computed labels get an explicit COMPUTED_LABEL_ALLOWLIST escape. - ratchet: parse errors and eslint-ignored files report 'could not be checked' instead of 'is clean', which would have directed authors to delist files the guards never ran on. - pageTab(): visible-label filter restored on top of the data-test hook so an unlabeled tab cannot pass invisibly. - pr.yml: pipefail so a failed git diff fails the advisory job instead of posting a false 'no frontend changes'. - shared scripts/lib/e2e-hooks.mjs: single definition of hook syntax for both consumers.
The first hardening pass overcorrected in three places, all fixed with
their failure scenarios reproduced and re-verified:
- e2e-impact matching regained lost coverage: hook matching now
accepts template-literal quotes, and element-selector tags match on
word boundaries again so compound selectors ('app-x form',
'app-x, app-y') count — generic hook names keep the quote-adjacent
rule, so the earlier noise fix stands (still 0 unrelated specs).
- e2e-impact import graph handles export-from, namespace, mixed
default+named, and side-effect imports; named imports still pierce
barrels, broad imports edge to every re-export target instead of
dead-ending at the barrel.
- deleted components harvest tokens from the base ref (IMPACT_BASE,
wired in pr.yml) — a deletion is the change most likely to strand
specs and used to report zero impact.
- ratchet counts suppressedMessages as violations (an inline
eslint-disable no longer forces delisting a still-violating file)
and only fatal/ignored results report unverifiable, so a stale
unused directive no longer masquerades as a parse error.
- changed .ts/.html halves of one component dedupe to a single entry;
pageTab() escapes regex metachars in labels; drift-check message no
longer claims definitions come only from templates.
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
2 participants
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.
Closes #5619 (sections 2 and 3, plus the two follow-ons recorded after #5648).
What this adds
Ratchet became-clean enforcement — the legacy list introduced in #5648 only shrank by convention; nothing failed when a listed file stopped violating.
scripts/lint-e2e-ratchet.mjsre-runs the two guard rules over the listed files and fails lint when an entry is clean or its file is gone, so list removal happens on the PR that cleans the file. Suppressed violations (inline eslint-disable) still count as violating, and parse errors report as unverifiable rather than clean.Per-tab data-test hooks on every tab surface — page-side-nav (
page-tab-+ label), signal-detail (signal-detail-tab-+ label, previously one shared static value), nested-tabs and custom-tabs. The e2epageTab()helper now binds by exact attribute match plus a visible-label filter, replacing the CSS-class + label-regex approach.Row-action menu hooks — kebab-menu entries in signal-list previously had no test hook, so specs matched them by text (the failure mode from the unmap-route example in #5619). Both card and table menus now render
act.dataTest ?? ('row-action-' + act.label), matching the header/bulk-action convention.Suffix validation for prefix-style hooks — a ref like
header-action-Deployused to pass on the prefix alone, so a renamed label left the old ref green. The drift check now harvests everylabel:/textLabel:literal anddataTest:config value from non-spec sources and requires a prefixed ref's remainder to be one of them. Spec-fixture literals are excluded so fixtures cannot mask renamed-label drift; computed labels have an explicit allowlist escape.Advisory component→e2e impact report —
scripts/e2e-impact.mjsderives which specs cover a set of changed files: component hooks and element selectors matched against e2e sources, with hits in page objects walked up the import graph (piercing barrel re-exports so one hit doesn't fan out to every spec) to the specs that use them. Apr.ymljob posts the report and a scopedplaywright testcommand to the job summary on every PR. It never gates: the e2e suite needs a live CF endpoint, which CI does not have — a gating scoped run is left as separate infrastructure work.TESTING.mdgains the matching contributor rule.Notes for review
bun run lint, so bothmake check lintandmake check gateinherit it with no Makefile change.scripts/lib/e2e-hooks.mjsso the drift check and the impact report cannot diverge.