Skip to content

test: restore /xhr to the WPT filter - #5723

Open
RaphaelFakhri wants to merge 1 commit into
nodejs:mainfrom
RaphaelFakhri:test/5710-restore-xhr-wpt
Open

test: restore /xhr to the WPT filter#5723
RaphaelFakhri wants to merge 1 commit into
nodejs:mainfrom
RaphaelFakhri:test/5710-restore-xhr-wpt

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

Fixes #5710.

/xhr was dropped from the test:wpt filter in #4786, which is not mentioned in that PR's description and looks incidental. The practical effect is that undici lost WPT FormData coverage, because that suite lives under /xhr/formdata rather than under /fetch. The xhr subtree of expectation.json has been frozen since, while fetch, websockets, eventsource and mimesniff all moved on.

I ran the suite rather than assuming the stored data still held:

  • wpt-runner.mjs run /xhr against the untouched committed expectations exits 0. 161 test files, 82 passing and 495 expected-fail subtests, matching the 583 stored xhr expectations. The runner exits non-zero only when a success value drifts, and none did, so the frozen data is still accurate.
  • Full npm run test:wpt with the amended filter exits 0: 1201 test files, 4869 passing and 4537 expected-fail subtests. Nothing outside xhr moved.
  • Re-running /xhr against the finished branch leaves the working tree byte identical, so the refresh is stable rather than churning on every run.

The expectations refresh is 83 lines, all inside the xhr subtree: four WPT test files added since the removal, recorded with their observed expected-fail cases, plus two deterministic case-order moves. I diffed the parsed JSON to confirm that xhr is the only top-level subtree that changed.

This restores 82 passing subtests, roughly 60 of them FormData conformance: the formdata/ suites plus the FormData block in xhr/idlharness.

/serviceWorkers is dropped from the same list. The WPT directory is service-workers and the filter is a case-sensitive startsWith on the pathname, so the argument has never matched anything. I confirmed it empirically: wpt-runner.mjs run /serviceWorkers reports 0 test files, and expectation.json has never had a key for it. Correcting the spelling instead would be the wrong move, since those tests are browser scoped and the runner's skip guard matches serviceworker without the hyphen, so removing the dead argument is a no-op.

One tradeoff to be aware of: the xhr suite adds roughly two minutes to a test:wpt invocation. Until #5588 lands, that cost falls on every local npm test, which is presumably why the removal went unnoticed. If you would rather have the coverage without the whole suite, the runner supports the narrower /xhr/formdata; I restored the full /xhr since it is the exact revert of what was dropped and its expectations verifiably still hold.

/xhr was dropped from the test:wpt filter in nodejs#4786 without mention in
the PR, which removed WPT FormData coverage: that suite lives under
/xhr/formdata rather than /fetch. Restore the filter and record the
xhr expectation entries for test files added to WPT since the removal.

Also drop /serviceWorkers from the filter: the WPT directory is
service-workers, so the argument has never matched any test.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@codecov-commenter

codecov-commenter commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (fc3450d) to head (fb5eb27).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5723   +/-   ##
=======================================
  Coverage   93.47%   93.47%           
=======================================
  Files         110      110           
  Lines       38908    38908           
=======================================
  Hits        36368    36368           
  Misses       2540     2540           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

test: /xhr dropped from the WPT filter in #4786, losing FormData coverage

3 participants