Skip to content

Tracking: share CLI test helpers through one test-support module instead of 100+ per-file copies #824

Description

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: register comment.

Kind: tracking. Source: review Part 6.4 and Part 8.5 D; register C30.

Problem (re-counted on 045d7ec)

The CLI already has a shared tests/common/mod.rs (every test target can reach it through #[path = "../common/mod.rs"]), but most files keep their own copies:

  • fn binary() in 103 files, 7 variants (PathBuf, &'static str, one env-overridable); common::binary exists at L35-L37.
  • fn git_sha256 in 86 files, 10 variants, although common::git_sha256 exists and core exports compute_git_sha256_from_bytes.
  • 15 scrub_socket_env copies with 14 bodies, and 10 files that spawn the binary with no scrub at all. This one is a correctness problem, not just duplication: on 045d7ec an ambient SOCKET_DRY_RUN=true turns 18 of the 19 unscrubbed repair_vendor_e2e tests red while every scrubbed test in the same target is unaffected (details in Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823).
  • xorshift64* is written four times in core (test_rng.rs, oracle_support.rs, npm_crawler/oracle.rs, patch/copy_tree.rs); test_rng is the intended shared one.
  • The VEX e2e helpers are split across vex_e2e_common, vex_pdm_hatch_common, vex_pipenv_pip_common, vex_pypi_real_common and common/yarn_classic_vex.rs.

Target design

One test-support surface: tests/common for the CLI (it already reaches every target), which can later become a socket-patch-test-support dev crate if core and CLI tests need to share it. It owns the hermetic Command builder, binary(), git_sha256 (delegating to core), the envelope parsers and the seeded RNG. Each child deletes the copies it replaces, and none touches production code.

Children (in order)

  • Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823 Spawn every CLI test child through one hermetic Command builder; delete the 15 scrub_socket_env copies and cover the 10 unscrubbed files.
  • Replace the 103 private binary() with common::binary (mechanical; e2e_redirect_pnpm_build.rs keeps its env override as a named helper).
  • Replace the 86 private git_sha256 with common::git_sha256, built on compute_git_sha256_from_bytes.
  • Route core's three private xorshift copies through test_rng.
  • Merge the VEX e2e helper modules.

Only the first child is filed; the rest are mechanical and can be filed as the first lands.

Acceptance criteria

  • Each child lands as its own PR, with all tests green.
  • At the end, grep -rn "fn binary()\|fn git_sha256\|fn scrub_socket_env" crates/socket-patch-cli/tests matches only tests/common.

Dependencies

Unblocked. C31 (fewer test executables) and #793 (RunCtx, removing #[serial]) are easier after it.

Activity

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