CI: vex tests assert codes, not sentences (#1089)

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Refs #1089 (child 6, vex slice).

What changes

• tests/covgap_commands_vex.rs (1,258 lines, 13 tests) and tests/vex_terminal_output.rs (396 lines, 12 tests) were two test binaries. They are now one tests/vex/ suite binary:
  • vex/vex_failure_paths.rs (9 tests): the --json contracts, read through common::envelope and checked against assert_envelope_invariants. They cover exit code, status, error.code, skipped-event errorCodes, warnings[] codes and disk state: manifest_invalid with stale-doc removal and a FIFO at --output, redirect_ledger_corrupt as an advisory (with and without a manifest), vendor_ledger_corrupt failing closed (with and without a manifest), product_multiple_manifests, the --json dry run, json_requires_output, vex_stale_doc_removed and sorted omission events. Messages are read only for the file they must name and, for the vendor ledger, its cause. The --json halves of the old terminal-output tests moved here.
  • vex/vex_go_patches.rs (3 tests): the go-patches synthesis guards (version-less, stale and tampered replaces, vendor-entry precedence, unselected version, +incompatible key). They now read --json skipped events (go_version_not_selected, go_workspace_override, the tampered purl as the only skip) and the document written to --output, instead of matching omitting … from VEX (…) lines on stderr.
  • vex/vex_render.rs (15 tests): human output, one check per message family. These are the old terminal-output tests plus the human halves of the covgap tests. Duplicates were merged: the corrupt-manifest human check joins the existing one, the three vendor-ledger human runs (with a manifest, without one, and --silent) are one table, and the multi-manifest warning and --silent cases are one loop.
  • vex/vex_fixture.rs: one copy of the record and manifest writers, the corrupt-file writers and their untouched checks, and the runners. It merges the two record/write_manifest copies.
• cli_parse_vex.rs stays its own binary because it mutates process env under serial_test, and that must not share a binary with subprocess tests.
• scripts/ci-test-durations.json folds the old targets' 0.20 s + 0.18 s into vex.
• No production change.

CI cost

No workflow change. A lean PR run stays at about 28 Linux jobs, ~8 min (~110 job-min), before and after. The test shards link one fewer integration-test binary.

Test evidence

• cargo fmt --all -- --check: clean.
• cargo clippy --locked --workspace --all-features -- -D warnings (the CI command): clean. cargo clippy -p socket-patch-cli --all-features --test vex -- -D warnings: clean.
• cargo test -p socket-patch-cli --all-features --test vex: 47 passed (27 vex tests, plus the common self-tests every suite runs).
• cargo test -p socket-patch-cli --all-features --test cli -- helper_copies (the shared-helper and envelope-reader ratchets): 12 passed. --test command_module_layering: 3 passed.
• python3 -m pytest scripts/tests -q: 342 passed, 6 skipped.
• The suite includes a cfg(unix) test (FIFO at --output), so one full-scope ci.yml run was dispatched on d455525: https://github.com/SocketDev/socket-patch/actions/runs/38684769170 (success).
• Lean PR CI on d455525: green. Bugbot reviewed d455525: no issues.

/code-review high

Fixed in d455525:

• The --silent multi-manifest case now deletes the first run's document first, so it proves --silent still writes one.
• The vendor-ledger error must carry its cause (corrupt), and under --silent it must also print no package_not_found omission.
• The redirect-ledger render check again requires malformed and not consulted.
• Skipped events need a non-empty reason.
• An error envelope may omit an empty events array.
• The runner uses hermetic::binary_command().

Not fixed, and why:

• The --json manifest_invalid check doesn't pin "Failed to parse manifest JSON". vex_render.rs::corrupt_manifest_error_names_the_file pins that text for the human output, and Tracking: assert stable codes instead of human sentences in CLI tests, and fold the covgap suites #1089 keeps sentence checks out of the --json suites.
• The exact skipped-event reason sentence isn't pinned for the same reason. The human omission lines in omissions_are_sorted_and_listed_once carry the same phrase.

🤖 Generated with Claude Code

https://claude.ai/code/session_017CGVu5L8YkdSCGj3RxJvtF

---

Note

Low Risk
Test-only reorganization and assertion style changes; no runtime or CLI behavior is modified.

Overview
Consolidates the vex CLI integration tests from two binaries (covgap_commands_vex.rs, vex_terminal_output.rs) into a single tests/vex/ suite with shared vex_fixture.rs.

The refactor splits coverage by output mode: vex_failure_paths.rs and vex_go_patches.rs assert --json envelopes (exit codes, error.code, warnings[], skipped errorCodes, document at --output) via common::envelope helpers instead of matching stderr sentences; vex_render.rs keeps human-terminal checks (one family per test), with duplicated cases merged into tables/loops. Adds JSON-only cases such as json_requires_output, dry-run envelope, sorted skipped events, and stale-doc removal as warning codes. cli_parse_vex.rs stays a separate binary for serial_test env mutation. Updates ci-test-durations.json to replace the old targets with vex. No production code changes.

^{Reviewed by Cursor Bugbot for commit 143dac0. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at 378095e (merge of main into the branch; resolved a scripts/ci-test-durations.json conflict by dropping both covgap_commands_vex (folded into the vex suite here) and covgap_commands_scan_mod (removed on main by #1560)).

• CI: 35/35 run checks green, 19 skipped by path filters; no main-wide failures.
• Bugbot: reviewed 378095e, no findings.
• Local: cargo test -p socket-patch-cli --test vex → 47 passed.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

Final review of 378095e: no blockers. One fix pushed in 143dac0: under --silent, multi_manifest_auto_detect_warning_and_silent again fails if "Multiple project manifests" appears anywhere on stderr. The merged loop had only matched a line that also said "using package.json", which is weaker than the old auto_detect_multi_manifest_warning_suppressed_by_silent. Removed Ready for review until CI and Bugbot pass on the new head.

---

Generated by Claude Code

[cursor]

✅ 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 143dac0. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 143dac0.

• CI: 35/35 run checks green, 19 skipped by path filters (54 total); no main-wide failures.
• Bugbot: reviewed 143dac0, no new issues; no unresolved review threads.
• Reviewer note: test-only change (vex suite asserts error codes via shared envelope readers instead of English sentences); 143dac0 tightens the --silent assertion from the final review. Does not touch CHANGELOG.md.
• Slack announcement could not be sent this run (Slack send tool unavailable); the next run should retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief: #1563 at 143dac0

What it does: This PR only changes tests. It folds covgap_commands_vex.rs and vex_terminal_output.rs into one tests/vex/ binary, which means one fewer link step in CI. The vex failure-path tests now assert error codes through the shared envelope readers instead of English sentences (#1089). Production code is unchanged.

Risk: low. The PR is test-only, and the number of #[test] functions is the same (27 before and after). The one thing to watch is a weakened assertion, and the last final review found and fixed one.

Look here:

• vex/main.rs: the module layout. cli_parse_vex.rs stays separate on purpose, because it mutates env under serial_test.
• vex_fixture.rs:175: the shared vex_command/vex_json helpers and the corrupt-ledger fixtures.
• vex_failure_paths.rs:25: the code-based failure assertions (manifest invalid, FIFO output, corrupt ledgers).
• vex_render.rs:309: the merged --silent test, which 143dac0 tightened back to "the phrase is absent anywhere on stderr".
• scripts/ci-test-durations.json: swaps the two old binaries for vex (0.38s).

Verified:

• Re-read the full diff (7 files, all on topic). There is no debug code and CHANGELOG.md is untouched.
• Ran cargo test -p socket-patch-cli --test vex locally: 47 passed.
• No leftover references to the removed test binaries.
• CI: 35/35 run checks green and 19 skipped by path filters, including ci-ok and clippy. Cursor Bugbot: no new issues on 143dac0. There are no unresolved threads.

Changes I made: none this run. 143dac0 came from the previous final-review pass.

Open questions: none.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code