E2E coverage for the native AFP product pages, and three defects it found - #1229
Open
busbyk wants to merge 6 commits into
Open
E2E coverage for the native AFP product pages, and three defects it found#1229busbyk wants to merge 6 commits into
busbyk wants to merge 6 commits into
Conversation
Next hands a dynamic path segment percent-encoded, so a zone whose name contains `&` arrived as `soldier-%26-wood-river-valley-mtns` and matched nothing in the zone list, which is built from the center's own zone URLs. Three of Sawtooth's four active zones are affected: the page rendered "Zone not found." — including when following the link the all-zones grid itself renders. Decoding the param means the slug is now a literal `&` where it is used to build a query string, so the OG image route and the archive fetch encode it explicitly rather than relying on it having arrived pre-encoded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The thumbnail buttons contain only a decorative `<img alt="">`, so they had no accessible name at all — a WCAG 4.1.2 failure. Names them by position, which is also what the media lightbox E2E coverage locates them by. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The E2E suite for the native product pages runs against a production build, which would otherwise report every synthetic failure it provokes to the real Sentry project. More importantly, the SDK's OpenTelemetry module instrumentation is loaded even when reporting is disabled, and it races this app's async server chunks: with Sentry wired in, concurrent requests intermittently returned `ReferenceError: Cannot access 'h' before initialization` from the Payload API — 185 occurrences in one run, and zero once the build stopped carrying it. That instrumentation still ships to production; worth investigating on its own. Also gives the mocked build its own `distDir`, so a build made against the live AFP API can never be served as if it were mocked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deterministic, offline end-to-end coverage for the native forecast pages, with MSW intercepting the AFP API at the Node network layer inside Next and Playwright driving the browser. Closes the E2E half of #1208. The seam is a `NODE_OPTIONS --import` preload, not `src/instrumentation.ts`: Next skips the instrumentation hook during a production build, so mocks started there could never shape `generateStaticParams` or any prerendered page — which is exactly the HTML a reader is served. The preload reaches the build workers, so the suite asserts against real prerendered output. An unmocked run is impossible rather than merely detected. The mocked build and server point `NAC_HOST`/`AFP_HOST` at the reserved `.invalid` TLD, which the mock handlers read too, so they agree by construction while a process where interception failed gets a DNS failure instead of real forecast data. The preload proves interception in its own process before Next renders anything; the build lands in its own `distDir`; and any unmapped NAC/AFP request answers 501, is recorded, and fails the run — necessary because these pages degrade to visible copy rather than crashing, so a missing fixture would otherwise read as a passing test of a degraded page. Fixtures are captured, never hand-authored: the AFP products-api golden corpus is vendored with a source commit and a sha256 per file, checked in CI and pre-commit so a "golden" cannot be quietly edited into saying what a test wants. Two responses the corpus does not cover yet are captured into a separate, clearly-marked staging area, each blocking a named test and carrying the upstream Case that retires it. Covers single-zone render, the danger block, expired and archived products, the all-zones grid, the media lightbox, off-season degradation, the native-vs-widget rollout flag, the DVAC→NWAC alias, and phone width. Six further specs are written and skip themselves with a reason until the upstream capture they need lands. Rollout state is fixed per tenant in the seed so no test mutates state another worker can observe. Documented in docs/afp-products/e2e-mocks.md, including the two defects the suite found: the ampersand slug bug fixed separately, and — carried as a `fixme` — that revalidating a forecast page's cache tag turns the page into a 404, so the freshness mechanism meant to deliver a correction removes it instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The revalidate-on-view path calls `revalidateTag` the moment a forecast changes. That is a *hard* cache invalidation — the next read of any page carrying the tag misses outright rather than going stale, which is how it differs from the `revalidate` window beside it. The zone route was `dynamicParams = false`, so Next answered that miss by abandoning the route; `[center]/[...segments]` picked the request up, found no Payload page, and 404'd — and cached it. Measured at ~70 seconds, on the corrected zone and on every zone sharing its weather product. One weather product covers all ten NWAC zones. So the mechanism that exists to deliver a correction removed the forecast instead, during exactly the conditions that draw readers to it. Nothing reached production: the rollout flag is off for every center. Generates the route on demand instead, which is what the dated route beside it already did, and 404s an unknown zone from an explicit check rather than from routing — before the rollout flag is read, so a bad slug behaves the same way on native and on the widget. `freshness.e2e.spec.ts` asserted this and was carried as a `fixme`; it now runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…guards The claim that Sentry's instrumentation destabilises the production server was wrong, and the docs said so in three places. Re-tested as a matrix: a normal production build carries Sentry without the MSW preload and survived three cold starts x 36 concurrent requests with zero errors, where the mocked build with both produced 185 in one run. It is an interaction between two libraries that each hook module loading, and production never loads the preload. The mocked build still skips `withSentryConfig` — that keeps the suite stable and keeps its synthetic failures out of the real project. Also in this commit, from the same working tree: - The mock-role flag becomes `NEXT_PUBLIC_E2E_MOCK_ROLE`. `sentry-base-config.ts` is imported by the browser SDK too, and Next inlines only `NEXT_PUBLIC_` vars there — unprefixed, the client read it as undefined and kept reporting from E2E runs. - `globalSetup` now compares the build id the running server booted from, not just the two on-disk records. A rebuild rewrites both at once, so a server left running from the previous build agreed with them — and `reuseExistingServer` makes that the likely case, not the exotic one. - `--check` runs in CI, not pre-commit; the docblock said otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Preview deployment: https://native-product-pages-v2.preview.avy-fx.org |
Contributor
|
Here's a visual recap of what changed: Open the full interactive recap |
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
1 participant
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.
Description
Deterministic, offline end-to-end coverage for the native AFP product pages, with MSW intercepting the AFP API at the Node network layer inside Next and Playwright driving the browser.
pnpm test:e2e:native— 25 passing, 5 skipped, in its own CI job.It also carries three behaviour fixes, each in its own commit, because building the suite surfaced them. Two of them are why this PR is more than tests.
Related Issues
native-product-pages, so it will not auto-close; close by hand once this lands.Key Changes
The seam is a
NODE_OPTIONS --importpreload, notsrc/instrumentation.ts. Next skips the instrumentation hook during a production build, so mocks started there could never shapegenerateStaticParamsor any prerendered page — which is exactly the HTML a reader is served. The preload reaches the build workers, so the suite asserts against real prerendered output.An unmocked run is impossible rather than merely detected. The mocked build and server point
NAC_HOST/AFP_HOSTat the reserved.invalidTLD, which the mock handlers read too — so they agree by construction, while a process where interception failed gets a DNS failure instead of real forecast data. Plus a boot probe, a separatedistDir, and a loud 501 + run failure for any unmapped request. That last one matters because these pages degrade to visible copy rather than crashing, so a missing fixture would otherwise read as a passing test of a degraded page.Fixtures are captured, never hand-authored. The products-api golden corpus is vendored with a source commit and a sha256 per file, gated in CI. Two responses the corpus does not cover yet (
/v2/public/avalanche-center/{NWAC,SAC}) sit in a clearly markedprovisional/staging area — the build cannot complete without them — each carrying the upstream Case that deletes it.Three fixes, worth reviewing separately:
d50c999d— zone slugs containing&never resolved. Next hands a dynamic segment percent-encoded, so three of Sawtooth's four zones rendered "Zone not found", including from their own links on the all-zones grid.e7b33c66— a correction 404'd the page it was correcting.revalidateTagis a hard cache invalidation; withdynamicParams = falseNext abandoned the route and[center]/[...segments]answered 404, for ~70s, on the corrected zone and every zone sharing its weather product. The mechanism meant to deliver a correction removed the forecast instead. Nothing reached production — the rollout flag is off for every center.3b4c9f77— forecast media thumbnails had no accessible name (WCAG 4.1.2).Rollout state is fixed per tenant in the seed (snfac and dvac native, nwac and sac on the widget) so no test mutates state another worker can observe. This changes what every developer's seeded database looks like.
How to test
The 404 fix reproduces without Playwright:
pnpm e2e:build && pnpm e2e:start, confirm the four Sawtooth zones answer 200,curlthe freshness endpoint for one of them, and watch two of them turn into 404s on the pre-fix commit.Mechanism, fixture provenance, and the full blocked list are in
docs/afp-products/e2e-mocks.md.Screenshots / Demo video
n/a — the artifact is the suite.
Migration Explanation
None. The seed change is data, not schema.
Future enhancements / Questions
periodsweather-table shape has no example I could find — every SNFAC weather product I sampled in 2026 and 2021 iscolumns/rows, and NWAC published zero v2 weather products in January 2026. Open question for the AFP.withSentryConfig. Sentry and the MSW preload both hook module loading and together produce intermittent 5xx; a normal production build carries Sentry without the preload and does not reproduce it, so this is a harness interaction rather than a production defect.frontendE2E project still loads the widget CDN for real and still builds against the live AFP API. Making the whole suite hermetic is a natural follow-up once the corpus covers the other centers — deliberately out of scope here.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.