Run the PR gate's workspace Rust tests through cargo-nextest - #698
Draft
ciaranra wants to merge 2 commits into
Draft
Run the PR gate's workspace Rust tests through cargo-nextest#698ciaranra wants to merge 2 commits into
ciaranra wants to merge 2 commits into
Conversation
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.
Draft until the local full-lane run and the CI timing are in.
Why
After #683/#684 the Python side of the PR gate finishes in ~14 min and
pr-core-rust(~30 min) is the critical path. Inside that job the workspace test phase is 11 min, and the per-binary times from a green run add up to 656 s across 323 test binaries with the longest at 314 s:cargo testruns the binaries one after another, so the phase is serial regardless of how many cores the runner has. nextest schedules tests from all binaries at once, so on the 4-core runner the phase is bounded by the longest single test (~5 min) instead of the sum.What
scripts/ci/ensure-nextest.sh: installs the pinned cargo-nextest 0.9.143 prebuilt release for x86_64 Linux into the cargo bin directory, verified against the sha256 nextest publishes with the release, idempotent, curl retries. Any other platform fails loudly. Acargo installwould spend minutes compiling on every run andcache-bin: falsekeeps it out of the Rust cache.pecos rust test --nextest: new flag. The workspace phase runscargo nextest runfollowed bycargo test --doc, both with exactly the same package selection and features (nextest does not run doctests). The three follow-up phases (pecos-cli with runtime, zlup with cli, pecos-decoders) are unchanged. The invocation list is built by a small pure function with unit tests pinning the nextest/cargo profile mapping: nextest's own--profileselects a nextest profile, so the cargo profile goes through--release/--cargo-profile native.just rstest <mode> <runner>:runneriscargo(default, unchanged) ornextest. Local use is untouched.pr-core-gate.yml: installs nextest after Rust and runsjust rstest debug nextest.Verification
cargo clippy -p pecos-cli --all-targets -- -D warnings,cargo fmt --check, the two new unit tests, pre-commit on all changed files: clean.scripts/ci/ensure-nextest.sh: fresh install into an emptyCARGO_HOMEsucceeds and prints the version; a second run is a no-op; a copy with a wrong pinned checksum fails atsha256sum -cand installs nothing.just rstest debug nexteston a cold worktree (14 cores, RTX 4090 present): workspace phaseStarting 10352 tests across 270 binaries (60 tests skipped)->10352 passed, 60 skippedin 524 s, thencargo test --docover the same 51 doc-test binaries, 818 doctests passed; the pecos-cli, zlup and pecos-decoders phases passed. The only failure was in the GPU-onlypecos-gpu-simsphase, which this machine runs because it has a GPU and CI skips (GPU not detected):gpu_density_matrix_two_qubit_roots_preserve_rotation_channelsfails on the untoucheddevbase too (GpuDensityMatrix64 SXX: max density-matrix error 1.5e-7 exceeds 1e-12), and that phase still runs through plaincargo test. Unrelated to this change; reported separately.pr-core-ruston this PR (run 33841091323, warm Rust cache): 25 min total; compile 9m02, nextest phaseSummary [626.409s] 10352 tests run: 10352 passed, 60 skipped, doctests 7 s, follow-up phases 3 min.Result: no gain, recommend closing
The serial
cargo testphase was 656 s; nextest is 626 s. The per-test times nextest reports add up to 2486 CPU-seconds, which on a 4-core runner is 10.4 min regardless of scheduling, andneo_surface_ler_testalone takes 560 s while everything else runs alongside it (314 s when it had the machine to itself).cargo testwas already saturating the cores because tests within each binary run on all threads, so running binaries one after another cost nothing. The workspace test phase is CPU-bound, not scheduling-bound, and the ~5 min the job did lose came from the now-warm Rust cache (#683), not from nextest. Adding a pinned tool and an install step for a 30 s difference is not worth carrying; the levers that would move this job are the CPU cost of the debug-profile simulators under test (e.g.[profile.test] opt-level) or sharding the job. Left as a draft for the record.Review
Independent correctness review of the first commit: no blocker. It confirmed the workspace selection is identical between
cargo testandcargo nextest run+cargo test --doc(ignored tests stay ignored, no non-libtest harness targets in the selection, required-feature targets keep cargo's selection, the later pecos-cli phase stays authoritative for thepecosbinary), that--locked/--release/--cargo-profile nativeare valid for nextest 0.9.143 and that--cargo-profile nativelands intarget/native/like the doctest command after it, and that the defaultcargorunner is unchanged. One robustness finding, fixed in the second commit: the installer wrote to${CARGO_HOME}/binbut onlyensure-rust.shput a cargo bin directory on PATH, and that one is$HOME/.cargo/binregardless ofCARGO_HOME; the installer now appends its own directory toGITHUB_PATHon both the fresh and already-installed paths (verified with a scratchCARGO_HOMEandGITHUB_PATH).