GeezerVison is showing github.com — Use tests/common's binary() and git_sha256 in CLI tests (#824) by mikolalysenko · Pull Request #1124 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Refs #824 (children 2 and 3, slice: the test files no open PR changes).
Summary
72 CLI test files kept a private fn binary() and/or fn git_sha256 that tests/common/mod.rs already provides. They now import common::binary / common::git_sha256. A one-sided ratchet in the cli test binary stops new copies.
Why
What changed
- 72 test files: deleted the private helpers and imported common's. Top-level files declare
#[path = "common/mod.rs"] mod common;. Modules of directory binaries use crate::common::…. rollback/main.rs and e2e_vex_lockfile/main.rs now declare common.
sha2::{Digest, Sha256} and PathBuf imports that only the deleted helpers used are gone.
- New
tests/cli/shared_helper_copies.rs:
- fails when a file outside
tests/common defines its own binary() or git_sha256( and isn't on PENDING_PRIVATE_HELPERS;
- stale entries don't fail, so a PR that migrates a pending file never turns another PR red;
- a detector test covers every former copy shape (
PathBuf and &'static str binaries, nested, pub, both git_sha256 parameter names) and the near misses (git_sha256_file, call sites, imports).
The former copies took five shapes. All of them return the same value as common's:
binary() → env!("CARGO_BIN_EXE_socket-patch"), as a PathBuf or a &'static str. Every call site compiles unchanged against the PathBuf.
git_sha256 → either the Git-blob framing written out (byte-identical to common's), or compute_git_sha256_from_bytes. Common's existing git_sha256_agrees_with_production_hash test pins the two as equal. That test runs in every binary that declares common, and so in every former caller.
Deleted
git diff --stat origin/main: 77 files, +451 / −584. Production lines changed: 0. Everything except the one-line ci.yml port below is test code.
- Migration: −583 / +≈270. The added lines are the
mod common; and use lines.
- Guard: +≈180.
Left for later slices (listed in PENDING_PRIVATE_HELPERS)
CI port
This PR also carries #1118's one-line ci.yml fix: the label on the setup-php pin changes from # v2 to # 2.37.2. zizmor's Audit GitHub Actions check fails every new PR head on main's label until #1118 merges. The change does nothing once #1118 lands.
Behavior
None. No production code changed, and every test keeps its assertions.
Test evidence
cargo test -p socket-patch-cli --all-features --no-run: every test target builds with no warnings in the touched files. The first build listed 71 unused-import warnings left behind by the deleted helpers; all were removed.
cargo test -p socket-patch-cli --all-features --test spawn_env_hygiene: 12 passed. The first push failed it, because the guard's near-miss sample spelled out a bare binary spawn; the sample is now a plain binary() call.
cargo test -p socket-patch-cli --all-features with --test for cli, rollback, remove, apply, scan, vendor, cli_apply_silent, cli_get_silent, cli_remove_silent, crawl_fd_limit_e2e, ecosystem_dispatch_e2e, in_process_rollback_all_ecosystems, maven_sidecar_cli, gradle_agent_cli, covgap_commands_vex, e2e_vex, e2e_vex_lockfile, coverage_fix_apply_silent_mute_exit, coverage_fix_vendor_silent_mute_exit and vendor_crash_safety_e2e: all pass (≈1,300 tests, 0 failed). The guard is included.
- CI on
2c7fffd: all 389 checks green, including coverage (the whole workspace's cargo test --tests), test-release, test (windows-latest) and the e2e matrices. Bugbot is clean.
rustfmt --check: the touched files are clean, except for diffs that are already on main in e2e_redirect_yarn_classic_build.rs and e2e_vendor_yarn_berry_build.rs. Those were left alone.
- Clippy (
--workspace --all-features -- -D warnings) doesn't build test targets, and no production file changed.
Risk
Low. The change is mechanical and test-only. The main risk is a merge conflict with PRs that later touch these test files' import blocks.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Nc6wa5b2SpVJQF9kDbquS6
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
72 CLI test files kept private copies of binary() and git_sha256
that tests/common already provides (five shapes, all returning the
same path and the same Git-blob SHA-256). They now import common's,
and the rollback and e2e_vex_lockfile binaries declare common.
No test behavior changes; files that open PRs change keep their
copies for a later slice (#824).
Assisted-by: Claude Code:claude-opus-5-5
A ratchet in the cli test binary fails when a test file outside
tests/common defines its own binary() or git_sha256, unless it is on
the pending list (files open PRs change, plus the vlt_* shared
modules). Stale entries don't fail, so migrating one never turns
another PR red.
Assisted-by: Claude Code:claude-opus-5-5
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Ports #1118's one-line fix: zizmor's ref-version-mismatch audit
fails every new PR head on main's `# v2` label for this pin. No-op
once #1118 lands.
Assisted-by: Claude Code:claude-opus-5-5
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The detector test's near-miss sample spelled a bare binary spawn,
which spawn_env_hygiene's raw-spawn scan reads as a real one and
fails test-release and coverage. Use a plain binary() call instead.
Assisted-by: Claude Code:claude-opus-5-5
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ 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 2c7fffd. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review (burn-down agent).
- Head:
2c7fffd2c1b41e2aed67e04cb8b5f45abc4cf3d4
- CI: 389/389 check runs green (success/skipped/neutral) on this head, mergeable, no conflicts.
- Bugbot: reviewed this head (Cursor Bugbot check: success), no unresolved review threads.
- Changelog: untouched.
Nothing specific flagged for the reviewer beyond the PR description.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
arch-refactor
PR opened by the scheduled architecture refactor routine
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
refactor
Structural change: duplicated code or logic, missing abstraction, layering, dead code
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|