Astro integration test - #3
Merged
Merged
Conversation
`readDocumentHead` awaited `reader.cancel()` in its `finally`. Releasing the socket is cleanup — the text is already in hand — so waiting for it buys nothing, and a stream whose `cancel()` never settles hangs the call forever. The request deadline cannot rescue that case. `AbortSignal.timeout` covers getting the response and reading the body, but by the time cancel runs the response has already been delivered, so nothing interrupts the wait. Measured: a hung cancel blocked for the full 120s test timeout with `fetch.timeout` at its 5s default. Found while wiring up an Astro integration test against MSW, whose mocked response bodies exhibit exactly this: reading one chunk and then awaiting `reader.cancel()` never resolves, while `fetch(...).then(r => r.text())` on the same handler is fine. Isolated to three cases before changing anything: OK fetch + text() text/html | 95b HANG read one chunk then reader.cancel() TIMED OUT HANG plugin via markdownToHtml TIMED OUT Real undici streams settle cancel promptly, so this is not a bug users are hitting today — but the fix is justified on its own terms rather than to accommodate a test double: an unbounded wait on best-effort cleanup, immune to the timeout, is worth removing wherever it came from. Any interceptor or proxy in a consumer's environment could behave the same way. Coverage stays at 100%; the rejection handler is exercised by the existing stalled-body test, which errors the stream so that cancel rejects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The suite drove `markdownToHtml` from Sätteri directly, which proved the plugin
against Sätteri but never against the thing it exists for: Astro's wiring. This
runs a real `astro build` with the plugin registered exactly as the Astro docs
prescribe — `markdown.processor: satteri({hastPlugins: [...]})` — and asserts
the built HTML carries a title fetched over a mocked network.
Verified rather than assumed, since these were the load-bearing unknowns:
- A `.md` page under `src/pages/` really does route through
`markdown.processor`: astro's `vite-plugin-markdown` `load` hook calls
`processor.createRenderer(...).render(...)` inline.
- The build stays in the calling process (no child processes or workers in its
build path), which is why MSW's `globalThis.fetch` patch reaches the plugin.
- `satteri()` keeps the `hastPlugins` array by reference and Astro's zod schema
preserves that identity, so the instance constructed in the test is the one
that runs.
Kept off the release path: its own config, script and CI job on a single Node
version. `prepublishOnly` runs `test:coverage`, so folding a ~290-package Astro
build into the default suite would let an unrelated Rolldown or napi hiccup
block publishing a package whose own code is unchanged. `vitest.config.ts`
excludes the directory by spreading `defaultExclude` — note vitest 4 flattened
`configDefaults`, so `configDefaults.test.exclude` is `undefined` and a bare
`exclude` would silently start globbing node_modules.
Containment, all of it needed:
- `ASTRO_TELEMETRY_DISABLED=1` (saved and restored). Without it `build()` writes
a config file under the user's home directory and POSTs to
telemetry.astro.build, which the unhandled-request guard would then fail on.
- `outDir` and `cacheDir` inside the fixture, *under cwd*. Astro's
`getOutDirWithinCwd` silently redirects the intermediate build to
`<cwd>/.astro/` when `outDir` escapes cwd — a temp dir put build droppings in
the repo root, which the first version of this test did. Asserted in
`beforeAll` so it cannot regress.
- `cache: memoryCache()`, or the default lowdb cache writes ./db.titles.json
into the repo. Asserted absent at the end.
- `onUnhandledRequest` as a callback, not `'error'`: MSW skips the string
strategies for "common asset" requests — anything ending .html/.js/.json,
`file:` URLs, paths containing node_modules — and lets those reach the real
network. The callback branch returns before that check, so it sees everything.
The test proves it can fail, which is the part worth stating. Mutating the
served title while leaving the expectation alone fails with
`expected 'A Different Title' to be 'Example Domain (from MSW)'`; pointing the
fixture link at an unmocked URL fails six assertions and trips MSW's guard. An
earlier mutation that changed the shared constant passed — it moved both sides
at once and proved nothing, which is why the mock title is deliberately not the
real example.com title: a silent fall-through to the internet yields a wrong
value, not a green test.
pnpm-workspace.yaml now answers pnpm 11's install-script review: esbuild
allowed (its postinstall links the platform binary Vite and Astro shell out
to), msw skipped (its postinstall only copies a browser Service Worker, and
this repo uses `msw/node`).
197 unit tests at 100% coverage, unchanged; 11 integration tests in ~2s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
No description provided.