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 — Use tests/common's binary() and git_sha256 in CLI tests (#824) by mikolalysenko · Pull Request #1124 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 8, 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 (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

  • Tracking issue #824, register row C30 (register), living document Part 8 (doc/08-tests-ci-docs.md) and summary item 16.
  • Leverage: B 0 (test-only). U 1 (C31's binary merge and the rest of Tracking: share CLI test helpers through one test-support module instead of 100+ per-file copies #824 get smaller). D ≈ 100 copies: 52 binary() and 45 git_sha256 deleted. R L: no production code changes.
  • Every other ranked candidate changes files that one of the 33 open PRs changes. This slice was the highest-leverage work inside the free files.

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)

  • 68 files that open PRs change.
  • The vlt_e2e_common, vlt_hosted_common and vlt_vendor_common modules. Some of the binaries that include them don't declare common yet, and those binaries' files are in open PRs.
  • Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823 (the scrub_socket_env copies) is untouched: its guard file spawn_env_hygiene.rs is changed by Remove --download-mode and the diff download path (#792) #1049.

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.

All reactions
@claude
Start refactor for #824
b9bfd4c
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code arch-refactor PR opened by the scheduled architecture refactor routine labels Oct 8, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 8, 2026
Tracking: share CLI test helpers through one test-support module instead of 100+ per-file copies #824
Closed
7 tasks
Claude (claude) added 2 commits October 8, 2026 10:14
@claude
Use common's binary() and git_sha256 in tests
8012b42
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
@claude
Guard against new private test helper copies
8485886
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
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 8, 2026 10:16
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 8, 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.

@claude
Label the setup-php pin with its real tag
c8ab3e3
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]
cursor Bot reviewed Oct 8, 2026
View reviewed changes

@cursor cursor Bot left a comment •
edited
Loading

Uh oh!

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

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.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

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

All reactions
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 8, 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 8, 2026
View reviewed changes

@cursor cursor Bot left a comment •
edited
Loading

Uh oh!

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

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.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

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

All reactions
@claude
Keep a spawn spelling out of the helper guard
2c7fffd
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
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 8, 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 8, 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 2c7fffd. Configure here.

Sorry, something went wrong.

Uh oh!

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

All reactions
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 8, 2026
@claude
refactor: record #1124 CI lessons
1bd096f
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 8, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

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

Tanmay182003
Tanmay Singla (Tanmay182003) approved these changes Oct 8, 2026
View reviewed changes
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 8, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 8, 2026
Evict failing merge-queue entries ~35 min sooner #1146
Merged
Merged via the queue into main with commit aa558c5 Oct 8, 2026
389 checks passed

Uh oh!

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

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the arch-refactor/824-shared-test-helpers branch October 8, 2026 16:48
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 8, 2026
Fix sbt docker e2e flake on Maven Central blips #1148
Merged
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 8, 2026
@claude
refactor: run 2026-10-08T17:20Z (#1151, merged #1121/#1124/#1021)
f71cabe
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 9, 2026
Use tests/common's binary() and git_sha256 in 40 more CLI tests (#824) #1258
Merged
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

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

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.