GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — CI: share test helpers in 17 more test files (#824) by mikolalysenko · Pull Request #1402 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 11, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

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:

  • The ratchet matches only the names `binary()` and `git_sha256(`, so `git_blob_sha256` (2 files), inline `env!("CARGO_BIN_EXE_socket-patch")` (about 10 files) and `const BINARY` get past it. That needs a wider detector and more migrations, so it is a follow-up for Tracking: share CLI test helpers through one test-support module instead of 100+ per-file copies #824 and not this slice.
  • `e2e_vex_vendor` and `remove_rollback_api_overrides` still build their own `SOCKET_*` scrub instead of using `common::hermetic`. That is Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823's subject (CI: hermetic spawns in scan/repair tests (#823) #1398 covers scan/ and repair/), so it is a follow-up.
  • `has_cargo` and `has_gem` hand-roll `common::has_command`. They behave slightly differently (`success()` vs `is_ok()`), so merging them is a behaviour change and a follow-up.
  • `in_process_remove_repair_lifecycle` and `scan_vendor_e2e` include both `prebuilt_common` (which brings its own `jvm_env`) and `common`, so `jvm_env` selftests run twice there. Fixing that means changing the shared `prebuilt_common` module, which every other includer uses too, so it is out of scope.
  • Each migrated top-level target now also compiles `common` and runs its few millisecond-scale selftests. This is the same trade the earlier slices (Use tests/common's binary() and git_sha256 in 40 more CLI tests (#824) #1258, Use tests/common's binary() and git_sha256 in 5 more CLI test files (#824) #1366) made.

🤖 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.

All reactions
@claude
Share test helpers in 17 more CLI test files
1ba76a0
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
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 11, 2026
Tracking: share CLI test helpers through one test-support module instead of 100+ per-file copies #824
Closed
7 tasks
@claude
Drop duplicate cache_env module in two tests
52739d9
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
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 11, 2026 22:19
@mikolalysenko Mikola Lysenko (mikolalysenko) added ci-perf CI / merge-queue performance finding (profiler routine) and removed ci-perf CI / merge-queue performance finding (profiler routine) labels Oct 11, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

cursor[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor[bot]
cursor Bot reviewed Oct 11, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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.

All reactions
Tanmay182003
Tanmay Singla (Tanmay182003) approved these changes Oct 11, 2026
View reviewed changes
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 11, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 11, 2026
This was referenced Oct 11, 2026
Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823
Closed
CI: hermetic spawns in apply/rollback tests (#823) #1405
Merged
Merged via the queue into main with commit 108084a Oct 11, 2026
86 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-test-helper-copies branch October 11, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@Tanmay182003 Tanmay Singla (Tanmay182003) Tanmay182003 approved these changes

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

Projects

None yet

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.

3 participants

@mikolalysenko @Tanmay182003 @claude
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.