test: serve a fresh mock response per fetch in the keep-alive test - #802
Open
dealerweb wants to merge 1 commit into
Open
test: serve a fresh mock response per fetch in the keep-alive test#802dealerweb wants to merge 1 commit into
dealerweb wants to merge 1 commit into
Conversation
The keep-alive test parked one shared Response instance in mockResolvedValue, but a Response body is single-use - as soon as more than one request lands in the 30s window the second read dies with "Body is unusable" and the ping reports a lost connection. Serve a fresh Response per call via mockImplementation, the pattern the rest of this file already uses.
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.
Summary
npx vitest runcurrently fails onmain: the keep-alive test inlib/__tests__/jmap-client-resilience.test.tsreports a lost connection that never happened. The test parks a singleResponseinstance inmockResolvedValue, but a Response body is single-use — as soon as more than one request lands in the simulated 30s window, the second read dies with "Body is unusable: Body has already been read", the ping counts as failed, and theonConnectionChange(true)assertion seesfalse. Recent client changes made a second request in that window the norm, so what used to be an occasional flake under parallel load is now a deterministic failure.Changes
Responseper fetch call viamockImplementation— the pattern the rest of this file already uses — instead of one shared instance.Related issues
None — test-only repair; the production client reads each response exactly once, so there is no runtime counterpart to this failure.
Type of change
Checklist
npm run typecheck && npm run lintand there are no errorsnpm run build)locales/) if my changes affect user-facing textScreenshots / demo
No UI involved — one-line test fix.
Notes for reviewers
mainwithnpx vitest run lib/__tests__/jmap-client-resilience.test.ts: the keep-alive test fails with the "Body is unusable" stderr trace; with this change the file passes 33/33.npm run typecheck,npm run lintand the fullnpx vitest run.