GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||
GeezerVison is showing github.com — CI: update tests assert codes, not sentences (#1089) by mikolalysenko · Pull Request #1580 · SocketDev/socket-patch Conversation
|
| Old | New |
|---|---|
covgap_commands_update.rs already-latest --json |
update/update_json_contracts.rs: skipped/already_latest event, details.current/latest, non-empty reason, summary, warnings omitted, assert_envelope_invariants; the reason sentence assert is gone |
covgap_commands_update.rs pin-to-current (human) |
update_json_contracts.rs with --json: one skipped event, dryRun:false, zero network, notifier cache not written; plus one wording check in update/update_render.rs |
covgap_commands_update.rs human dry-runs + PTY decline |
update/update_render.rs, one check per message family |
covgap_update_swap.rs (asserted Updated socket-patch) |
update_json_contracts.rs with --json: downloaded + updated events (from/to), plus the same disk, inode and lock-file proofs |
covgap_update_download.rs |
self_update_failures_e2e.rs::asset_server_error_is_download_failed, next to its 404 sibling |
DEAD_ENDPOINT moves into tests/common/update_fixture.rs. No product code changes.
CI cost: test-only. Lean PR run is unchanged: the same suites and job count, and the same test count (2 tests become 4 checks, all zero-network or local wiremock, under a second each).
Test evidence
cargo test -p socket-patch-cli --all-features --test update --test self_update_failures_e2e --test self_update_e2e: 63 + 38 + 30 passed.cargo fmt --all -- --check,cargo clippy --locked --workspace --all-features -- -D warnings: clean.- This PR moves tests, so there is no red→green regression test. The moved checks cover the same paths as before, and the pinned and unlocked-swap paths now also check their
--jsonenvelope.
/code-review high findings
Fixed: the bare "500" match (it could hit the mock's port), the missing reason check, the pinned test's missing dryRun/summary checks, DEAD_ENDPOINT placement, and the render-suite doc.
Not fixed:
- Pinned no-op reports
already_latestwithlatest = <pin>. That is product behavior. This PR no longer asserts that code or field for the pinned path, so it doesn't lock them in. Whether the pinned envelope should use a different code is a product decision for a separate issue. - The hand-written 500 wiremock mock should become a
FakeReleaseBuilderoption. It was moved verbatim from the covgap file. Adding a builder option would widen this PR for one caller.
🤖 Generated with Claude Code
https://claude.ai/code/session_017tesJSyAELgQJ9X4mF2mWn
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Assisted-by: Claude Code:claude-opus-5-5
The self-update covgap tests pinned English sentences, so a copy edit to an update message broke tests that were really about behavior. - update_json_contracts.rs: the already-latest and pin-to-current no-ops and the unlocked swap now read the --json envelope (event actions, already_latest code, from/to details) plus disk state. - update_render.rs: the human wording (pinned already-there, both dry-run reports, the PTY reinstall decline), one check per message. - The download_failed 500 test joins its 404 sibling in self_update_failures_e2e.rs. No covgap files remain in the update suite. Refs #1089. Assisted-by: Claude Code:claude-opus-5-5
- The 500 test matches "returned 500" again: a bare "500" could hit the mock server's random port. - The already-latest event must still carry a non-empty reason. - The pinned no-op checks dryRun and summary like its sibling, and no longer pins already_latest/latest for a pin (a pin says nothing about the latest release). - DEAD_ENDPOINT lives in update_fixture for both update files. Refs #1089. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
All reactions
-
👍 1 reaction
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 9dd5b35. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
3 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.