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 — test(cli): foundation for CLI-contract unit-test campaign by mikolalysenko · Pull Request #68 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented May 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Foundation PR for a comprehensive unit-test campaign on the socket-patch CLI. Stacked on #67 (so it can land alongside the per-package/diff pathways work). No behavior change to the binary. Follow-up PRs will be stacked on this branch and add the actual per-command tests in parallel.

Three changes:

  1. Expose the clap parser as a library so integration tests can verify the public CLI contract without spawning the binary:

    • New crates/socket-patch-cli/src/lib.rs hosting Cli, Commands, looks_like_uuid, and parse_with_uuid_fallback (extracted from main.rs).
    • Cargo.toml gains [lib] entry alongside the existing [[bin]].
    • main.rs is now a thin wrapper that delegates to the lib.
  2. De-duplicate manifest-path resolution. The same 5-line absolute-vs-relative join block was copy-pasted across apply/rollback/list/remove/repair. New helper socket_patch_core::manifest::operations::resolve_manifest_path replaces all 5 copies. 3 unit tests cover the relative/absolute/dotted cases.

  3. New crates/socket-patch-cli/CLI_CONTRACT.md documenting every subcommand, flag, default value, alias, JSON-output shape, and exit code as semver-significant surface. Spells out the bump policy (rename a flag → MAJOR; add an optional flag → MINOR; …) and how to invoke scripts/version-sync.sh afterward. A 4-line comment block above pub enum Commands points readers at it.

Why now

The CLI crate currently has zero unit tests under src/ — only network-dependent tests/e2e_*.rs suites gated behind --ignored. A flag rename, a default change, or a JSON key drift could land green and silently break every shipped wrapper (npm @socketsecurity/socket-patch, pypi socket-patch, cargo, prebuilt binaries). This PR lays the groundwork; the follow-up PRs will lock in every flag and helper.

What lands later (stacked on this branch)

10 parallel test PRs:

  • tests/cli-parse-main — looks_like_uuid + parse_with_uuid_fallback (the bare-UUID fallback no test guards today)
  • tests/cli-parse-output — format_severity, color, confirm
  • tests/cli-parse-ecosystem-dispatch — partition_purls
  • tests/cli-parse-apply — every ApplyArgs flag + verify_status_str + result_to_json
  • tests/cli-parse-rollback — every RollbackArgs flag + find_patches_to_rollback
  • tests/cli-parse-get — every GetArgs flag, download visible alias, hidden --no-apply alias, detect_identifier_type, select_patches
  • tests/cli-parse-scan — every ScanArgs flag, --batch-size default = 100
  • tests/cli-parse-list — every ListArgs flag + async run tests (missing/empty/populated manifest)
  • tests/cli-parse-repair — every RepairArgs flag, gc visible alias, pin --download-mode default = file (the divergent one)
  • tests/cli-parse-remove-setup — both RemoveArgs/SetupArgs flags + async error-path JSON shape tests

Each will be a sibling PR targeting tests/cli-contract-foundation.

Test plan

  • cargo build --workspace --all-features — clean
  • cargo clippy --workspace --all-features -- -D warnings — clean
  • cargo test --workspace --all-features --lib — 415 unit tests pass (was 412; +3 from resolve_manifest_path tests)
  • cargo test --workspace --all-features --no-run — all 8 e2e test binaries still compile
  • Smoke: socket-patch download --help, socket-patch gc --help, socket-patch get --no-apply <id> --help, bare-UUID socket-patch <uuid> all behave identically to pre-refactor

Assisted-by: Claude Code:claude-opus-4-7

Sorry, something went wrong.

Uh oh!

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

All reactions
Base automatically changed from feat/diff-package-pathways to main May 19, 2026 21:16
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the tests/cli-contract-foundation branch from 0bd4f50 to 6cb84b8 Compare May 20, 2026 18:47
@mikolalysenko
test(cli): comprehensive unit-test campaign for the CLI contract
ef0deac
Add unit-test coverage for every subcommand, flag, default, alias, and
helper in the socket-patch CLI, plus a new CLI_CONTRACT.md that documents
the surface as semver-significant.

