Skip to content

test(streams): vendor full WPT transform-streams suite into CI - #32601

Closed
alii wants to merge 3 commits into
ali/transformstream-transformer-cancelfrom
ali/wpt-transform-streams
Closed

test(streams): vendor full WPT transform-streams suite into CI#32601
alii wants to merge 3 commits into
ali/transformstream-transformer-cancelfrom
ali/wpt-transform-streams

Conversation

@alii

@alii alii commented Jun 22, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Stacked on #32595. Vendors the complete streams/transform-streams/*.any.js WPT suite (11 files, 133 tests) plus its streams/resources/ helpers, byte-identical from web-platform-tests/wpt@e4a4672e9e, with a self-contained testharness shim mapped onto bun:test.

132/133 pass. The 1 remaining known failure run as test.todo with a one-line root cause each in known-failures.ts:

test root cause
errors.any.js › TransformStream transformer.start() rejected promise should error the stream pre-existing: writer.write()/reader.read() never settle when start() returns a promise that rejects asynchronously — reproduces on releases without the transformer.cancel changes

The runner replaces #32595's cancel-only driver with a generic loader so adding readable-streams/ / writable-streams/ / piping/ later is just appending file names.

Why

Directly addresses the review concern on #31728 ("hesitant to make a large change to TransformStream internals without running web platform tests in CI") with the actual upstream tests, not hand-mirrored ones — and surfaced two real spec gaps (the WritableStream Promise.$resolve short-circuit, now fixed in #32620; the start-reject hang) that the hand-mirrored tests missed.

How did you verify your code works?

bun bd test test/js/third_party/wpt-streams/run.test.ts

132 pass / 1 todo / 0 fail. streams.test.js, compression.test.ts, wpt-h2/run.test.ts unaffected.

@robobun

robobun commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator
Updated 11:06 AM PT - Jun 24th, 2026

@alii, your commit 9789a6cf8252363577e828332d7fd1c7323b21de passed in Build #64530! 🎉


🧪   To try this PR locally:

bunx bun-pr 32601

That installs a local version of the PR into your bun-32601 executable, so you can run:

bun-32601 --bun

@claude claude Bot 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.

No issues found — the vendored WPT files and shim look correct — but since this is stacked on #32595 and introduces a new testharness-shim pattern that future WPT suites will build on, it's worth a human sanity-check on the shim/loader design.

Extended reasoning...

Overview

