Skip to content

Match the Coverage toggle exactly in the staging E2E suite - #370

Merged
Babissimo merged 1 commit into
mainfrom
fix/coverage-locator-strict-mode
Sep 14, 2026
Merged

Babissimo merged 1 commit into
mainfrom
fix/coverage-locator-strict-mode

Conversation

@Babissimo

@Babissimo Babissimo commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Why

Every push to main since 2026-09-13 19:00 has failed, and production has not
deployed since 2026-09-11 22:17 (#361). Eleven merges are sitting on main
undeployed, including the node_ref publication work: production is currently
serving raw node ids publicly (curl -s https://api.retina.fm/api/radar/nodes
returns 11 ret… ids), which is the disclosure that work exists to close.
Unblocking this gate is what ships it.

Three of the failing runs died in Playwright E2E (staging) on a strict-mode
violation:

strict mode violation: getByRole('button', { name: /Coverage/i }) resolved to 2 elements:
  1) <button class="toggle-btn">Coverage</button>
  2) <button class="toggle-btn" title="Show red lines…">Coverage gaps</button>

The "Coverage gaps" button arrived in #358 (667ab72a). Two assertions in
live-map.spec.ts still use a loose /Coverage/i, which now matches both.

Why no PR caught it

ci.yml:302 gates that job on github.ref == 'refs/heads/main' && github.event_name == 'push',
and everything from "Deploy to staging" (ci.yml:596) onward carries the same
condition. Pull requests run only build, lint, tests, docker and env-parity, so
#358 could not have seen this. Its own main run was cancelled, so the failure
first surfaced on the next push.

That applies to this PR too: the suite it fixes will not run here. It is proven
only once this lands on main.

The change

Both locators pin to the exact accessible name. Playwright's own error output
suggested { name: 'Coverage', exact: true } as the locator resolving to the
button we want, and the button's markup is bare text with no icon or title,
so the accessible name is exactly Coverage.

The neighbouring /Labels/i and /Arcs/i locators are proven unambiguous by
the tests at lines 274 and 286, which passed in the same run. /Trails/i never
executed (line 147 threw first), but the only other "trails" text is "Export
trails as CSV", which is gated behind menuOpen && and carries
role="menuitem", so getByRole("button") cannot match it.

Not fixed here

The other three failing runs died earlier, in Staging smoke tests, on
Active nodes > 0 FAIL (0 < 1). That is the deploy concurrency race already
filed as 123zgec26td: deploy-staging carries concurrency: staging-deploy
(ci.yml:597), but staging-smoke-tests (ci.yml:729) and e2e-staging
(ci.yml:295) carry none, so a following run's deploy lands between a previous
run's deploy and its verification. Merges landing in the same minute (#357,
#358 and #360 at 19:00; #349, #362 and #363 at 20:40) are what trigger it.

It gates the production deploy independently of this fix, so it can still block
a run on its own. Fixing the selector makes the E2E gate deterministic; it does
not make the pipeline reliable.

🤖 Generated with Claude Code

The "Coverage gaps" button added in #358 also matches /Coverage/i, so both
toolbar assertions started failing on a Playwright strict-mode violation.
That job only runs on a main push, so the PR that introduced the button
could not have caught it, and every main push since has failed: production
has not deployed since 11 September.

Pin both locators to the exact accessible name. Playwright's own error
output suggested this locator for the button we want.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown

Claude finished @Babissimo's task in 51s —— View job


Code Review — PR #370

Todo list


Verified against frontend/src/components/map/Toolbar.tsx:

  • The Coverage button (line 181-183) has no title attribute and only the text node Coverage, so its accessible name is exactly "Coverage".
  • The Coverage gaps button (line 223-229) has a title attribute, but since it has visible text content (Coverage gaps), the accessible name is computed from that text, not the title — per the accessible-name algorithm, title is only used as a fallback when there's no other content. So its accessible name is "Coverage gaps", which no longer matches { name: "Coverage", exact: true }.

Both changed locators (live-map.spec.ts:148 and :262) are the only two /Coverage/i usages in the file — confirmed via grep, no other loose match was missed. The fix is minimal, correct, and matches the root cause described in the PR body.

No other files needed to change: this is a test-only fix, so the OpenAPI contract regeneration requirement in CLAUDE.md doesn't apply here (no /v1/nodes routes touched).

One nit, non-blocking: the reasoning in the PR body about /Trails/i never having executed is sound (the only other "trails" text, "Export trails as CSV", is gated behind menuOpen && and uses role="menuitem", so it can't be matched by getByRole("button")) — confirmed in Toolbar.tsx. No change needed there.

Verdict: LGTM. No blocking issues found. As the PR notes, this can only be proven once merged to main since the staging E2E job is gated to push-on-main.

@Babissimo
Babissimo merged commit a494b97 into main Sep 14, 2026
31 checks passed
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.

1 participant