Repository navigation
test(build): make the fake fetch method-aware and header-readable - #297
Merged
Merged
Conversation
netravnen
force-pushed
the
test/browser-shim-fetch-fidelity
branch
from
August 19, 2026 22:27
f11585b to
0fd347f
Compare
Base automatically changed from
fix/dp-whitelist-hostname-validation
to
dev-next
August 19, 2026 22:28
The node:test harness could not express the cases needed to test
lib/admincom-common.js's retry/backoff wrappers, which is why that code
shipped with a defect nothing caught.
Two limits caused it. The fake response exposed only headers.forEach(),
while fetchWithRetry reads response.headers.get('retry-after') -- so any
503 fixture threw TypeError before reaching the logic under test. And the
fake fetch was `async (url) =>`, dropping the init argument entirely, so a
test could not observe the request method or count requests. Since a
retried request and a single one return the same value, request count was
the only observable that distinguishes them, and it was unavailable.
This closes both so the following commit can test the retry policy, and
pins the harness's own capabilities: a gap in the harness is invisible in
a green run, and produces confident, empty coverage.
Changes:
- Extracted makeFakeResponse(status, body, headers) with a real
case-insensitive headers.get()/has()/forEach(), correct ok derivation,
and json()/text().
- makeFakeFetch now takes (url, init) and appends { url, method, body,
headers } to a per-load call log, returned from loadScript as
`fetchCalls`.
- fetchMap entries may now be a response descriptor
({ __response: true, status, body, headers }) to model failures and
header-bearing responses; a bare JSON body still means 200, unchanged.
- Added tests/browser-shim.test.js asserting these capabilities directly.
Security:
- N/A -- test harness only, never inlined into a generated .user.js.
Testing:
- node --test from user.js/: 480 tests, 479 pass, 1 skipped (live tests
are opt-in), up from 476/475.
- All 476 pre-existing tests pass unchanged, confirming the bare-body
fetchMap form and the 404-on-unmapped-URL behavior are preserved.
Backwards Compatibility:
- Purely additive for existing callers: the descriptor form is opt-in via
an explicit __response marker, and loadScript's return value gains a
property rather than changing one.
Assisted-by: Claude:claude-opus-5
netravnen
force-pushed
the
test/browser-shim-fetch-fidelity
branch
from
August 19, 2026 22:28
0fd347f to
c2f41e1
Compare
netravnen
marked this pull request as ready for review
August 19, 2026 22:29
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.
The node:test harness could not express the cases needed to test
lib/admincom-common.js's retry/backoff wrappers, which is why that code
shipped with a defect nothing caught.
Two limits caused it. The fake response exposed only headers.forEach(),
while fetchWithRetry reads response.headers.get('retry-after') -- so any
503 fixture threw TypeError before reaching the logic under test. And the
fake fetch was
async (url) =>, dropping the init argument entirely, so atest could not observe the request method or count requests. Since a
retried request and a single one return the same value, request count was
the only observable that distinguishes them, and it was unavailable.
This closes both so the following commit can test the retry policy, and
pins the harness's own capabilities: a gap in the harness is invisible in
a green run, and produces confident, empty coverage.
Changes:
case-insensitive headers.get()/has()/forEach(), correct ok derivation,
and json()/text().
headers } to a per-load call log, returned from loadScript as
fetchCalls.({ __response: true, status, body, headers }) to model failures and
header-bearing responses; a bare JSON body still means 200, unchanged.
Security:
Testing:
are opt-in), up from 476/475.
fetchMap form and the 404-on-unmapped-URL behavior are preserved.
Backwards Compatibility:
an explicit __response marker, and loadScript's return value gains a
property rather than changing one.
Assisted-by: Claude:claude-opus-5
Stack created with GitHub Stacks CLI • Give Feedback 💬