Skip to content

test(cli): foundation for CLI-contract unit-test campaign#68

Merged
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
tests/cli-contract-foundation
May 20, 2026
Merged

test(cli): foundation for CLI-contract unit-test campaign#68
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
tests/cli-contract-foundation

Conversation

@mikolalysenko
Copy link
Copy Markdown
Contributor

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

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

Base automatically changed from feat/diff-package-pathways to main May 19, 2026 21:16
@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
This was referenced May 20, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@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) merged commit b96a13f into main May 20, 2026
21 checks passed
@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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mikolalysenko @Wenxin-Jiang