ci: stabilize nextest across platforms#82
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 34 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughTwo targeted updates to test infrastructure: removed explicit successful test reporting from Nextest JUnit configuration, and enhanced e2e test stability by redacting timestamps in snapshot comparisons while correcting a mock registry request expectation count. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request updates the test infrastructure and snapshots for the shipper-cli crate. Key changes include modifying the nextest configuration to disable successful test reporting in JUnit output, enhancing the test output normalization to handle timestamps, and updating the preflight_allow_dirty_snapshot test to account for multiple registry calls. Additionally, the preflight test snapshot was updated to reflect the expected report output. I have no feedback to provide.
The preflight flow issues two registry calls per crate (version_exists + check_new_crate), but the test mock spawn_registry_not_found(1) only responded to one. The second request timed out after 30s, producing a non-deterministic reqwest error that included the mock's ephemeral port number. Bump the mock to 2 expected requests so preflight completes normally, then regenerate the snapshot against the actual preflight report output. Also: - Drop the unknown profile.ci.junit.report-successful key from .config/nextest.toml to eliminate the spurious warning in every CI run. - Add Timestamp: normalization to e2e_expanded::normalize_output to match the one already in cli_e2e, so the preflight report snapshot is stable.
b9a5897 to
be1e45f
Compare
Completes the PR-matrix / deep-lane split that the previous commit on this branch stopped short of. - `test` matrix (3 OSes, every trigger): PROPTEST_CASES=16 always. Drop the event-driven ternary; the matrix no longer carries the full-strength case count on any trigger. Also cap shrink iterations (PROPTEST_MAX_SHRINK_ITERS=1000) so failures surface fast. - New `crypto-proptests-heavy` job: Ubuntu-only, runs just `shipper-encrypt` with PROPTEST_CASES=256. Gated on schedule, push-to-main, and workflow_dispatch. Not run on pull_request, so developer feedback stays fast; opt in per-branch via dispatch if needed. Net effect: - PR CI: three OSes at reduced intensity, no Argon2 tax. - main push + nightly cron: full-strength crypto proof on Ubuntu. - workflow_dispatch: full-strength on demand against any ref. Rationale for Ubuntu-only heavy lane: crypto correctness is platform-independent at the Rust layer we exercise here, and the three-OS cost for full proptests was 30-60 min of cumulative wall time every PR without proportional signal. Note on the stale `profile.ci.junit.report-successful` nextest key: already removed on main (see #82). No change needed to nextest.toml.
#87) * ci: fast-path shipper-encrypt proptests on PR matrix The Argon2-heavy crypto proptests in `shipper-encrypt` (derive_key_*, decrypt_truncated_*, double_encrypt_roundtrip_*, each_encrypt_produces_*) were dominating wall-clock on the three-platform `test` matrix at PROPTEST_CASES=256, costing 10-20 minutes per platform on every PR. Split the case count by event: - PR / push to main: 16 cases (fast developer feedback) - schedule (nightly cron) / workflow_dispatch: 256 cases (full strength) Also pin coverage to 16 cases unconditionally, since llvm-cov instrumentation on top of Argon2 proptests is the worst-of-both and coverage does not need max-strength property exploration. No change to nextest config, no new jobs, no platform carve-out yet. * ci: split crypto proptests into Ubuntu-only heavy lane Completes the PR-matrix / deep-lane split that the previous commit on this branch stopped short of. - `test` matrix (3 OSes, every trigger): PROPTEST_CASES=16 always. Drop the event-driven ternary; the matrix no longer carries the full-strength case count on any trigger. Also cap shrink iterations (PROPTEST_MAX_SHRINK_ITERS=1000) so failures surface fast. - New `crypto-proptests-heavy` job: Ubuntu-only, runs just `shipper-encrypt` with PROPTEST_CASES=256. Gated on schedule, push-to-main, and workflow_dispatch. Not run on pull_request, so developer feedback stays fast; opt in per-branch via dispatch if needed. Net effect: - PR CI: three OSes at reduced intensity, no Argon2 tax. - main push + nightly cron: full-strength crypto proof on Ubuntu. - workflow_dispatch: full-strength on demand against any ref. Rationale for Ubuntu-only heavy lane: crypto correctness is platform-independent at the Rust layer we exercise here, and the three-OS cost for full proptests was 30-60 min of cumulative wall time every PR without proportional signal. Note on the stale `profile.ci.junit.report-successful` nextest key: already removed on main (see #82). No change needed to nextest.toml.
Summary
preflight_allow_dirty_snapshotusedspawn_registry_not_found(1), but preflight issues two registry calls per crate (version_exists+check_new_crate). The second call timed out against the exhausted mock, producing a reqwest error that embedded a non-deterministic port and "operation timed out" text — flaking on Linux/macOS/Windows.profile.ci.junit.report-successfulkey from.config/nextest.toml— it was the source of theignoring unknown configuration keywarning in every CI log.Timestamp:normalization toe2e_expanded::normalize_output(matching the helper already incli_e2e) so the preflight report snapshot stays stable across runs.Verification
cargo nextest show-config— no longer warns about the unknown key.cargo test -p shipper-cli --test e2e_expanded— 126/126 pass locally (previously 125 pass, 1 fail).Test plan
This is PR 2 in the release-hardening sprint (audit → nextest → fuzz → package-truth → rehearsal → publish).