Skip to content

Run tests/common's selftests in one binary: 21 helper selftests are compiled into and re-run by every one of ~104 includers #1957

Open

Description

[agent] Filed by the scheduled architecture audit (audit-cli-tests). Register: register comment.

Kind: refactor. Register ID: T58.

Problem

The shared test-support modules carry their own #[test] selftests, which are deliberately left ungated:

104 test files include #[path = "common/mod.rs"] mod common;, so each of those binaries compiles and runs all of them. cargo test --test cli_apply_silent reports 27 tests for 7 owned. That adds up to roughly 2,000 to 2,500 redundant test runs per cargo test. Some binaries include cache_env.rs both through common/mod.rs and directly (14 found, e.g. e2e_redirect_bun_build), so those run the cache_env set twice.

The justification comment is false. mod.rs L416-L420 says "Integration-test crates do NOT have cfg(test) set", and the same claim appears in cache_env.rs L361-L364 and bundler_e2e.rs L167-L169. cargo test --test self_update_failures_e2e -- --list lists update_fixture::fixture_selftests, which is #[cfg(test)], so cfg(test) is set (executed). Gating on cfg(test) would not help, though: every includer is a test crate.

Proposed change

  • Move the three selftest modules into one dedicated binary, e.g. tests/common_selftests.rs, which includes common/mod.rs, cache_env.rs and jvm_env.rs and owns the tests. Delete the modules from the shared files.
  • Delete the three false cfg(test) comments.
  • Drop the direct #[path = "common/cache_env.rs"] includes from binaries that already get it through common (re-export cache_env via common::cache_env).

Size

4 support files plus 1 new test file, and about 14 #[path] lines. About −40 / +15 lines, with 21 tests moved verbatim.

Acceptance criteria

  • cargo test -p socket-patch-cli --test cli_apply_silent -- --list lists only that suite's own tests.
  • cargo test -p socket-patch-cli --test common_selftests runs all 21 selftests, unchanged and green.
  • No other test changes behavior.

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Nov 1, 2026
  2. mikolalysenko commented on Nov 1, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triage: priority:p3 (test-only: where tests/common selftests are compiled). No duplicate or open PR found.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions