also looking at this
Close unauthenticated /api/requests CRUD, and make the session cookie HttpOnly
#77
11 files
+552
-57
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -1,4 +1,29 @@ | ||
| 1 | import { expect, test } from 'bun:test' | |
| 1 | import { afterAll, beforeAll, expect, test } from 'bun:test' | |
| 2 | import { GlobalRegistrator } from '@happy-dom/global-registrator' | |
| 3 | ||
| 4 | /** | |
| 5 | * DOM-dependent tests. | |
| 6 | * | |
| 7 | * The DOM is registered HERE rather than in `tests/setup.ts`, because | |
| 8 | * `GlobalRegistrator.register()` swaps `Request`/`Response`/`Headers` for | |
| 9 | * happy-dom's browser implementations, and a browser `Headers` silently drops | |
| 10 | * `Set-Cookie` and `Cookie` (the Fetch spec calls them forbidden header names). | |
| 11 | * Registering it globally therefore made `tests/unit/session-cookie.test.ts` | |
| 12 | * unable to see the `HttpOnly` session cookie it exists to assert. See | |
| 13 | * `tests/setup.ts` for the measurement. | |
| 14 | * | |
| 15 | * Bun runs test files in one process, so the registration is bracketed: any | |
| 16 | * file that needs a DOM takes it for its own duration and hands the native | |
| 17 | * globals back afterwards, instead of mutating the whole run. | |
| 18 | */ | |
| 19 | ||
| 20 | beforeAll(() => { | |
| 21 | GlobalRegistrator.register() | |
| 22 | }) | |
| 23 | ||
| 24 | afterAll(async () => { | |
| 25 | await GlobalRegistrator.unregister() | |
| 26 | }) | |
| 2 | 27 | |
| 3 | 28 | test('dom test', () => { |
| 4 | 29 | document.body.innerHTML = `<button>My button</button>` |
| @@ -1,4 +1,3 @@ | ||
| 1 | import { GlobalRegistrator } from '@happy-dom/global-registrator' | |
| 2 | 1 | import { setupTestEnvironment } from '@stacksjs/testing' |
| 3 | 2 | |
| 4 | 3 | /** |
| @@ -8,33 +7,31 @@ import { setupTestEnvironment } from '@stacksjs/testing' | ||
| 8 | 7 | * e.g. you may abstract your module mocks here, if you want |
| 9 | 8 | * to prevent the original module from being evaluated. |
| 10 | 9 | * |
| 11 | * ## Why happy-dom is registered here | |
| 10 | * ## Why the DOM is NOT registered here | |
| 12 | 11 | * |
| 13 | * `bun test` runs in a plain server runtime with no `document`, and | |
| 14 | * `setupTestEnvironment()` does not add one — `@stacksjs/testing` depends only | |
| 15 | * on `@stacksjs/auth` and `@stacksjs/ts-cloud`, and calling it leaves | |
| 16 | * `globalThis.document` undefined. So `tests/feature/ui.test.ts`, which came | |
| 17 | * with the project scaffold and touches `document.body`, failed with | |
| 18 | * `ReferenceError: document is not defined` from the first commit onward. The | |
| 19 | * `test` job has been red on `main` ever since, which also meant it could not | |
| 20 | * report a real regression. | |
| 12 | * `bun test` has no `document`, and `setupTestEnvironment()` does not add one | |
| 13 | * (`@stacksjs/testing` depends only on `@stacksjs/auth` and | |
| 14 | * `@stacksjs/ts-cloud`). happy-dom supplies it — but registering it *globally* | |
| 15 | * from this preload breaks server-side tests, so it is done per-file instead. | |
| 21 | 16 | * |
| 22 | * `GlobalRegistrator.register()` installs happy-dom's `window`/`document` onto | |
| 23 | * the global scope — the pattern Bun documents for DOM testing — so a test that | |
| 24 | * needs a DOM has one, and the suite stops being decorative. | |
| 17 | * `GlobalRegistrator.register()` swaps `Request`, `Response` and `Headers` for | |
| 18 | * happy-dom's BROWSER implementations, and the Fetch spec classes `Set-Cookie` | |
| 19 | * and `Cookie` as forbidden header names — so a browser `Headers` silently | |
| 20 | * drops them. Measured directly: | |
| 25 | 21 | * |
| 26 | * Registered BEFORE `setupTestEnvironment()` deliberately: anything the | |
| 27 | * framework's setup does that sniffs for a browser-ish global should see the | |
| 28 | * DOM already in place rather than racing it. | |
| 22 | * new Response('x', { headers: { 'Set-Cookie': 'a=b; HttpOnly' } }) | |
| 23 | * .headers.get('set-cookie') | |
| 24 | * // happy-dom registered -> null | |
| 25 | * // after unregister() -> "a=b; HttpOnly" | |
| 29 | 26 | * |
| 30 | * Note this makes every test file DOM-ish. That is fine for this project (the | |
| 31 | * only non-DOM concerns here are pure functions and `Response` objects, neither | |
| 32 | * of which care), but if a test ever needs to assert genuinely server-only | |
| 33 | * behaviour, move that file out from under this preload rather than | |
| 34 | * conditionally unregistering — partial teardown of a registered global DOM is | |
| 35 | * its own source of cross-test bleed. | |
| 27 | * With a global DOM, `tests/unit/session-cookie.test.ts` — which exists | |
| 28 | * precisely to prove the session cookie carries `HttpOnly` — could not observe | |
| 29 | * a single `Set-Cookie` header, and three of its assertions failed against code | |
| 30 | * that was working correctly. A test environment that silently hides the | |
| 31 | * security property under test is worse than one with no DOM at all. | |
| 32 | * | |
| 33 | * So: a test that needs a DOM registers it itself and tears it down after. See | |
| 34 | * `tests/feature/ui.test.ts` for the pattern. | |
| 36 | 35 | */ |
| 37 | 36 | |
| 38 | GlobalRegistrator.register() | |
| 39 | ||
| 40 | 37 | setupTestEnvironment() |