Harden non-streaming fallback tests for fetchWithCorsProxy#3495
Harden non-streaming fallback tests for fetchWithCorsProxy#3495
Conversation
Follow-up to #3440. The non-streaming fallback tests relied on implicit mock ordering to simulate an unsupported streaming environment. On Node.js/V8 (where streaming IS supported), the GET test was not actually exercising the non-streaming path. Force `supportsReadableStreamBody()` to return `false` via a new `__testing.setStreamBodySupported()` setter and assert it in beforeEach, so every test positively confirms it runs in non-streaming mode. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Strengthens the “non-streaming fallback” test suite for fetchWithCorsProxy by deterministically forcing supportsReadableStreamBody() to report “unsupported”, ensuring tests actually exercise the intended fallback path.
Changes:
- Added a
__testing.setStreamBodySupported(false)hook to force non-streaming mode in tests. - Updated the non-streaming fallback test setup to assert the forced mode and simplified fetch mock ordering/call counts accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/php-wasm/web-service-worker/src/utils.ts | Adds a test-only setter to control the cached supportsReadableStreamBody() result. |
| packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.spec.ts | Forces non-streaming mode in beforeEach and updates mocks/assertions to match the new deterministic behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| __testing.setStreamBodySupported(false); | ||
| expect(await supportsReadableStreamBody()).toBe(false); | ||
| }); | ||
|
|
There was a problem hiding this comment.
This beforeEach mutates module-level cached state (streamBodySupported) but there’s no corresponding cleanup after the describe block. That can leak into later tests in the same file (or other files) that rely on the default probe behavior. Add an afterEach/afterAll in this describe to call __testing.resetStreamBodySupported() (or otherwise restore the previous value) to keep the suite hermetic.
| afterEach(() => { | |
| __testing.resetStreamBodySupported(); | |
| }); |
Summary
Follow-up to #3440 addressing Brandon's review feedback about strengthening the non-streaming fallback tests.
beforeEach()in the "non-streaming fallback" describe block now explicitly forcessupportsReadableStreamBody()to returnfalse(via a new__testing.setStreamBodySupported()setter) and asserts it, so every test positively confirms it's running in non-streaming mode.[direct, probe, proxy]to[direct, proxy]since the probe no longer fires when the result is pre-cached.Test plan
fetch-with-cors-proxy.spec.tspassMade with Cursor