Skip to content

Additional streams test consolidation gaps - #7182

Merged
jasnell merged 5 commits into
mainfrom
jasnell/streams-test-consolidation-11
Sep 9, 2026
Merged

jasnell merged 5 commits into
mainfrom
jasnell/streams-test-consolidation-11

Conversation

@jasnell

@jasnell jasnell commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Filling more gaps

@jasnell
jasnell requested review from a team as code owners August 28, 2026 23:24
Comment thread src/tests/streams/piping/error-propagation.js Outdated
Comment thread src/tests/streams/piping/error-propagation.js
Comment thread src/tests/streams/writable/abort-semantics.js Outdated
@ask-bonk

This comment was marked as outdated.

@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 1e7a415 to 69d6a88 Compare September 3, 2026 18:39
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 69d6a88 to 26396a9 Compare September 3, 2026 18:47
@jasnell
jasnell requested review from guybedford and npaun September 3, 2026 18:48
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 26396a9 to f0e8bc8 Compare September 3, 2026 22:00
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from f0e8bc8 to 6d34416 Compare September 3, 2026 22:40
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 6d34416 to 46a6db6 Compare September 4, 2026 00:05
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 46a6db6 to 85dea2e Compare September 4, 2026 01:32
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 85dea2e to 729db00 Compare September 4, 2026 01:51
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 729db00 to aa05643 Compare September 4, 2026 03:03
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from aa05643 to 183f521 Compare September 4, 2026 14:00
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 183f521 to a267e48 Compare September 4, 2026 18:13
@codecov-commenter

codecov-commenter commented Sep 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.31%. Comparing base (8bfef8f) to head (02552e4).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7182   +/-   ##
=======================================
  Coverage   37.31%   37.31%           
=======================================
  Files         800      800           
  Lines      251740   251740           
  Branches    20021    20021           
=======================================
  Hits        93943    93943           
  Misses     146447   146447           
  Partials    11350    11350           

☔ 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.

@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from a267e48 to 925088e Compare September 4, 2026 19:28
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 925088e to 19350d6 Compare September 4, 2026 19:29
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 19350d6 to bff3250 Compare September 8, 2026 18:24

@guybedford guybedford 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.

Ran all readable-byte / writable / piping targets with --nocache_test_results: 55/55 pass. The piping additions and the WPT map look good (the 34 count is right). Approving, with a handful of comments where I probed both implementations and the comments/attributions don't match what the code actually does. None block, but the mislabelled spec claims are worth fixing since these files are the normative artifact for the suites.

Divergences I found while probing that are not yet pinned anywhere:

  • Remainder after a partial BYOB read then default read: C++ copies into a fresh 4096-byte buffer (byteOffset 0), TypeScript returns a view into the original enqueued buffer (buffer.byteLength 16, byteOffset 8), which is exactly what the WPT asserts.
  • byobRequest after a partial enqueue() into a pending pull-into: C++ invalidates it (null), TypeScript shrinks the view (byteLength 2 -> 1). enqueueDiscardsByobRequest only covers the full-fill case.

Comment thread src/tests/streams/readable-byte/byob-reader.js Outdated
Comment thread src/tests/streams/readable-byte/AGENTS.md Outdated
Comment thread src/tests/streams/readable-byte/controller.js Outdated
Comment thread src/tests/streams/readable-byte/AGENTS.md Outdated
Comment thread src/tests/streams/writable/abort-semantics.js Outdated
Comment thread src/tests/streams/writable/abort-semantics.js Outdated
Comment thread src/tests/streams/writable/abort-semantics.js Outdated
Comment thread src/tests/streams/readable/AGENTS.md Outdated
Base automatically changed from jasnell/streams-test-consolidation-10 to main September 8, 2026 19:55
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 36a8557 to 9bdf9bd Compare September 8, 2026 19:55
@jasnell
jasnell force-pushed the jasnell/streams-test-consolidation-11 branch from 9bdf9bd to 02552e4 Compare September 8, 2026 19:56
@jasnell
jasnell merged commit 21ee4d0 into main Sep 9, 2026
22 checks passed
@jasnell
jasnell deleted the jasnell/streams-test-consolidation-11 branch September 9, 2026 13:35
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.

3 participants