Skip to content

Use tests/common's helpers in the suites that already include it: has_command ×9, sha256_hex ×16, a dead stderr_chatter filter ×5, and rollback_json's drifted envelope readers #1959

Open

Description

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

Kind: refactor. Register ID: T55 (follows T51/#1941, which covers tests/apply).

Problem

Suites that already include tests/common still keep their own copies of its helpers:

  • has_command, 9 copies beside common::has_command L57: e2e_npm.rs L42, e2e_gem.rs L47, e2e_scan.rs L66, e2e_hosted_production.rs L275, e2e_vendored_production.rs L291, e2e_redirect_rush_sim.rs L97, e2e_redirect_yarn_berry_build.rs L118, e2e_yarn4_pnpm_linker_build.rs L113, e2e_yarn4_workspaces_build.rs L113. The common one has 2 users.
  • sha256_hex, 16 copies beside common::sha256_hex L136, e.g. e2e_redirect_cargo_build.rs L135,`` mode_migration_cargo.rs L128, `e2e_vex_lockfile/{maven,nuget,poetry,uv}.rs`, `maven_build_common`, `hosted_maven_common`, `vlt_e2e_common`, `jvm_fixture_repo`. `update_fixture::sha256_file` and `update_fixture::real_binary` duplicate `sha256_hex` and `oracle::binary` too.
  • git_sha256_file and assert_run_ok, 4 copies each, and npm_run, 3 copies, in e2e_npm, e2e_gem, e2e_scan and e2e_pypi: the files common was extracted from. common::npm_run and common::env_map have 0 users.
  • stderr_chatter, 5 byte-identical copies (md5 equal): cli_apply_silent.rs L49-L59, cli_scan_silent.rs L69, apply/apply_silent_staging_errors.rs L30, rollback/cli_rollback_silent.rs L43, vendor/vendor_silent_errors.rs. The same filter is also inlined 6× in cli_remove_silent.rs (L77-L84). The filter is dead: --silent mutes the SOCKET_API_TOKEN notice in core (utils/notice.rs L51-L58). Removing the clauses leaves all tests green in cli_apply_silent (7), cli_scan_silent (6) and cli_remove_silent (10) (executed). So it should be deleted, not centralised: it can only hide a regression.
  • rollback_json re-implements envelope: events (L16-L21),``` warning_codes (L162), `events_with_mode` (= `envelope::mode_events`) and `assert_rollback_envelope` L174-L199.```` The last has drifted from `envelope::assert_envelope_invariants` L193: it requires `skipped ==` the event count where envelope allows `<=`, accepts 3 statuses instead of 10, and skips the `error`/`dryRun` checks. 20 of its 22 includers already have `common::envelope`.

Proposed change

  1. Delete the local has_command, sha256_hex, git_sha256_file, assert_run_ok and npm_run copies and call common::*. For files that don't include common/mod.rs (e.g. docker_e2e_pypi), include common/oracle.rs, or move sha256_hex there.
  2. Delete stderr_chatter and the inline filter. Assert directly on stderr.
  3. Make rollback_json re-export envelope::{events, warning_codes, mode_events}. Express assert_rollback_envelope as assert_envelope_invariants(v, "rollback") plus its rollback-only checks.
  4. Delete the dead common::env_map and update_fixture::{raw_asset, delay_asset}.

Size

About 35 test files. About −400 / +30 lines, all mechanical.

Acceptance criteria

  • grep -rn "fn has_command\|fn sha256_hex\|fn stderr_chatter" crates/socket-patch-cli/tests finds only the common definitions.
  • Every suite stays green. Any rollback test that fails under assert_envelope_invariants is a real envelope finding, to be reported, not masked.

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 helper reuse). No duplicate or open PR found. Follows #1941 (tests/apply fixtures, open PR #1944), which does not cover these suites.


    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