This PR vendors the full WPT streams/transform-streams/*.any.js suite (11 test files + 3 resource helpers, stated byte-identical to upstream wpt@e4a4672e9e) into test/js/third_party/wpt-streams/. The non-vendored pieces are a new ~150-line testharness-shim.ts that maps WPT's promise_test/test/assert_* surface onto bun:test, a known-failures.ts registering 3 tests as test.todo with documented root causes, and a generalized run.test.ts loader that concatenates the resource scripts and evaluates each .any.js file inside new Function("test", ...) to avoid bun:test's per-module test binding shadowing the WPT global.

All changes are confined to test/js/third_party/wpt-streams/ — no production/runtime code is touched.

Security risks

None. This is test-only code executed in CI. The new Function(...) evaluation runs over locally-vendored, version-pinned files (not network-fetched), which is the same pattern the previous run.test.ts already used. patched-global.any.js temporarily monkey-patches Object.prototype and the global ReadableStream/WritableStream constructors, but the shim runs add_cleanup callbacks in a finally block so globals are restored even on assertion failure, and the effect is scoped to this test file's process.

Level of scrutiny

Low-to-moderate. The bulk of the diff is mechanically vendored upstream test fixtures with no behavioral consequence beyond pass/fail signal. The part that merits human eyes is testharness-shim.ts: it's small and reads correctly (cleanups reversed in finally, Object.is for equality, proper rejection assertions), but it establishes the pattern for vendoring further WPT suites (readable-streams/, writable-streams/, piping/) per the PR description, so a maintainer should confirm they're happy with the approach (e.g., self-contained shim here vs. reusing/extending wpt-h2/testharness-shim, which the previous revision imported from).

Other factors

  • The PR is stacked on #32595 and is part of a chain addressing an explicit reviewer concern on #31728 about TransformStream WPT coverage — a human is already engaged with this series and should see this increment.
  • The CI failures reported by robobun are build-rust infrastructure failures on unrelated targets, not test failures from this change.
  • The 3 known-failures are well-documented with spec-level root causes and registered as test.todo, so the suite is green by construction.

@claude claude Bot 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.

LGTM — test-only addition vendoring upstream WPT transform-streams suite with a small testharness shim.

Extended reasoning...

Overview

This PR vendors the WPT streams/transform-streams/*.any.js suite (11 test files plus 3 resource helpers, byte-identical to upstream wpt@e4a4672e9e) into test/js/third_party/wpt-streams/, replaces the previous single-file run.test.ts driver with a generic loader, and adds a self-contained testharness-shim.ts that maps the WPT test/promise_test/assert_* surface onto bun:test. A known-failures.ts registry routes one not-yet-passing test through test.todo. All 16 changed files live under test/; no runtime/source code is touched.

Security risks

None. This is purely additive test infrastructure executed only during bun test. The vendored .any.js files are unmodified upstream WPT content and exercise only in-process TransformStream/ReadableStream/WritableStream APIs — no network, filesystem, child processes, or credentials. The shim and runner do nothing beyond readFileSync of co-located test fixtures and new Function evaluation of those fixtures (the same pattern the prior run.test.ts already used for cancel.any.js).

Level of scrutiny

Low. The bulk of the diff is verbatim third-party test content; the only authored logic is ~150 lines of shim plus ~50 lines of runner, both straightforward and confined to the test tree. patched-global.any.js does mutate Object.prototype and the global ReadableStream/WritableStream constructors, but each mutation is paired with t.add_cleanup and the shim runs cleanups in a finally block, so global state is restored even on failure. The worst realistic failure mode is a flaky or red CI test, which is self-correcting and carries no production risk.

Other factors

No CODEOWNERS cover this path. The bug-hunting pass found nothing. The PR is stacked on #32595 but the diff here is independent (it generalizes that PR's runner). The known-failures mechanism uses test.todo, so documented gaps stay visible without breaking CI. Given it's test-only, mechanical, and additive, this is safe to approve.

@alii
alii force-pushed the ali/transformstream-transformer-cancel branch from 800f5f5 to aa1f955 Compare June 23, 2026 10:31
@alii
alii force-pushed the ali/wpt-transform-streams branch from 0e42d7f to 47ddbca Compare June 23, 2026 10:31

@claude claude Bot 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.

LGTM — test-only addition vendoring upstream WPT files plus a small self-contained testharness shim.

Extended reasoning...

Overview

This PR vendors the complete WPT streams/transform-streams/*.any.js suite (11 test files + 3 resource helpers, byte-identical from web-platform-tests/wpt@e4a4672e9e) into test/js/third_party/wpt-streams/. The only original code is a ~150-line testharness-shim.ts mapping WPT's promise_test/test/assert_* API onto bun:test, a tiny known-failures.ts registry (1 entry, run as test.todo), and a generalized run.test.ts driver that loads resource scripts + each .any.js file via new Function. No production/runtime code is touched.

Security risks

None. All changes are confined to test/. The vendored files are upstream WPT verbatim. The driver reads local fixture files and evaluates them in-process during bun test — standard for a WPT harness and no different in trust model from any other test file in the repo. patched-global.any.js mutates Object.prototype and the global ReadableStream/WritableStream, but registers add_cleanup callbacks which the shim runs in a finally block, so globals are restored even on failure.

Level of scrutiny

Low. ~95% of the diff is vendored third-party test fixtures that need no line-by-line review. The shim is small, straightforward, and only needs to cover the API surface the vendored files actually use; I read through it and the assertion/cleanup semantics look correct (Object.is for equality, instanceof for assert_throws_js, cleanups reversed and run in finally). Worst-case failure mode is a flaky test, which the known-failures mechanism already handles.

Other factors

No CODEOWNERS rule covers this path. The bug-hunting system found nothing. The PR is stacked on #32595 and replaces its cancel-only driver with a generic one, so the run.test.ts delta is mostly mechanical. CI build is in progress per robobun; if it goes red the issue will be obvious and contained to this test file.

@claude claude Bot 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.

LGTM — test-only addition vendoring upstream WPT transform-streams tests with a thin bun:test shim.

Extended reasoning...

Overview

This PR vendors the complete WPT streams/transform-streams/*.any.js suite (11 test files + 3 resource helpers, byte-identical from upstream) into test/js/third_party/wpt-streams/, replaces the previous single-file driver in run.test.ts with a generic loader, and adds a self-contained testharness-shim.ts (~150 lines) that maps WPT globals (promise_test, assert_*, promise_rejects_*, cleanup, etc.) onto bun:test. One known failure is registered as test.todo via known-failures.ts. No production/runtime code is touched.

Security risks

None. All changes live under test/. The new Function(...) evaluation in run.test.ts only loads vendored files from disk, not external input. patched-global.any.js mutates Object.prototype and the global ReadableStream/WritableStream, but the shim runs registered cleanups in a finally block, so globals are restored even on failure — and this is scoped to the test process.

Level of scrutiny

Low. ~90% of the diff is verbatim upstream WPT content that needs no line-by-line review. The remaining ~200 lines (shim, runner, known-failures) are straightforward test infrastructure following the existing wpt-h2 precedent. CI exercises the whole thing (132 pass / 1 todo per the description), so any shim defect would surface immediately as a test failure rather than silently breaking anything.

Other factors

No CODEOWNERS entries cover these paths, no outstanding reviewer comments, and the bug-hunting system found nothing. The PR is large by line count but mechanically simple — adding more conformance tests is low-risk and high-value, and the shim is easy to audit.

@alii

alii commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

@robobun adopt

@robobun

robobun commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Closed as superseded by #33193, which vendors the complete WPT streams suite (68 files, 1174 subtests) to the same path. Final state here for reference: rebased, mergeable, bun bd test test/js/third_party/wpt-streams/run.test.ts → 132 pass / 1 todo / 0 fail.

@alii
alii force-pushed the ali/transformstream-transformer-cancel branch from aa1f955 to 2caa760 Compare June 23, 2026 16:25
@alii
alii force-pushed the ali/wpt-transform-streams branch from 47ddbca to 04eda32 Compare June 23, 2026 16:25

@claude claude Bot 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.

LGTM — test-only change vendoring upstream WPT files plus a small testharness shim; no runtime code touched.

Extended reasoning...

Overview

This PR vendors the complete WPT streams/transform-streams/*.any.js suite (11 test files + 3 resource helpers, byte-identical from upstream web-platform-tests) into test/js/third_party/wpt-streams/, and adds a ~150-line testharness-shim.ts that maps the WPT testharness API onto bun:test. The existing run.test.ts driver is generalized from a single-file loader to iterate over the full file list, and a known-failures.ts file marks one pre-existing spec gap as test.todo. No production/runtime code is modified.

Security risks

None. All changes are confined to the test tree. The vendored .any.js files are evaluated via new Function(...) inside the test runner, but this is test-only code executing trusted vendored content under bun test — not user input, not shipped in the binary, and not reachable at runtime.

Level of scrutiny

Low. The vast majority of the diff (~2,300 lines) is verbatim upstream WPT content that does not need line-by-line review. The actually-authored code is the shim (~150 lines of straightforward assert/test wrappers), the runner refactor (~30 net lines), and the known-failures registry (~13 lines). The shim correctly handles add_cleanup via try/finally so patched-global.any.js restores Object.prototype / global stream constructors after mutating them.

Other factors

  • robobun confirmed 132 pass / 1 todo / 0 fail after maintainer adoption.
  • No CODEOWNERS coverage for this path.
  • No bugs surfaced by the bug-hunting system.
  • Worst-case failure mode is a red CI test, which is immediately visible and easily reverted.

@claude claude Bot 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.

LGTM — test-only PR vendoring upstream WPT transform-streams suite plus a small testharness shim; no runtime code touched.

Extended reasoning...

Overview

This PR vendors the complete WPT streams/transform-streams/*.any.js suite (11 test files) and its streams/resources/ helpers into test/js/third_party/wpt-streams/, byte-identical from upstream web-platform-tests. The authored portions are: a ~150-line testharness-shim.ts mapping WPT's promise_test/assert_* surface onto bun:test, a generic run.test.ts driver that loads each .any.js file via new Function (so bun:test's per-module test binding doesn't shadow the WPT test(fn, name) global), and a known-failures.ts registering the one not-yet-passing test as test.todo. All 16 files live under test/; zero runtime/src code is touched.

Security risks

None. This is test-only infrastructure. The new Function(...) evaluation is over static, repo-vendored test sources — not dynamic or user-controlled input. patched-global.any.js mutates Object.prototype and the global ReadableStream/WritableStream, but cleanups are registered via t.add_cleanup and the shim runs them in a finally block, matching WPT's own contract.

Level of scrutiny

Low. The bulk of the diff is verbatim upstream WPT content (a widely-reviewed reference suite). The authored shim is mechanical assertion/registration glue with no clever logic. The driver follows the same pattern as the existing wpt-h2/ sibling. The change strictly adds coverage — it cannot regress runtime behavior, and a flaky test would only affect CI signal.

Other factors

robobun confirmed 132 pass / 1 todo / 0 fail locally and CI is green so far. No CODEOWNERS rule covers this path. No outstanding review threads. The bug-hunting system found nothing. Stacked on #32595, but the incremental diff here (replacing the cancel-only driver with the full suite) is self-contained and clear.

@robobun
robobun force-pushed the ali/transformstream-transformer-cancel branch from 6aeb0d8 to 40f5bac Compare June 24, 2026 15:12
alii and others added 3 commits June 24, 2026 17:06
Vendors all 11 streams/transform-streams/*.any.js plus their
streams/resources/ helpers byte-identical from
web-platform-tests/wpt@e4a4672e9e, with a self-contained testharness shim
mapped onto bun:test (assert_*, promise_rejects_*, t.step/unreached_func/
add_cleanup, step_timeout). 130/133 pass; the 3 known failures are recorded
in known-failures.ts and run as test.todo with a one-line root cause each:

- 1 is the WritableStream Promise.$resolve vs Web IDL "a promise resolved
  with x" (= new Promise(r => r(x))) hop-count divergence already documented
  in the cancel.any.js runner.
- 1 is the same hop-count divergence observed via the abort-before-started
  race.
- 1 is a pre-existing hang: writer.write()/reader.read() never settle when
  start() returns a promise that rejects asynchronously (reproduces on
  releases without the transformer.cancel changes).

The runner replaces the cancel.any.js-only driver with a generic loader so
adding more streams/ WPT directories is just appending files.

Claude-Session: https://claude.ai/code/session_01Qn8dDrArUZxc4towq11qzu
With the Web IDL promise-resolved-with fix underneath, the
controller.error()-inside-cancel and abort-before-started WPT cases pass.
Only the pre-existing start()-rejects-asynchronously hang remains as a
known failure. 132/133.

Claude-Session: https://claude.ai/code/session_01Qn8dDrArUZxc4towq11qzu
@robobun
robobun force-pushed the ali/wpt-transform-streams branch from 04eda32 to 9789a6c Compare June 24, 2026 17:10

@claude claude Bot 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.

LGTM — test-only vendoring of upstream WPT files plus a small, self-contained testharness shim.

Extended reasoning...

Overview

This PR vendors the full WPT streams/transform-streams/*.any.js suite (11 files) and its streams/resources/ helpers into test/js/third_party/wpt-streams/, all stated to be byte-identical to upstream wpt@e4a4672e9e. The non-vendored portion is small: a ~150-line testharness-shim.ts that maps the WPT testharness surface onto bun:test, a generalized run.test.ts driver (replacing the prior cancel-only loader with a file list and shared resource preamble), and a known-failures.ts that registers the one not-yet-passing test as test.todo. No production/runtime code is touched.

Security risks

None. Everything lives under test/ and only runs in CI. The new Function(...) evaluation pattern was already in place in the previous run.test.ts and only consumes static, vendored, in-repo files — no network input, no privilege boundaries crossed.

Level of scrutiny

Low. The vast majority of the diff is upstream WPT test data copied verbatim, which doesn't need line-by-line review. The shim and runner are mechanical: assertion helpers (assert_equals, assert_throws_js, etc.) that throw on mismatch, a WPTTest stub with add_cleanup/unreached_func/step, and a loop that reads each .any.js file and wraps it in a describe. The patched-global.any.js cleanup path is correctly handled via the shim's try/finally around fn(t).

Other factors

No CODEOWNERS cover this path, no outstanding reviewer comments, and the bug-hunting system found nothing. The PR description shows local verification (132 pass / 1 todo / 0 fail) and confirms adjacent suites are unaffected. This is additive test coverage that strengthens CI for the TransformStream work it's stacked under.

@alii

alii commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

Superseded by #33193, which vendors the complete WPT streams suite (68 .any.js files, 1174 subtests) to the same test/js/third_party/wpt-streams/ path with an expectations-driven harness.

@alii alii closed this Jul 1, 2026
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