Skip to content

test(network): fix response.body() navigation test on WebKit#41570

Merged
Skn0tt merged 2 commits into
microsoft:mainfrom
Skn0tt:fix-41527-followup
Jul 1, 2026
Merged

test(network): fix response.body() navigation test on WebKit#41570
Skn0tt merged 2 commits into
microsoft:mainfrom
Skn0tt:fix-41527-followup

Conversation

@Skn0tt

@Skn0tt Skn0tt commented Jul 1, 2026

Copy link
Copy Markdown
Member

Follow up to #41527.

The added test failed on WebKit, but not because WebKit keeps the body after navigation — it evicts and throws just like Chromium. The test used EMPTY_PAGE, and WebKit returns an empty buffer for a zero-length body without hitting the eviction error path, so it never threw.

Load a page with a real body instead. Now it passes on WebKit too; only Firefox stays skipped, since it genuinely keeps the body available after navigating away.

@Skn0tt Skn0tt requested a review from dgozman July 1, 2026 13:33

it('should give a readable error when response.body() races with navigation', async ({ page, server, browserName }) => {
it.skip(browserName === 'firefox', 'Firefox has a separate eviction error path');
it.skip(browserName !== 'chromium', 'Only Chromium evicts the response body on navigation and throws; Firefox and WebKit keep it available');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How come WebKit keeps it available? I don't think so!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right, it doesn't. WebKit evicts and throws just like Chromium — the test was using EMPTY_PAGE, and WebKit returns an empty buffer for a zero-length body without hitting the eviction path, so it never threw. Switched to a page with a real body and now WebKit throws too; only Firefox actually keeps the body around.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

WebKit does evict the response body on navigation and throws, just like
Chromium. The test only passed a zero-length body (EMPTY_PAGE), for which
WebKit returns an empty buffer without hitting the eviction error path.
Load a page with a real body instead, and keep skipping only Firefox,
which genuinely keeps the body available after navigating away.
@Skn0tt Skn0tt changed the title test(network): skip response.body() navigation test outside Chromium test(network): fix response.body() navigation test on WebKit Jul 1, 2026
@Skn0tt Skn0tt requested a review from dgozman July 1, 2026 14:44
@Skn0tt Skn0tt merged commit 62c2348 into microsoft:main Jul 1, 2026
43 of 46 checks passed
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [chromium] › mcp/annotate.spec.ts:446 › should switch screencast to -s session on show --annotate @mcp-windows-latest-chromium
❌ [chromium] › mcp/cli-killall.spec.ts:42 › kill-all kills filtered dashboard pid @mcp-ubuntu-latest-chromium

7459 passed, 1132 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

1 failed
❌ [chromium-page] › page/page-event-request.spec.ts:279 › resource should have type image @chromium-ubuntu-22.04-arm-node20

3 flaky ⚠️ [chromium-library] › library/video.spec.ts:337 › screencast › should work for popups `@realtime-time-library-chromium-linux`
⚠️ [firefox-library] › library/inspector/cli-codegen-3.spec.ts:224 › cli codegen › should generate frame locators (4) `@firefox-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/browsertype-connect.spec.ts:714 › run-server › should record trace with sources `@webkit-ubuntu-22.04-node20`

49460 passed, 1164 skipped


Merge workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants