Skip to content

js/net's lite-07 datagram queue test holds up under load - #8326

Merged
kixelated merged 4 commits into
mainfrom
quest/m0/test-flakes-2/datagram-queues
Nov 9, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m0/test-flakes-2/datagram-queues

Conversation

@kixelated

@kixelated kixelated commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes and deletes quest/m0/test-flakes-2/datagram-queues.md.

Summary

  • Removes the test.skip that #8287 added to js/net/src/lite/datagram-tracks.test.ts's "lite-07 bounds and prioritizes independent createWritable datagram queues".
  • Deletes the quest and its link in the test-flakes-2 README. No code fix: I could not reproduce the failure, so I did not change the test or the publisher.

Reproduction (the quest's first step)

On the un-skipped test, every run passed:

  • bun test --rerun-each 200 with the test filtered, idle: 200/200.
  • The same under about 40 busy-loop burners on 24 cores: 200/200.
  • The whole file, --rerun-each 100, under load near 190: 3900/3900.
  • Every Budget.poll() forced to yield (js/net/src/util/yield.ts, reverted before commit), file --rerun-each 50: 1950/1950.
  • The whole js/net package, five runs under 48 burners: this test passed in all five. Unrelated tests failed in two runs (a lite draft-07 re-offer test in publisher.test.ts, and a compressed-frame test); they are not this quest's.

The write-budget yield only sits in the subscription's update loop. The test's later assertions follow from ordering rather than timing. The one implicit step is that the audio send queue opens after its DatagramsInfo write, and no event exposes that open. That is a candidate cause, not a confirmed one.

Verification

  • quest check: 605 documents ok.
  • just js check: passes.
  • bunx biome check on the test file: passes.
  • bun test on js/net: all tests pass except the unrelated failures above.

Decisions

Impact

Test-only. No public API, no wire change, no MIGRATE.md note.

Choices made without asking

  • Did not change the test or the publisher, since the failure did not reproduce and no cause was confirmed.

Follow-ups

  • The unrelated failures seen under load (the lite draft-07 re-offer test in publisher.test.ts, and the compressed-frame test) belong to other quests or need a new one. I did not check whether a quest already covers them.

🤖 Generated with Claude Code

(Written by Haiku 5.5; updated by Opus 5.5 with low)

Models

Implementer: Claude Code, claude-haiku-5-5 (start, S)
Implementer: Claude Code, claude-opus-5-5, low effort (iterate, S)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, low effort
  • Session: 01a1bdf4-4a58-7ff1-a986-ad0f6d226847, round 1
  • Head: 02aa1d2
  • Base: origin/main at 9a07302
  • Score: 9/10, The change restores existing coverage without changing production behavior; no actionable defects were established, and builds and tests were not run as instructed.
  • Findings: none

kixelated and others added 2 commits November 8, 2026 13:05
The test was quarantined in #8287 as flaky under load. Its suspected cause is
an ordering assumption, not a timeout, and I could not reproduce a failure:
it passed every run on the un-skipped test (about 6,000, most under heavy CPU
load, including forced write-budget yields on every poll and five runs of the
whole js/net suite).

The quest stays open with the un-reproduced cause written down, and a Waiting
item asks the maintainer whether to keep the test un-skipped.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Maintainer decision (via /quest-complete):

  • ✅ Keep the lite-07 datagram queue test un-skipped without a reproduced cause and finish the quest; if it fails again, re-quarantine it and reopen with that run's log (rejected: keep it quarantined and investigate further).

(Written by Claude Opus 5.5 with high effort)

@kixelated
kixelated marked this pull request as ready for review November 9, 2026 01:38
@coderabbitai

coderabbitai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 619495ba-c5b8-4a65-837c-8dcd42e44fb2
Commits

Reviewing files that changed from the base of the PR and between 9a07302 and 02aa1d2.

Files selected for processing: 3

  • js/net/src/lite/datagram-tracks.test.ts
  • quest/m0/test-flakes-2/README.md
  • quest/m0/test-flakes-2/datagram-queues.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

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

ℹ️ One stale record in the epic README; the test and quest changes look right.

Reviewed changes

  • test.skip removed: js/net/src/lite/datagram-tracks.test.ts's "lite-07 bounds and prioritizes independent createWritable datagram queues" runs again, with its quarantine comment deleted.
  • Quest deleted: quest/m0/test-flakes-2/datagram-queues.md and its ## Required line in the epic README are gone.

I checked the leftovers: no other reference to datagram-queues.md (or its path) survives in the tree, and there is no sh/gh/test-inventory.allow entry naming it. The reproduction evidence in the body is consistent with the recorded maintainer decision, so I am not disputing the un-skip.

ℹ️ The epic README still calls the datagram queue test quarantined

quest/m0/test-flakes-2/README.md:202-206 still reads "All are quarantined with test.skip until their children fix them" for the bandwidth sampling, datagram queue, and lite-06 session-close cases. This PR removes the datagram queue skip and deletes its child, so that sentence now records a quarantine the PR has ended. The neighbouring resolved entries (the datagram-end case at lines 134-146, the relative-start case at lines 174-179) were rewritten in place to record how the quarantine ended, so the same treatment is expected here.

Technical details
# Epic README still records a removed quarantine

## Affected sites

- `quest/m0/test-flakes-2/README.md:202-206` — the "Added 2026-11-08" paragraph
  lumps three tests together and ends "All are quarantined with `test.skip`
  until their children fix them." After this PR the datagram queue test is
  un-skipped and `datagram-queues.md` is deleted.

## Required outcome

- The paragraph no longer claims the datagram queue test is quarantined.
- The bandwidth sampling and lite-06 session-close cases stay described as
  quarantined, since `bandwidth-sampling.md` and `session-close-end.md` are
  still open children with their skips in place.

## Suggested approach

Split the datagram queue case into its own record (or a trailing sentence in
the same paragraph) noting it was re-enabled in this PR after reruns under load
found no reproduction — the body's 200/200 idle, 3900/3900 loaded, and
1950/1950 forced-yield results — rather than claiming a cause was fixed.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed head 02aa1d2. No new actionable findings established.

This restores the existing datagram-queue test without changing its assertions or production behavior, consistent with the recorded maintainer decision to re-enable it despite the unreproduced failure. The quest deletion and index update agree. This is restoration of coverage, not a demonstrated fix for the original flake; its cause remains unconfirmed.

Verification limits: GitHub-only static review of the complete diff, test sequence and recorded decision. Reported stress runs were not independently repeated, and no builds or tests were run.

@kixelated
kixelated added this pull request to the merge queue Nov 9, 2026
Merged via the queue into main with commit b4606ef Nov 9, 2026
11 checks passed
@kixelated
kixelated deleted the quest/m0/test-flakes-2/datagram-queues branch November 9, 2026 02:50
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.

1 participant