Before this change the CLI crate had zero unit tests under src/ —
only network-dependent tests/e2e_*.rs suites gated on --ignored. A flag
rename, a default change, or a JSON key drift could land green and break
every shipped wrapper (npm @socketsecurity/socket-patch, pypi
socket-patch, cargo, prebuilt binaries).

## What's covered

Library surface (new src/lib.rs):
  - Cli, Commands, looks_like_uuid, parse_with_uuid_fallback extracted
    from main.rs so integration tests can verify the parser without
    spawning the binary.
  - main.rs becomes a thin wrapper that delegates to the lib.
  - Cargo.toml gains [lib] alongside [[bin]].

Core helper extraction:
  - socket_patch_core::manifest::operations::resolve_manifest_path
    replaces the 5-line absolute-vs-relative join block previously
    copy-pasted into apply/rollback/list/remove/repair (5 callers).

CLI_CONTRACT.md (new):
  - Documents every subcommand, flag, default value, visible alias
    (download, gc), hidden alias (--no-apply), JSON output shape, and
    exit code as semver-significant.
  - Pins the divergent defaults: --download-mode=diff for apply/get/scan
    and --download-mode=file for repair, --batch-size=100 for scan.
  - Spells out the bump policy and how to invoke scripts/version-sync.sh.

Helper unit tests (#[cfg(test)] mod tests in each file):
  - src/lib.rs — looks_like_uuid (valid/invalid shapes, case-insensitive),
    parse_with_uuid_fallback (success, fallback, fallback-fails preserves
    original error, no double-rewrite).
  - src/output.rs — format_severity, color, confirm (skip_prompt and
    is_json short-circuits; interactive path intentionally not tested).
  - src/ecosystem_dispatch.rs — partition_purls (filter, dedup, unknown
    ecosystem dropped).
  - commands/apply.rs — verify_status_str (all 4 VerifyStatus variants),
    result_to_json (top-level + filesVerified key sets).
  - commands/rollback.rs — find_patches_to_rollback (None=all, PURL match,
    UUID match, no-match=empty).
  - commands/get.rs — detect_identifier_type (UUID/CVE/GHSA/PURL/None,
    case-insensitive for CVE+GHSA), select_patches (paid auto, free
    single auto, free multi+is_json -> Err(1)).

Clap parser snapshot tests (new tests/cli_parse_*.rs):
  - One file per command: apply, get, list, main, remove, repair,
    rollback, scan, setup.
  - Every flag (long + short) has at least one parser assertion.
  - Every #[arg(default_value)] is asserted on the no-flag parse.
  - The download visible alias on get is exercised.
  - The gc visible alias on repair is exercised.
  - The hidden --no-apply alias on get --save-only is exercised.
  - --ecosystems CSV splitting is verified on apply/rollback/scan.
  - The bare-UUID rewrite is exercised end-to-end via Cli::try_parse_from.
  - Failure paths assert clap::error::ErrorKind variants.

Async run() integration tests:
  - tests/cli_parse_list.rs covers missing manifest -> 1, empty -> 0,
    populated -> 0, absolute-path override, and a subprocess JSON-shape
    assertion against the compiled binary.
  - tests/cli_parse_remove.rs covers missing-manifest -> 1.
  - tests/cli_parse_setup.rs covers no-package-json -> 0 with the
    JSON status:"no_files" shape pinned via subprocess.

## Verification

  cargo build --workspace --all-features
  cargo clippy --workspace --all-features -- -D warnings
  cargo test --workspace --all-features

All clean. 79 new lib tests + 156 new integration tests added on top of
the existing 415 unit tests; cumulative 650 tests pass.

## Why squashed

Originally landed as a foundation PR (#68) plus 10 sibling test PRs
(#69-#78), one per command/file, dispatched in parallel. Squashing the
sibling PRs back into #68 so the contract + tests land as one
self-contained unit — reviewers see the full picture, and a future
revert touches one commit instead of eleven.

Assisted-by: Claude Code:claude-opus-4-7
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the tests/cli-contract-foundation branch from 6cb84b8 to ef0deac Compare May 20, 2026 19:05
This was referenced May 20, 2026
test(cli): unit-test output.rs format_severity/color/confirm #69
Closed
test(cli): unit-test partition_purls ecosystem filter #70
Closed
test(cli): pin RollbackArgs flag surface + find_patches_to_rollback #71
Closed
test(cli): unit-test looks_like_uuid + parse_with_uuid_fallback #72
Closed
test(cli): pin ApplyArgs flag surface + verify_status_str #73
Closed
test(cli): pin ScanArgs flag surface (batch-size=100, download-mode=diff) #74
Closed
test(cli): pin RepairArgs flag surface (download-mode=file, gc alias) #75
Closed
test(cli): pin GetArgs flag surface, download/no-apply aliases, detect_identifier_type #76
Closed
test(cli): pin ListArgs flag surface + run() error/empty/populated paths #77
Closed
test(cli): pin RemoveArgs/SetupArgs flag surface + run() error paths #78
Closed
Mikola Lysenko (mikolalysenko) added 2 commits May 20, 2026 15:09
@mikolalysenko
ci: tighten CI matrix — add macOS, release-mode tests, explicit build…
99f5fc6
… step

The CI workflow's unit-test job ran on ubuntu-latest + windows-latest;
extend the matrix with macos-latest so the new CLI parser tests are
exercised on every platform the binary ships for. socket-patch ships
prebuilt binaries for x86_64-apple-darwin and aarch64-apple-darwin (see
release.yml), so silent macOS-specific regressions in path handling,
TTY detection, or terminal escapes are real risks today.

Three changes:

  - Add `macos-latest` to the test matrix.
  - Add `fail-fast: false` so a failure on one OS doesn't mask failures
    on the others.
  - Add an explicit `cargo build --workspace --all-features` step
    before `cargo test`. `cargo test` already builds, but a dedicated
    build step gives a cleaner red signal when a build-only failure
    happens (e.g. a feature-gated compile error) without the noise of
    test-discovery output.
  - New `test-release` job: `cargo test --workspace --all-features
    --release` on ubuntu-latest. Catches optimization-level regressions
    that debug mode hides (e.g. release-mode-only inlining changes that
    affect assertion behavior). One OS keeps total CI time reasonable
    while still locking in release-mode correctness.

Assisted-by: Claude Code:claude-opus-4-7
@mikolalysenko
fix(patch): reject POSIX-style absolute paths in archive entries on W…
9950835
…indows

`read_archive_to_map` rejects entries whose path is absolute or contains
a `..` component, but the check used `Path::is_absolute()` alone. On
Windows that function requires a drive letter or UNC prefix, so a tar
entry like `/etc/passwd` is NOT considered absolute and would slip
through the guard — when later joined to the target directory, Windows
would treat it as relative to the current drive's root.

Add an explicit check for a leading `/` or `\` byte alongside
`Path::is_absolute()` so the guard rejects POSIX-style absolute paths
on every platform. The new test_read_archive_rejects_backslash_absolute_paths
case locks the symmetric backslash form in.

This was uncovered when the CI matrix was extended to actually run on
Windows. The existing test_read_archive_rejects_absolute_paths failed on
windows-latest because it constructed the archive with a POSIX-style
path that the platform-specific `is_absolute()` did not catch.

Assisted-by: Claude Code:claude-opus-4-7
@mikolalysenko
Mikola Lysenko (mikolalysenko) enabled auto-merge (squash) May 20, 2026 20:13
Wenxin-Jiang
Wenxin Jiang (Wenxin-Jiang) approved these changes May 20, 2026
View reviewed changes
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit b96a13f into main May 20, 2026
21 checks passed

Uh oh!

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

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the tests/cli-contract-foundation branch May 20, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Wenxin-Jiang Wenxin Jiang (Wenxin-Jiang) Wenxin-Jiang approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

2 participants

@mikolalysenko @Wenxin-Jiang
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.