GeezerVison is showing github.com — CI: rollback tests assert codes, not sentences (#1089) by mikolalysenko · Pull Request #1552 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Refs #1089 (child 4: rollback). #704 and #1027 have closed, so rollback's --json error is a {code, message} object and this child is unblocked.
What changes
tests/covgap_commands_rollback.rs (3,568 lines, 49 tests, its own binary) is split into the rollback suite binary:
rollback/rollback_failure_paths.rs (29 tests) checks exit code, status, error.code, event errorCodes, warnings[] codes and disk state through common::envelope and rollback_json. Messages are read only for what they must name (an ecosystem, a path, a remedy). It has 0 .contains("…") asserts of 4+ words; the old file had 33.
rollback/rollback_render.rs (27 tests) holds the human output checks, one per message family. This includes the human halves of the old *_in_both_modes tests, which are now split into a JSON test and a render test. 25 sentence asserts remain here, all in the render suite, as the tracker's target design calls for.
rollback/rollback_fixture.rs holds the shared fixtures once: the vendored and hosted fixtures, the mock npm registry, the DirModeGuard, and a single macOS ChflagsGuard where there used to be two inline copies.
- Sentence matches replaced by codes:
vendor_ledger_missing (ledger-less wired lock), vendor_revert_failed (unknown backend), vendor_state_write_failed (ledger save), hosted_restore_refused (per-pin refusal), hosted_write_failed (lockfile write, message match dropped) and manifest_invalid (message match dropped).
- Two human-only behaviour tests now run
--json: the blob-hash directory squatter (already_original count) and the Bun orphaned store entry (not_installed empty, rolledBack names the purl).
coverage_fix_rollback_ecosystem_scoped_hosted.rs moves unchanged into the same binary as rollback/rollback_ecosystem_scoped_hosted.rs. It had no sentence asserts.
cli/envelope_helper_copies.rs: covgap_commands_rollback.rs leaves the pending-private-readers list, because its private parse_envelope/warning_codes copies are gone.
docs/testing/bun-compatibility.md points at the new files.
scripts/ci-test-durations.json (Windows shard timings only): the two removed binaries' entries are dropped, and rollback goes from 4.66s to 6.61s (their runtime summed; per-target compile time is counted separately).
CI cost
Workflows are unchanged. A lean PR run has the same ~28 Linux jobs before and after, at roughly the same job-minutes (~8 min wall). The Linux test shards link two fewer integration-test binaries, which saves a few seconds per shard. That is too small for a ci-perf label.
The moved tests include cfg(unix) and cfg(target_os = "macos") bodies. Locally, I compiled the suite with the macOS cfg flipped to unix, and again with every unix/macOS item compiled out (approximating Windows). Both built with no warnings. The one full-scope run on ab2e710 passed all 99 jobs, including the macOS and Windows test shards: https://github.com/SocketDev/socket-patch/actions/runs/38651659602. The follow-up commit 3c229e2 changes no platform-gated code.
Test evidence
cargo test -p socket-patch-cli --all-features --test rollback: 119 passed. The vanished-manifest choreography test passed 5 runs in a row.
cargo test -p socket-patch-cli --all-features --test cli envelope_helper: 11 passed (the private-reader ratchet).
cargo fmt --all -- --check: clean.
cargo clippy --locked --workspace --all-features -- -D warnings (CI's invocation): clean. With --test rollback, clippy reports nothing in the new files; the errors it shows are in shared modules and appear on main too (e.g. --test repair).
- CI on 3c229e2:
ci-ok and clippy green, and every lean job passed. Cursor Bugbot's one finding (env leakage from the in-process tests) was a false positive: replied with the evidence and resolved; the re-review found no issues.
python3 -m unittest discover -s scripts/tests: 347 OK, 6 skipped. This includes the shard planner test that reads ci-test-durations.json.
/code-review high
Fixed: the vanished-manifest test no longer tells its two interleavings apart by sentence (it now checks whether the message names manifest.json); a redundant negative sentence match is dropped; an inverted failure message is fixed; a shadowing IO_PURL local is renamed; stage_blob now uses common::write_blob; the human legacy-ledger test keeps ../evil.lock inside its own tempdir; the docs list both files; a malformed comment is fixed.
Not taken: move NpmRegistry / DirModeGuard / readonly_dir_enforced into tests/common. Near-identical copies live in the remove and get suites, so merging them is a cross-suite change outside this slice. This PR goes from two copies to one within the rollback binary.
Still open in #1089
Child 5 (covgap_commands_scan_mod.rs, covgap_commands_scan_hosted.rs, covgap_commands_get.rs) and child 6 (render suites and the remaining non-covgap files).
🤖 Generated with Claude Code
https://claude.ai/code/session_01HsgT23cgsoRcwB1reaDykA
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
The rollback coverage-gap tests pinned English sentences in --json
error messages, so copy edits broke them and they hid which error code
a path actually returns. They now live in the rollback suite binary:
- rollback_failure_paths.rs checks exit code, status, error.code,
event errorCodes, warning codes and disk state through
common::envelope (vendor_ledger_missing, vendor_revert_failed,
vendor_state_write_failed, hosted_restore_refused, ...). It has no
sentence asserts of four or more words.
- rollback_render.rs keeps the human output checks, one per message
family, including the human halves of the old "both modes" tests.
- rollback_fixture.rs holds the shared fixtures once.
- The --ecosystems scoped hosted regression test joins the same
binary, so CI links two fewer test binaries.
Refs #1089.
Assisted-by: Claude Code:claude-opus-5-5
- The manifest-vanishes-under-lock test tells its two interleavings
apart by whether the message names manifest.json, not by matching
whole sentences.
- Drop a negative sentence match the JSON parse already enforces, and
fix an inverted failure message in the Bun orphan test.
- Rename a local IO_PURL that shadowed the fixture constant.
- Stage blobs through common::write_blob instead of a private copy.
- The human legacy-ledger test keeps its ../evil.lock target inside
its own tempdir.
- bun-compatibility.md lists both rollback files.
Refs #1089.
Assisted-by: Claude Code:claude-opus-5-5
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 3c229e2. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Labeled Ready for review at head 3c229e2.
- CI: 86/86 checks green (36 success, 50 skipped), no main-wide failures.
- Bugbot: reviewed
3c229e2, no new issues; its one thread is resolved.
- Test-only change (rollback covgap split into the
rollback suite). No CHANGELOG.md change.
- Reviewer focus: that the JSON-path tests now assert
error.code/warning codes rather than English sentences, with sentence checks confined to rollback_render.rs.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Mikola Lysenko (mikolalysenko)
pushed a commit
that referenced
this pull request
Oct 15, 2026
Resolves the ci-test-durations.json and bun-compatibility.md conflicts
with #1552 (rollback split): both old covgap targets are gone and the
doc lists the new suite files.
Assisted-by: Claude Code:claude-opus-5-5
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
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.
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.
|