Skip to content

Assert error codes instead of sentences in remove's covgap suite and add shared JSON assertion helpers #1090

Description

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: discussion #560 register.

Kind: refactor. Source: Part 8.5 G/H (register C32). Child 1 of #1089. Measured on main @ 05ecc6e.

Problem

remove/covgap_commands_remove.rs (2,127 lines, 35 tests) pins 27 sentences of four or more words. Some examples:

  • "Removed 1 patch from manifest:" appears 4 times (L832, L1224, L1548, L1650).
  • "The manifest was not modified." (L435, L985).
  • "cannot restore pkg:npm/left-pad@1.3.0 to its upstream registry entry" (L982, L1014).
  • "Warning: blob cleanup failed", "…diffs cleanup failed" and "…packages cleanup failed" (L1654-L1662).

remove already emits the typed envelope (status, error.code, events[], summary). The file even has parse_envelope/event_purls helpers (L36-L55), but they are private to this file, while sibling suites re-implement them. Several tests are _json/_human twins over the same path, for example L332-L405.

Symptoms: none filed. The cost shows up as churn in copy-edit PRs such as #248 and #1043.

Proposed change

  1. Add tests/common/envelope.rs with:

    • parse_envelope(stdout);
    • assert_error_code(&Value, &str);
    • event_codes(&Value) -> Vec<(action, code)>;
    • event_purls(&Value, action).

    Delete this file's private copies.

  2. Rewrite each sentence assertion as the equivalent --json assertion: the error code, event action/code, summary count, or the file bytes on disk.

  3. Delete each _human twin whose _json sibling covers the path. Where the human rendering is the point (the prompt text at L1854-L1862), keep one assertion per message family.

  4. Rename the file into the remove suite (for example remove/remove_failure_paths.rs) when done.

Size and scope

Acceptance criteria

  • At most 3 sentence (4+-word) .contains assertions remain in the file, each in a render-focused test.
  • cargo test -p socket-patch-cli --test remove (or the renamed target) is green, as are the whole CLI tests.
  • Coverage of commands/remove.rs is not lower than on main (coverage job).
  • tests/common/envelope.rs is used by this file and documented for the next children.

Dependencies


Backlog review — 2026-10-08

Consolidated into #1089. The retained tracker(s) preserve this issue’s implementation scope and acceptance criteria. Closing this separate scheduling item as not planned, not as completed.

Explicit remove-test child of the stable-error-code test tracker; keep it as a checklist step.

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Oct 7, 2026
  2. added a commit that references this issue on Oct 7, 2026
  3. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triaged: priority:p3 (test-suite refactor, child 1 of #1089). Not a duplicate; no open PR covers it.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions