Repository navigation
test(router): content-negotiation HTTP-only e2e moved to dispatch and serveShellRequest unit suites (S22 pilot) - #1255
Merged
Merged
Conversation
…st unit suites (S22 pilot) The HTTP-only content-negotiation e2e cases (79 tests across the router test-app and cloudflare-basic, dev and production) move to unit suites that run the production request path: dispatch for a response variant's win, serveShellRequest for a page route's win and its document or flight representation. Each app keeps one dev and one production smoke for what a unit suite cannot see: the route trie the build emits and the real SSR document (and workerd, in cloudflare-basic). cloudflare-basic's negotiated routes move into src/pages/negotiation.tsx, included without a name scope, so its unit suite dispatches the app's own routes. Route names and router.named-routes.gen.ts are unchanged. The test-app's /negotiate-mw-test, used only by the removed e2e cases, is removed and its generated route files regenerated. The testing skill notes that a negotiated URL's page-route win goes through serveShellRequest.
49 of 50 tasks
commit: |
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
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.
devcorner: rango-t150 · session rango-s307
Pilot: content-negotiation HTTP-only e2e to
dispatch/serveShellRequestunit suites (S22)This is the one-area pilot the owner approved for S22 (rango-d195, section 7). Hard rule 3 is unchanged by this PR; the owner decides the remaining areas from this result.
Problem statement
The content-negotiation contract (Accept ranking, definition-order fallback, the HTML document versus the flight stream, variant middleware,
Vary: Accept) was pinned by 79 e2e tests across two apps, every one of them an HTTP request with no page (nopage.orgoto(in either file): 39 in the router test-app (23 dev + 16 production) and 40 in cloudflare-basic (24 dev + 16 production). Since S14/S14b,dispatch()runs the production request stages, andserveShellRequestrunscreateRSCHandleritself, so the same contract can be pinned in unit suites through the public testing primitives, keeping e2e for what only a real build and server can show.Before / after (test shape)
Before, one e2e test per Accept case, written twice (dev and production):
After, the case is a unit row through the public primitive, with the exact content type, the body, and the Vary token checked on every negotiated response:
An Accept the page route wins cannot go through
dispatch(it refuses component routes by design), so those rows go throughserveShellRequest, which also decides the representation:Code-change summary
packages/rangojs-router/src/testing/__tests__/content-negotiation.rsc-test.tsx(new, 22 tests, rsc project): a self-contained router with the test-app's negotiated URL shapes (/negotiate-test: RSC, JSON, Markdown;/negotiate-test-json-first: JSON, RSC),/negotiate-mw-test(a middleware per variant) and a plain route. The test-app has no unit harness, and importinge2e/test-app/srcfrom the router'ssrc/would cross the package's typecheck boundary, so the router suite builds its own routes, as the router's other suites do.tests/cloudflare-basic/test/content-negotiation.rsc-test.tsx(new, 17 tests, rsc project): the app's own negotiated routes.tests/cloudflare-basic/src/pages/negotiation.tsx(new) +negotiation.gen.ts: the app's negotiated routes move out ofsrc/urls.tsxintonegotiationPatterns, mounted withinclude("/", negotiationPatterns, { name: "" })(theholeyThrowsPatternsshape), so the unit suite imports the real routes. Route names are unchanged;router.named-routes.gen.tsdid not change after a dev run and a build.packages/rangojs-router/e2e/content-negotiation.test.ts(539 to 64 lines) andtests/cloudflare-basic/e2e/content-negotiation.test.ts(457 to 63 lines): one smoke each, in a dev and a(production)describe, through onenegotiationSmoke(f: Fixture)per file. The smoke retries a cold dev route's first request (toPass, 10 s), as the removed suite did for one case.packages/rangojs-router/e2e/test-app/src/urls.tsx:/negotiate-mw-testremoved (only the removed e2e cases used it);urls.gen.tsandrouter.named-routes.gen.tsregenerated.packages/rangojs-router/skills/testing/response-routes.md:dispatchis scoped to negotiation won by a response route, and a new caveat says to serve a page-route win throughserveShellRequest.Lines: e2e -869 net (996 to 127), unit +446, cloudflare-basic app +21 net (
negotiation.tsx58 andnegotiation.gen.ts16 new;urls.tsx+3 -56), test-app -33.What stays in e2e, and why
each route of a negotiated URL answers through the route trie:/negotiate-testwith*/*is the SSR document containing "Negotiate Test RSC";/negotiate-testwithtext/markdownis the third route's Markdown;/negotiate-test-json-firstwith*/*is{ source: "json" };acceptonce in Vary on all threeloadManifestloads the winner) and itsrfpage-route-first flag.dispatchandserveShellRequestrebuild the trie from the routes at runtime (ensureRouterTrie), so a build that dropped or reordered variants is invisible to them. The SSR step is stubbed inserveShellRequest(the body is the Flight payload), so the real HTML document is e2e-only. Variant middleware is collected at runtime from the entry the variant's route key loads (collectRouteMiddlewareinrouter/content-negotiation.ts), so the unit suite pins it (mutation M6).a negotiated URL serves each pipeline through workerd:/test/negotiatewith*/*is the bare JSON, with a browser Accept the SSR document containing "HTML version";/test/negotiate-wild/deep/pathwithapplication/jsonkeeps its wildcard param;acceptonce in Vary on all threeAcceptadded in the worker.No removed test depended on
NODE_ENVfolding: each production test asserted the same contract as its dev twin. Both rsc unit projects run withNODE_ENV=production(vitest.rsc.config.ts).Mapping: removed e2e test to the unit test that pins it
Router test-app (
packages/rangojs-router/e2e/content-negotiation.test.ts). Unit tests are in the router'scontent-negotiation.rsc-test.tsx: describes "a response variant wins (dispatch)" (V), "the RSC route wins (serveShellRequest)" (R), "a plain RSC route (no variants)" (P).Accept: text/markdown,text/html,*/* prefers markdown (listed first)client order: markdown listed firstAccept: text/html,text/markdown,*/* prefers HTML (listed first)client order: HTML listed firstAccept: application/json,text/html prefers JSON (listed first)client order: JSON listed firsttext/markdown;q=0.5 loses to text/html;q=1.0q-value: text/markdown;q=0.5 loses to text/htmltext/html;q=0.9 loses to text/markdown;q=1.0q-value: text/html;q=0.9 loses to text/markdownapplication/json;q=0.8 loses to text/markdown;q=0.9q-value: application/json;q=0.8 loses to text/markdown;q=0.9browser Accept header returns HTML (text/html is q=1.0, first)a browser AcceptAccept: text/markdown returns markdownsingle type: text/markdown; also the kept smokeAccept: application/json returns JSONsingle type: application/jsonAccept: */* serves the HTML document when RSC is defined first*/* picks the RSC route defined first; also the kept smokeAccept: */* returns JSON when JSON is defined first*/* picks the JSON route defined first; also the kept smokeno Accept header serves the HTML document when RSC is defined firstan empty Accept picks the RSC route defined firstno Accept header returns first defined (JSON)an empty Accept picks the JSON route defined first, Vno Accept picks the JSON route defined firstAccept: text/x-component returns the flight stream (RSC first)text/x-component on an RSC-first routeAccept: text/x-component returns the flight stream (JSON first)text/x-component on a JSON-first routeAccept: */* serves the HTML document*/*no Accept header serves the HTML documentan empty AcceptAccept: application/json serves the HTML document (no JSON variant)application/json (no JSON variant)Accept: text/x-component serves the flight streamtext/x-componentall negotiated responses include Vary: AcceptexpectVaryAcceptOnce)emits Accept in Vary exactly once (no duplicate token)Accept: application/json runs JSON variant middlewareruns the winning variant's own middlewareAccept: text/html runs RSC variant middlewareruns the RSC route's middleware when the document winscloudflare-basic (
tests/cloudflare-basic/e2e/content-negotiation.test.ts). Unit tests are in cloudflare-basic'stest/content-negotiation.rsc-test.tsx(V, R) unless marked "router". A production title is the dev title's case with "in production".Accept: text/html returns HTML (RSC pipeline)JSON-first: a browser Accept; also the kept smokeAccept: application/json returns bare JSONJSON-first: application/jsonno Accept header returns JSON (negotiate handler)JSON-first: an empty Accept picks JSON, VJSON-first: no Accept picks JSONAccept: */* returns JSON (no explicit text/html)JSON-first: */* picks JSON; also the kept smokeAccept: text/x-component returns the flight stream on a JSON-first routeJSON-first: text/x-componentplain RSC route: Accept: */* serves the HTML document*/*(framework behaviour with no app route involved)plain RSC route: Accept: text/x-component serves the flight streamtext/x-component(same)Vary: Accept present on negotiated JSON responseJSON-first: application/json; also the kept smokeVary: Accept present on HTML response from negotiated URLJSON-first: a browser Accept; also the kept smoketext negotiate: Accept text/html returns HTMLtext: a browser Accepttext negotiate: no text/html returns plain texttext: text/plaintext negotiate: Vary: Accept presenttext: text/plainxml negotiate: Accept text/html returns HTMLxml: a browser Acceptxml negotiate: no text/html returns XMLxml: application/xmlxml negotiate: Vary: Accept presentxml: application/xmlmulti negotiate: text/html returns HTMLmulti: a browser Acceptmulti negotiate: application/json returns JSONmulti: application/jsonmulti negotiate: text/plain returns textmulti: text/plainmulti negotiate: application/xml returns XMLmulti: application/xmlmulti negotiate: */* returns first variant (json)multi: */* picks the first variant (JSON)wildcard negotiate: text/html returns HTMLwildcard: a browser Acceptwildcard negotiate: application/json returns JSON with wildcard paramwildcard: application/json carries the wildcard param; also the kept smokewildcard negotiate: Vary: Accept presentwildcard: application/json carries the wildcard parammulti negotiate: Vary: Accept on all negotiated responsesmulti: application/json,multi: text/plain,multi: application/xmlThe unit rows assert more than the e2e tests did: the exact content type (the cloudflare-basic HTML cases only asserted "not JSON"), the body, the document or flight representation of each page-route win, and
acceptexactly once in Vary on every negotiated response (cloudflare-basic only asserted "contains").Red before green: mutation matrix
Each mutation was applied alone to the negotiation code, both new suites were run, and the mutation was reverted (the source files show no diff afterwards). All 39 unit tests pass on the unmodified code, and every one of them fails under at least one mutation:
pickNegotiateVariantreturns the first candidate (ignores Accept)pickNegotiateVariantreturns the last candidateacceptsFlightExplicitlyback to the old rule (notext/htmlin Accept means flight)prefersFlightRepresentationreturns falseappendVaryAcceptappends without the dedup (D6)Per test
client order: markdown listed firstclient order: JSON listed firstq-value: text/html;q=0.9 loses to text/markdownq-value: application/json;q=0.8 loses to text/markdown;q=0.9single type: text/markdownsingle type: application/json*/* picks the JSON route defined firstan empty Accept picks the JSON route defined firstno Accept picks the JSON route defined firstruns the winning variant's own middlewareclient order: HTML listed firstq-value: text/markdown;q=0.5 loses to text/htmla browser Accept*/* picks the RSC route defined firstan empty Accept picks the RSC route defined firsttext/x-component on an RSC-first routetext/x-component on a JSON-first routeruns the RSC route's middleware when the document wins*/*an empty Acceptapplication/json (no JSON variant)text/x-componentJSON-first: application/jsonJSON-first: */* picks JSONJSON-first: an empty Accept picks JSONJSON-first: no Accept picks JSONtext: text/plainxml: application/xmlmulti: application/jsonmulti: text/plainmulti: application/xmlmulti: */* picks the first variant (JSON)wildcard: application/json carries the wildcard paramJSON-first: a browser AcceptJSON-first: text/x-componenttext: a browser Acceptxml: a browser Acceptmulti: a browser Acceptwildcard: a browser AcceptTiming (local, this machine)
Each e2e run's wall-clock is almost all server start and build (a dev server, a production build and a production server per app), which a full e2e run pays once for all suites; across the runs here it varied from 15 to 44 s (router) and 29 to 56 s (cloudflare-basic) for the same files, so the before/after wall-clock difference is noise. The removed tests' own cost was small: 4-50 ms per request once the servers were warm, 6.9 s summed across both apps. The kept smokes sum to 5.6 s in the final runs (router 1.3-2.3 s and cloudflare-basic 2.2-4.3 s across the runs with no other e2e run alongside), almost all of it the dev smoke's first request paying the cold route compile that the 47 dev tests used to share. So this area saves almost no local e2e time. CI e2e job time before and after: to be read from this PR's CI run against the base run; the results will be added here.
Test plan
content-negotiation.rsc-test.tsx(22 passed) and cloudflare-basiccontent-negotiation.rsc-test.tsx(17 passed); each test red under a mutation (matrix above).content-negotiationandcontent-negotiation (production)2/2; cloudflare-basiccontent negotiation (dev)andcontent negotiation (production)2/2.pnpm run typecheck: 16/16 tasks.pnpm run test:unit:all: router 9354 passed (5 skipped, 6 todo) and rsc 1089 passed (3 expected fail); cloudflare-basic 90 and rsc 56; vite-rsc-demo 27 and 1; mini 25 and 3; shared-e2e 20. No failures.oxlint --deny-warnings(0 warnings, 0 errors),oxfmt --check ..check-e2e-bucketing,check-e2e-parity --strict(it reports this file as a gap when the(production)describe is removed),check-e2e-console-baseline,check-suspense-contract,check-no-external-store,node --test tools/*.test.mjs(53 passed),check-doc-links,check-docs-api,check-skills,generate-skills-catalog --check,generate-route-types --check,check-internals.git status --porcelain | grep '\.gen\.ts'empty;negotiation.gen.ts, test-appurls.gen.tsandrouter.named-routes.gen.tscommitted.Practices
oxlint --deny-warnings0 findings andoxfmt --check .clean on the change; the pre-commit hooks (format, lint, e2e bucketing, route-gen, no-external-store) passed on each commit./negotiate-mw-testroutes were used only by the removed e2e cases, so they are removed and the generated route files regenerated; a repo-wide search (src, tests, e2e, docs, skills, tools) finds no reference left. Kept on purpose: the test-app's/negotiate-testJSON and Markdown routes and/negotiate-test-json-first(the smoke requests them), and cloudflare-basic's negotiated routes (its unit suite and smoke use them).dispatchandserveShellRequest, each shown red under a mutation; the kept smokes in dev and production in both apps;test:unit:alland typecheck. The full e2e suite runs on CI.acceptonce in Vary on workerd, and retry a cold dev route's first request; the middleware rows check status and content type, and the page-route one is titled by what it checks; the smokes take the sharedFixturetype; both unit suites use one row shape; cloudflare-basic's suite covers only the app's own routes; the skill note is scoped. Left, with reasons: production e2e for every Accept case (the pilot's premise; the build-only data and SSR are in the smokes, and the rest is runtime code the unit suites run); oneserveShellRequesttable for every row (dispatchis the HTTP-only primitive this pilot evaluates, and the split pins which pipeline answers); the router suite's own routes instead of importing the test-app (typecheck boundary, above);expectVaryAcceptOncein four files (PlaywrightAPIResponseand fetchResponsediffer, and a shared matcher would be new public testing API); thetoPassretry also wrapping the production smoke (it only delays a real failure's report by up to 10 s); unit rows that overlap the internal unit tests ofcontent-negotiation.ts,ssr-setup.tsandresponse-utils.ts(the userland rule asks for the public primitives on top of them).skills/testing/response-routes.mdnow scopesdispatchto negotiation won by a response route and says to serve a page-route win throughserveShellRequest. No router source file was added or removed, sofeature-map.mdandfeature-file-map.mdare unchanged.Notes
dispatchcannot answer a page-route win (it throws on component routes by design), so 18 of the 39 negotiation rows go throughserveShellRequestand the rsc (react-server) unit project. Areas whose cases end in a component render will lean onserveShellRequestthe same way; areas that end in a response route (response routes, redirects) fitdispatchalone.