[Mikola Lysenko (mikolalysenko)]

Problem

Fixes #1957. The 20 selftests for the shared test helpers (oracle_selftests ×8 in common/mod.rs, cache_env_selftests ×10, jvm_env_selftests ×2) were left ungated, so every integration binary that declares mod common (118 of them) compiled and re-ran all 20. The 25 binaries that also include common/cache_env.rs directly ran its 10 a second time. That adds up to about 2,600 redundant test runs per cargo test, and a broken helper was reported once per binary rather than once. (The issue says 21; one of those #[test] matches is inside a comment.)

CI context for this run: the merge queue is empty, nothing is queued or in progress, and main push and merge_group are green. Every nightly failure from the last 5 days is already fixed on main (#1994, #1914, #1891, #1905) or covered by the open #1999. No flakes or reruns turned up since 10-31. That leaves redundancy (priority 3) as the highest-value target.

Fix

• Move the three modules verbatim into a new binary, tests/common_selftests.rs, which includes common/mod.rs once. The only change inside them is use super::* → use crate::common::{,cache_env::,jvm_env::}* plus the std imports they used to inherit.
• Widen the four private cache_env items those tests touch (TOOLCHAIN_ROOTS, parent_var_names, scrub_parent_vars, is_ambient_vlt_tool_var) to pub(crate).
• common/mod.rs's pub use envelope::{…} was "used" in every binary only because of the oracle selftest. It now carries #[allow(unused_imports)], so includers that never read envelopes don't get a new warning.
• Remove the false "integration crates don't set cfg(test)" comments in mod.rs, cache_env.rs and bundler_e2e.rs. They do set it; gating still wouldn't help, because every includer is a test crate.

Not done here: dropping the 25 direct #[path = "common/cache_env.rs"] includes. They no longer run tests, so that change only saves compile time and touches 25 files. It can go in a follow-up.

Proof

• cargo test -p socket-patch-cli --test common_selftests: 20 passed (8 + 10 + 2, which matches the count of real #[test] attributes in the three modules on main).
• cargo test -p socket-patch-cli --test cli_apply_silent -- --list: 0 selftests listed (the issue measured 27 listed for 7 owned on main).
• cargo check -p socket-patch-cli --all-targets --all-features: no warnings or errors in tests/common/* or the new file across every test target.
• cargo clippy -p socket-patch-cli --test common_selftests --test cli_apply_silent --test e2e_bun_lockb --test e2e_golang_workspace_build -- -D warnings: clean except for 7 pre-existing lints in the untouched prebuilt_common/mod.rs (CI's clippy job doesn't lint test targets).
• rustfmt --check on the touched files passes.

Where the moved tests run

Same place as before: test / coverage run cargo test over every test target, and common_selftests is auto-discovered. They now run once per leg instead of about 130 times. No test was removed and no assertion changed.

---

Generated by Claude Code

---

Note

Low Risk
Test-harness refactor only; no production or CLI behavior changes, and assertions were moved verbatim.

Overview
Fixes redundant execution of shared test-helper selftests (#1957) by moving 20 checks (oracle, cache_env, jvm_env) out of tests/common/* into a dedicated tests/common_selftests.rs integration target that includes common/mod.rs once.

Integration binaries that only mod common no longer compile or run those selftests (~2,600 duplicate runs per full cargo test); failures surface once instead of per binary. Test logic is unchanged aside from import paths (crate::common::…).

Supporting tweaks: pub(crate) on four cache_env symbols the selftests need, #[allow(unused_imports)] on envelope re-exports now unused in most binaries, and trimmed misleading cfg(test) comments in bundler_e2e.rs. The bundler version_grammar selftest stays in bundler_e2e.rs.

^{Reviewed by Cursor Bugbot for commit 31b91ca. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 31b91ca. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 31b91ca3fddb32905573c61522332f641fa4fd97
• CI: 31/31 runs green (19 skipped by path filters), no failures, no main-wide failures
• Bugbot: reviewed 31b91ca, no findings
• Diff touches only test helpers under crates/socket-patch-cli/tests/common/ plus a new tests/common_selftests.rs; no CHANGELOG.md change. Reviewer focus: confirm the selftests still run exactly once (in common_selftests) and none were dropped in the move.

Slack announcement could not be sent this run (Slack send tool unavailable); a later run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Moves the 20 self-tests for the shared test helpers (oracle ×8, cache_env ×10, jvm_env ×2) out of tests/common/* into one new integration binary, tests/common_selftests.rs. Before this, every binary that declares mod common (118 of them) compiled and ran all 20, about 2,600 redundant runs per cargo test. Test-only change; no production code touched.

Risk: low. The diff only touches test files, the moved test bodies are unchanged, and the new target is auto-discovered by cargo test in CI's test and coverage legs.

Look here

• common_selftests.rs:10: includes common/mod.rs once; the three modules start at L30, L305, L586
• common/cache_env.rs:78 and L302-L352: four private items widened to pub(crate) so the moved tests can reach them. All four are still used by non-test code, so no new dead-code warnings.
• common/mod.rs:43: #[allow(unused_imports)] on the envelope re-export, which only the oracle self-test used in most binaries

Verified

• Compared the removed and added lines as sorted multisets. Apart from imports (use super::* → use crate::common::…, plus std imports), the module docs, and the dropped "integration crates don't set cfg(test)" comments, the test code is identical. No assertion changed.
• cargo test -p socket-patch-cli --test common_selftests: 20 passed. cli_apply_silent: 7 passed, and no selftests remain in it.
• e2e_bun_lockb::workspace_text_migration_heals_on_rerun fails locally with bun 1.4.4, and it fails the same way on origin/main. That's this container's environment, not this PR; it is green in CI.
• CI: ci-ok and clippy green on 31b91ca3fd. Bugbot: success, no findings. No review threads. CHANGELOG.md untouched.

Changes I made: none.

Open questions: none. The follow-up the PR mentions (dropping the 25 direct #[path = "common/cache_env.rs"] includes, a compile-time saving only) is optional.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code