Skip to content

Harden non-streaming fallback tests for fetchWithCorsProxy - #3495

Merged
brandonpayton merged 1 commit into
trunkfrom
fix/non-streaming-fallback-tests
Jul 17, 2026
Merged

Harden non-streaming fallback tests for fetchWithCorsProxy#3495
brandonpayton merged 1 commit into
trunkfrom
fix/non-streaming-fallback-tests

Conversation

@ashfame

@ashfame ashfame commented Apr 15, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #3440, specifically addressing Brandon's comments the request to assert unsupported streaming in each fallback test and the concern that resetting the cache allows Node.js/V8 to detect streaming support.

  • The non-streaming fallback suite now sets the cached supportsReadableStreamBody() result to false in beforeEach() and asserts that value before every test runs.
  • The suite resets that test-only cache in afterEach(), keeping the test state hermetic.
  • The proxy-retry test now correctly expects only direct-fetch and proxy-fetch calls, because no detection probe runs when unsupported mode is already cached.

Test plan

  • npm exec nx test php-wasm-web-service-worker --testFile=fetch-with-cors-proxy.spec.ts
  • npm exec nx lint php-wasm-web-service-worker
@ashfame
ashfame requested review from a team, bgrgicak and Copilot April 15, 2026 07:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ashfame
ashfame marked this pull request as draft April 16, 2026 18:53
@brandonpayton

Copy link
Copy Markdown
Member

@ashfame Thanks for faithfully following up on prior PR review comments. Let's merge this!

@brandonpayton
brandonpayton merged commit 8917cb4 into trunk Jul 17, 2026
56 checks passed
@brandonpayton
brandonpayton deleted the fix/non-streaming-fallback-tests branch July 17, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment