Repository navigation
perf(test): build zone_server's integration tests as one binary - #117
Merged
Merged
Conversation
large_enum_variant fails Rust Lint on main: Routing::Chat carries an 856-byte ChatRow plus Route. Matches the identical change in the open migrate/* PRs so whichever lands first merges cleanly with the rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cargo built each of the 100 files in zone_server/tests as its own executable, and each statically linked all of zone_server plus ONNX Runtime. On a clean build that was 14.8 GiB of 19 GiB in debug/deps, and every edit to zone_server relinked all of them. The files now live under tests/integration/ as modules of a single `integration` target (autotests = false), with the shared helpers at crate::common. Module names drop the redundant _tests suffix, so a test is addressed as `integration api::name` instead of `api_tests name`. With autotests off, a file dropped in tests/ or left out of main.rs would silently never compile; layout.rs fails in either case, which also catches branches that add test files at the old paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IMAGE_GENERATIONS admits one media generation per process. chat_lifecycle and chat_ws_media_contract each serialized on their own mutex, which held while every file was its own binary. In the merged binary under plain cargo test or cargo llvm-cov, chat_ws's media tests hold the lane while chat_ws_contract::queued_media_stops_when_its_chat_lease_is_fenced expects it free, and the test failed 4 of 4 single-process runs. One common::MEDIA lock taken by every test that drives a generation makes it pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 113 files, which is 13 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
📒 Files selected for processing (113)
You can disable this status message by setting the
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Cargo built each of the 100 files in
runner/zone_server/tests/as its own test executable, and every one statically linked all ofzone_serverplus ONNX Runtime. That was most of the build dir, and every edit tozone_serverrelinked all of them — multiplied across the ~15 agent worktrees sharing one build dir.What
autotests = falseplus one[[test]] name = "integration"attests/integration/main.rs, declaring each former file as a module. Shared helpers moved totests/integration/common/, reached ascrate::common._testssuffix:api_tests.rs→integration/api.rs. Test IDs becomeintegration api::<name>in nextest; filters like-E 'test(/^api::/)'select one former file.tests/integration/layout.rsfails if a.rsfile lands intests/(it would never compile with autotests off) or a file intests/integration/is not declared inmain.rs. Branches still adding test files at the old paths fail loudly instead of silently not running.tests/CONTEXT.md, the "Run with" headers,srcand installer comments, the coverage workflow comment,docs/SYSTEM-PROMPT-RESEARCH.md. CI commands (--workspace --all-targets) need no change.fix(chat): boxesRouting::Chat'sRoute. Rust Lint is red on main withlarge_enum_variant; this is byte-identical to the fix in fix(tools): sanitize tool output with abnegate-secret #110–fix(mail): deliver account mail over STARTTLS through abnegate-notify #114, so it merges cleanly whichever lands first.Shared process state
nextest still runs each test in its own process, so CI's test job is unaffected. Plain
cargo testandcargo llvm-cov(Rust Coverage) now run all modules in one process instead of one per file. The only test-side process mutations arechat_agent(ZONE_CHAT_AGENT_CWD, set once through aLazyLock) andchat_handover(prepends stand-inclaude/codextoPATH, also once). Both are safe to share.One real cross-module dependency surfaced:
IMAGE_GENERATIONSinws/chat.rsadmits one media generation per process.chat_lifecycleandchat_ws_media_contracteach serialized on a private mutex, so in one processchat_ws's media tests held the lane whilechat_ws_contract::queued_media_stops_when_its_chat_lease_is_fencedexpected it free — it failed 4/4 single-process runs. Those tests now sharecommon::MEDIA; 3/3 single-process runs pass.Measurements
Isolated build dir (
CARGO_BUILD_BUILD_DIR/CARGO_TARGET_DIRin scratch),cargo build --workspace --exclude zone_desktop --all-targets,debug = "line-tables-only", dependency debuginfo off, Homebrew rustc 1.98.1, M-series Mac. Edit-rebuild appends a line tozone_server/src/lib.rs, three runs.debug/depsdepsTests
Local, against disposable pgvector Postgres 17 + Valkey 8.1 with migrations applied:
cargo nextest run --workspace --exclude zone_desktop --all-targets --no-default-features --features zone_context/test-utils: 6389 passed, 61 skipped (1262 inzone_server::integration).cargo nextest run -p zone_server --test integration: 1261 passed, 6 skipped.cargo test -p zone_server --test integration(one process, as llvm-cov runs it): 1261 passed, 6 ignored, three runs.cargo clippy --workspace --exclude zone_desktop --all-targets --all-features -- -D warningsandcargo fmt --all --checkclean.🤖 Generated with Claude Code