GeezerVison is showing github.com — CI: share test helpers in repair/scan tests (#824) by mikolalysenko · Pull Request #1410 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Refs #824 (slice 5; the tracking issue stays open for the four shared *_common modules).
What
Four CLI test files still carried private copies of binary() and git_sha256, held back only because #1398 touched them. #1398 has merged, so they now use crate::common::{binary, git_sha256} like every other file in the repair and scan test binaries (both already declare mod common):
repair/covgap_commands_repair.rsrepair/covgap_commands_repair_vendor.rsrepair/repair_invariants.rsscan/covgap_commands_fetch_stage.rs
The ratchet in cli/shared_helper_copies.rs drops these entries, so a new private copy in any of them now fails no_new_private_binary_or_git_sha256.
Still pending
vex_e2e_common, vlt_e2e_common, vlt_hosted_common and vlt_vendor_common are #[path]-included by dozens of test targets, many of which don't declare mod common. Migrating them needs a separate decision (add mod common to every includer, or re-export) and is left for a later slice.
Test evidence
cargo fmt --all -- --check: clean.cargo check -p socket-patch-cli --all-features --test repair --test scan --test cli: no warnings.cargo test -p socket-patch-cli --all-features --test cli: 110 passed (includes the shared_helper_copies ratchet).--test scan: 125 passed.--test repair: 126 passed, 2 failed. The 2 failures (repair_exits_zero_and_stays_quiet_when_lock_file_unremovable, repair_cleanup_failure_is_reported_in_json_and_silent_modes) fail the same way on unmodified origin/main. They chmod a dir to 0o555, and the sandbox runs as uid 0, which ignores the bit. CI runs as non-root./code-review high: no correctness findings. Fixed: an orphaned doc comment in covgap_commands_fetch_stage.rs, the rewrap of the ratchet comment, and a module-level Digest import moved into sri_of. Not fixed: sri_of is also duplicated in repair/repair_vendor_e2e.rs, but it is not one of the two helpers the Tracking: share CLI test helpers through one test-support module instead of 100+ per-file copies #824 ratchet tracks, so it is left for a later slice to keep this PR scoped.
CI cost
Test-only change. Lean PR run: same jobs (~28 Linux jobs, ~8 min wall) before and after; no workflow changes.
🤖 Generated with Claude Code
https://claude.ai/code/session_0156V3EY4FwW4tMqqcGRHn7J
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
Four repair/ and scan/ test files still kept their own copies of
the binary() and git_sha256 helpers, held back while #1398 changed
them. That PR has merged, so they now use the shared ones in
tests/common, and the helper-copy ratchet drops them from its
pending list so a new copy there fails the test. Only the four
shared *_common modules stay pending.
Refs #824
Assisted-by: Claude Code:claude-opus-5-5
Drop a doc comment left with no item under it, rewrap the ratchet's
pending-list comment, and import the sha2 Digest trait only inside
the one helper that uses it.
Refs #824
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 e1b04a6. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] coverage failed on e1b04a6 in -p socket-patch-cli --test in_process_edge_cases (job). This PR does not change that suite or anything it compiles: the diff only touches repair/, scan/ and cli/shared_helper_copies.rs, and tests/common/mod.rs is unchanged. Locally the suite passes 6 of 6 runs, with and without --all-features. The log API only returns the tail, which leaves out the failing test name. No fix exists to port yet, so I re-ran the job once. If it fails again I will treat it as real and root-cause it.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review (burn-down agent)
- Head:
e1b04a6
- CI: 35/35 non-skipped checks green (50 skipped by path filter);
coverage passed on its one re-run after a single failure in in_process_edge_cases
- Bugbot: reviewed
e1b04a6, no new issues; no unresolved review threads
- Reviewer focus: test-only change. Four repair/scan test files drop their private
binary()/git_sha256 copies and use tests/common
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Final review brief
What it does: Test-only cleanup for #824. Four repair/scan test files drop their private copies of binary() and git_sha256() and import the identical ones from tests/common; the four files come off the PENDING_PRIVATE_HELPERS allowlist that the shared-helper-copies guard test checks.
Risk: low. No production code touched; the removed helpers are byte-for-byte the same as common::binary / common::git_sha256.
Look here:
- shared_helper_copies.rs:15:`` allowlist shrinks by four entries
- covgap_commands_repair_vendor.rs:43:```
sri_of now imports `Digest as _` locally, since the file-level `sha2::Digest` import went away
- covgap_commands_fetch_stage.rs:21:```` also drops a dangling doc comment that had no item under it
Verified: read the full diff; compared the removed helpers with tests/common/mod.rs (same signatures and bodies); CHANGELOG.md untouched; no review threads. CI: ci-ok and clippy green on the head. Bugbot: found no issues on this head (its later check run shows as cancelled).
Changes I made: none.
Open questions: none.
Auto-merge is armed, so approving sends it straight to the merge queue.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 12, 2026
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
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
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.
|