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 — CI: rollback tests assert codes, not sentences (#1089) by mikolalysenko · Pull Request #1552 · SocketDev/socket-patch

Conversation

@mikolalysenko

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

All reactions
@claude
Start fix for #1089 child 4 (rollback)
a40cfcb
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 14, 2026
Tracking: assert stable codes instead of human sentences in CLI tests, and fold the covgap suites #1089
Open
14 tasks
@claude
CI: rollback tests assert codes, not sentences
ab2e710
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
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 14, 2026
Swarm health manager log #1382
Open
@claude
CI: address review of rollback test split
3c229e2
- 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
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 14, 2026 23:09
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

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

cursor[bot]
cursor Bot reviewed Oct 14, 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
Comment thread crates/socket-patch-cli/tests/rollback/main.rs Resolved

Uh oh!

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

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 14, 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 14, 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 3c229e2. Configure here.

Sorry, something went wrong.

Uh oh!

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

All reactions
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 15, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

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

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 15, 2026
View reviewed changes
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 1279a9a Oct 15, 2026
86 checks passed

Uh oh!

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

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-rollback-envelope-asserts branch October 15, 2026 00:56
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 15, 2026
CI: get tests assert codes, not sentences (#1089) #1553
Merged
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 15, 2026
@claude
Merge main into scan hosted test split
a6363e7
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
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 15, 2026
CI performance dashboard #1182
Open
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

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

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.