Repository navigation
test: link each crate's integration tests into one binary - #5689
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
moq-net, moq-relay, moq-tokio, and moq-uring linked one test binary per tests/*.rs file, 89 in all, each relinking the crate and its dependencies. Fold them into tests/integration/ modules, leaving 8 binaries: drills, allocations, loom, and wasm stay apart. The moq-cli play pass now runs only the binary's own tests, since its integration tests exercise no play code and the default pass runs them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome: the quest is complete in this PR. It folds 89 integration test binaries in moq-net, moq-relay, moq-tokio, and moq-uring into 8. It also narrows the moq-cli (Written by Claude Opus 5.5) |
# Conflicts: # quest/m0/relay-windows-stack.md
# Conflicts: # rs/moq-uring/benches/session_lite.rs
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # quest/m0/ci/README.md # quest/m0/ci/setup-cost.md
# Conflicts: # quest/m1/time/relay-seam.md
|
Iterate pass on 7d10ca8: no code changes were needed.
(Written by Claude Opus 5.5) |
Nightly already runs every mutation. The PR job stays advisory while the drills' sockets run on the host clock, so it spent runner time without gating anything. Drop it with its selector and point rs/AGENTS.md at the `tests/integration/` module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe Rust integration tests are declared in shared binaries for moq-net, moq-relay, moq-tokio, and moq-uring. Tests and benchmarks now reference shared integration-test support, and test recipes and CI filters target the integration binaries. The pull-request sensitivity-drill selector, workflow job, and related tests are removed. CI planning documents now cover a Dart FFI build item. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely identifies the primary change: combining each crate's integration tests into one binary. |
| Description check | |
The description is directly related to the changeset and explains the test-binary consolidation, CI changes, path updates, measurements, decisions, and validation. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 50 files. (41 skipped: 24 unsupported, 17 over the file limit.)
✨ Finishing Touches
-
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Clippy (1.98.1)
-
Clippy execution timed out
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.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5aa47a5c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mod finished_broadcast_mock; | ||
| mod finite_relay; | ||
| mod goaway; | ||
| mod group_charge; |
There was a problem hiding this comment.
With the default async feature on 64-bit targets, declaring group_charge here installs its #[global_allocator] for the entire integration crate, so every one of the other moq-net integration tests now performs thread-local accounting on every allocation. This defeats the file's explicit requirement that the counting allocator have a binary to itself and can materially increase the runtime of the whole suite; leave group_charge.rs as a top-level standalone test rather than importing it into this session-test binary.
AGENTS.md reference: rs/AGENTS.md:L51-L54
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. a6e1fb7 moves group_charge.rs back to a top-level test binary and notes why in integration/main.rs.
(Written by Claude Opus 5.5)
Its #[global_allocator] would otherwise count every allocation in every moq-net integration test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6e1fb72b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| //! apply to every test in the binary. | ||
|
|
||
| mod credit; | ||
| mod echo; |
There was a problem hiding this comment.
After relocating these tests, several repository references still point to paths that no longer exist: rs/moq-uring/README.md:95-102 names five old tests/*.rs paths, rs/moq-tokio/src/client.rs:1513 names tests/broadcast.rs, and quests such as quest/m1/public-model/ingest.md:46 and quest/m1/claim-answer-epochs.md:88 retain old paths. Update these references to tests/integration/... so contributors can locate the tests after this reorganization.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. 41ce0f0 updates the moq-uring README, the moq-tokio client comment, and both quests to point at tests/integration/. A wider search found no other stale paths.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Ready to merge at 41ce0f0.
(Written by Claude Opus 5.5) |
Follow #5689's layout, so the moq-net, moq-tokio and moq-uring `receive_credit` tests build as modules of `tests/integration` rather than as their own binaries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#5689 linked the integration tests into one binary, so the adapter source included by the transport module now resolves crate::quic at main.rs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lock Move the caught-up integration test into tests/integration (#5689), hold the snapshot's origin handle in a SendCell like the rest of the origin, and use @moq/clock's Milli in the Reload test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Completes
quest/m0/ci/test-cost.md.When I measured, running the Rust tests took little CPU. Compiling and linking the test binaries took most of it. Two changes cut that cost without dropping any test:
moq-net,moq-relay,moq-tokio, andmoq-uringbuilt one binary pertests/*.rsfile, 91 in total on currentmain. Each binary linked the crate and its dependencies again. The files are now modules undertests/integration/, which leaves 10 binaries. Six files stay as their own binaries:drills, so each sensitivity mutation rebuilds only the drills.allocations(moq-uring) andgroup_charge(moq-net), because each one's counting global allocator would apply to every test in the binary.loomandwasm, because they build for another cfg or target.send(from fix(net): prove native futures Send at the default recursion limit #5682), becausejust rs send-depthchecks it alone on nightly.just rs play. It runs only themoqbinary's own tests now. Before, it also re-ran moq-cli's four integration suites against a play-enabled binary. Those suites spawnexport,import,listen, andauthand never reach play code, and the default-feature pass already runs them.Other changes:
drill-prselector, and the selector's test are removed.rs/AGENTS.mdsends new session tests to the crate'stests/integration/module.max-agerecipe and the interop path trigger, the benches that include the test support, and the quest and README paths.quest/m0/ci/dart-ffi-build.md, records the Dart duplicate build described below.Measurements
Setup for each crate:
--testswithout mbx, so the compile CPU is real.playbuildThe
playpass also stops spending about 6 CPU-s and about 100 s of summed test wall time per run. Those were export linger waits. moq-cli is selected by most Rust PRs.Test counts are unchanged.
moq-netruns 2241 tests, andmoq-relay,moq-tokio, andmoq-uringtogether run 919. Both match thecache.ymlwarm run onmain.Inventory (what I measured and left alone)
main, nextest ran 7845 tests in 160 s wall, with 636 s of summed test time. Locally the wholemoq-relaysuite used 15.7 CPU-s for 192 s of summed wall time. The slow tests block on real timeouts and use little CPU, so retiring them would save runner slots but little CPU.capture-testre-runs the 466moq-videoandmoq-audiotests in 6 s.capturechanges encoder code paths, so the re-run is distinct signal. Kept.@moq/publish(25 s) and@moq/net(20 s) are mostly real-timer waits, which costs wall time rather than CPU. Kept.moq-ffitwice: once ingenerate.sh, then again indart test's build hook. In Check run 38853884676 that took about 6.7 min plus 6 min, the largest single item in the Check job. This is a duplicated build, not a redundant test, so I recorded it asquest/m0/ci/dart-ffi-build.mdinstead of fixing it here.Public API and wire impact
None.
Nextest binary IDs change: integration tests now report as
<crate>::integration <module>::<test>. Any external filter on the oldbinary_idneeds atest(/^module::/)filter instead. The in-repo filters are updated.Checks
Run locally:
just rs testformoq-net,moq-relay,moq-tokio,moq-uring, andjust rs playjust rs net-featuresandjust rs uring(includinguring-checkand qlog)moq-netandmoq-uringbenches with all featuresmax-agefilter listingcargo fmt,quest check, markdown,just gh check, justfile fmt,sh/rs/test-lints.test.pyRun locally after the decisions:
just gh checkandjust --fmt --checkontest/justfile.Not run locally:
just check. Thers/justfileand nextest config changes select the whole workspace, so CI covers it.max-ageinterop run itself.Decisions
Settled by the maintainer.
tests/*.rsfiles? A stray file still runs its tests; it only costs one more link.rs/AGENTS.mdalone.rs/AGENTS.mdat the crate'stests/integration/module. Done in this PR.autotests = falsewith explicit[[test]]entries. Rejected: a stray file would silently not run.drill-prselector, and the selector's test.just checkstill runs--apply-onlywhen Rust ortest/drill/changes.send.rs) and feat(quic): hold connection credit until the receiver releases it #5675 (credit.rs) already merged and are handled above. About 19 open PRs touch the moved files. Trial merges of feat(net)!: share exclusion scopes and report discovery refusals #5627, feat(net)!: carry group ranges on track::Subscription #5732, and moq-net: bound incomplete messages with receive credit #5735 show git follows the renames, so edits to moved files merge cleanly. New top-level files need a move:mod support;): moq-net: bound incomplete messages with receive credit #5735 and test(quic): prove receive memory with JavaScript senders and benchmarks #5752 (moq-uringreceive_credit.rs), moq-net: bound incomplete messages with receive credit #5735 (moq-netreceive_credit.rs), feat(net)!: carry group ranges on track::Subscription #5732 (subscribe_ranges.rs), fix(net): consumers see the source's real error, never Dropped #5729 (source_errors.rs), feat(net): announce consumers report when they have synced #5724 (caught_up.rs).moq-tokioreceive_credit.rs), feat(relay): widen QUIC limits for cluster peers #5674 (peer_limits.rs), feat(quic)!: shard the relay's QUIC endpoint per core by default #5688 (moq-tokioworker.rs).tests/integration/when its PR rebases. Each affected PR gets a comment once this merges.Follow-ups
moq-ffibuild. It is recorded inquest/m0/ci/dart-ffi-build.mdwith run evidence.moq-cli(4),moq-audio(3),moq-json(2), andmoq-pattern(2) still build several integration binaries each. Recommendation: skip; the savings are small.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code