GeezerVison is showing github.com — CI: share test helpers in 17 more test files (#824) by mikolalysenko · Pull Request #1402 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Refs #824 (slice 4 of the shared-test-helper migration; the issue stays open for the remaining files).
What it does
Test-only. Seventeen CLI test files still defined their own binary() and/or git_sha256 helpers. They now use tests/common's versions: top-level targets through #[path = "common/mod.rs"] mod common;, and the apply/, cli/ and rollback/ directory targets through use crate::common::... Imports that the deleted copies needed (sha2::{Digest, Sha256}, PathBuf) are dropped. scan_vendor_e2e.rs keeps use sha2::Digest because it still calls Sha512::digest.
cli/shared_helper_copies.rs (the ratchet that stops new copies) removes those 17 files from PENDING_PRIVATE_HELPERS, so any copy added back to them now fails the test. It also drops the stale diff_created_file_e2e.rs entry, because that file no longer exists.
Every copy removed here had one of the two shapes the ratchet's header says common pins: the Git-blob sha256("blob <len>\0" ++ content), or env!("CARGO_BIN_EXE_socket-patch"). So no test changes what it computes.
Still pending (unchanged)
repair/covgap_commands_repair.rs, repair/covgap_commands_repair_vendor.rs, repair/repair_invariants.rs, scan/covgap_commands_fetch_stage.rs: open PR CI: hermetic spawns in scan/repair tests (#823) #1398 changes these files, so they stay out of this PR to avoid conflicts.
vex_e2e_common, vlt_e2e_common, vlt_hosted_common, vlt_vendor_common: shared modules whose includers don't all declare mod common.
CI cost
No workflow change. A lean PR run is unchanged: same jobs and roughly the same job-minutes as before (about 28 Linux jobs, about 8 min wall time).
Validation
cargo fmt --all -- --check: clean
cargo test -p socket-patch-cli --all-features on all 17 touched targets (e2e_vex_vendor, the 11 in_process_* files, remove_rollback_api_overrides, scan_vendor_e2e, apply, cli, rollback): all pass with 0 warnings. That includes cli::shared_helper_copies, which checks the shrunken pending list. e2e_vex_vendor and scan_vendor_e2e have no #[ignore] tests.
- Not run locally: the whole
cargo test --workspace, because the sandbox disk could not hold every test binary at once. Workspace clippy does not lint test targets. CI runs both.
/code-review high
No correctness findings. One cleanup fixed in 52739d9: `in_process_alternate_installers` and `in_process_gem_apply` compiled `common/cache_env.rs` twice (once on its own, once inside `mod common`). Both files now use `common::cache_env`, and both targets pass again. Findings left out on purpose:
🤖 Generated with Claude Code
https://claude.ai/code/session_017k2mexmgXxWiKdHrkRuRJC
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Seventeen CLI test files still carried their own copies of the
binary() and git_sha256 helpers. They now use the shared ones in
tests/common, so a fix to either helper reaches every suite at
once.
The helper-copy ratchet (cli/shared_helper_copies.rs) drops these
files from its pending list, plus a stale entry for a deleted file,
so new copies there fail the test. The scan/ and repair/ files that
an open PR touches and the four shared *_common modules stay
pending.
Refs #824
Assisted-by: Claude Code:claude-opus-5-5
in_process_alternate_installers and in_process_gem_apply compiled
tests/common/cache_env.rs twice, once on its own and once inside
mod common, so its selftests ran twice per run. Both files now use
common::cache_env.
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 52739d9. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review at 52739d9.
- CI: 36/36 non-skipped checks green (50 skipped).
- Bugbot: reviewed
52739d9, no findings.
- Mergeable: yes, no conflicts with main. Already approved by Tanmay Singla (@Tanmay182003) at this head.
- Slack announcement not sent: the Slack send tool was unavailable this run, so the next run will retry.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 11, 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.
|