Skip to content

fix(e2e): settle the support note once, for the whole run - #553

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/settle-the-support-note-for-the-whole-run
Sep 5, 2026
Merged

fix(e2e): settle the support note once, for the whole run#553
rubenvdlinde merged 2 commits into
developmentfrom
fix/settle-the-support-note-for-the-whole-run

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

The tier-3 push run (#551) came back 62 failed / 85 passed. Every one of the 62 is a click that never became actionable, across a dozen unrelated spec files, and the call log names the same thing each time:

<div data-testid-modal="cn-support-dialog" class="dialog__modal modal-mask">
subtree intercepts pointer events

CnAppRoot mounts CnSupportDialog on first open by default (supportDialog defaults to true). This app rooted on CnAppRoot in launchpad-manifest-tier-3 and inherited it.

Why #551 was green on every check

The E2E leg is skipping on pull requests and runs only on the development push. And app-chrome.spec.ts — the spec that change was about — happens to dismiss [data-testid="cn-modal"] in its own beforeEach, so it was among the survivors. The regression was invisible until the merge.

The fix

Record the note as seen once in globalSetup. Dismissal persists per user through the app's own preferences endpoint, so one call settles it for every spec in the run.

That is the same shape as dismissFirstRunWizard sitting beside it, and the same reasoning the walkthrough sentinel two paragraphs down already documents:

The marker is per USER, not per test, so without it the suite is also order-dependent: whichever spec runs first wears the tour and the rest inherit a dismissed one.

The localStorage key (cn-support-dialog-shown:launchpad) goes with it, because the composable falls back to it when the server read fails — a flaky preferences call would otherwise put the mask back.

What this deliberately does not do

⚠️ Not :support-dialog="false" in the app. That would take a real feature away from real users to make the tests quiet. The note is a first-open note; the tests simply need it already open-and-seen, which is what a returning user has.

Verified

format, lint and check:manifest pass. The standalone tsc TS5097 on this file is pre-existing — identical on origin/development without this change, checked by stashing.

The 62 come back on the development push run after this merges.

🤖 Generated with Claude Code


Second commit: two of the 62 were not the mask

the dashboards report renders real numbers and Store opens the hosted store surface both asserted .workspace-shell on their page. Those are my own assertions, and they were stale the moment routing landed: before tier 3 every URL fell back to the dashboard, so the workspace was on every page. It routes now, and those pages are the report and the store — the workspace being absent is the change working. They assert the shared chrome instead.

The same run is the evidence tier 3 works

These all passed on it — every route that used to fall through to /dashboard:

  • each declared chrome destination opens
  • Reports lists the one report this app can honestly offer
  • the Flows page opens
  • the admin manifest pages redirect to where the admin surface is

The tier-3 push run came back 62 failed / 85 passed. Every one of the 62
is a click that never became actionable, across a dozen unrelated spec
files, and the call log names the same thing each time:

    <div data-testid-modal="cn-support-dialog" class="dialog__modal modal-mask">
    subtree intercepts pointer events

CnAppRoot mounts CnSupportDialog on first open by default. This app
rooted on CnAppRoot in launchpad-manifest-tier-3, and inherited it. The
chrome spec was among the survivors only because it happens to dismiss
[data-testid="cn-modal"] in its own beforeEach — which is why the tier-3
PR went green on every check and the regression only appeared on the
push run, where the E2E leg actually runs.

Dismissal persists per USER through the app's preferences endpoint, so
recording it once in globalSetup settles it for every spec — the same
shape as dismissFirstRunWizard beside it, and the same reasoning the
walkthrough sentinel already documents two paragraphs down: the marker is
per user, so without it the suite is order-dependent and whichever spec
runs first wears the dialog.

The localStorage key goes with it, because the composable falls back to
it when the server read fails and a flaky preferences call would
otherwise put the mask back.

⚠️ NOT `:support-dialog="false"` in the app. That would take a real
feature away from real users to make the tests quiet.

Verified: format, lint and check:manifest pass. The standalone `tsc`
TS5097 on this file is pre-existing — identical on origin/development
without this change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of the 62 were not the modal mask. They are my own assertions, and
they were stale the moment routing landed.

`the dashboards report renders real numbers` and `Store opens the hosted
store surface` both asserted `.workspace-shell` on their page. That
passed before `launchpad-manifest-tier-3` for the wrong reason: every URL
fell back to the dashboard, so the workspace WAS on every page. It routes
now, and those pages are the report and the store — the workspace being
absent is the change working.

They assert the shared chrome instead, which is what those pages do have.

The rest of that spec is the direct evidence tier 3 works: `each declared
chrome destination opens`, `Reports lists the one report this app can
honestly offer`, `the Flows page opens` and `the admin manifest pages
redirect` all PASSED on the same run — every route that used to fall
through to /dashboard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/launchpad @ de166f3

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
format
check-schema-l10n
composer ✅ 104/104
npm ✅ 527/527
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright
Hydra gates

Quality workflow — 2026-09-05 17:21 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/launchpad @ 2d270b5

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
format
check-schema-l10n
composer ✅ 104/104
npm ✅ 527/527
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates

Quality workflow — 2026-09-05 17:24 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 728f6cc into development Sep 5, 2026
43 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