Skip to content

fix(test): enforce diff dry-run exit code#289

Open
zengxm1979 wants to merge 1 commit into
TestSprite:mainfrom
zengxm1979:codex/fix-diff-dry-run-exit-code
Open

fix(test): enforce diff dry-run exit code#289
zengxm1979 wants to merge 1 commit into
TestSprite:mainfrom
zengxm1979:codex/fix-diff-dry-run-exit-code

Conversation

@zengxm1979

@zengxm1979 zengxm1979 commented Jul 26, 2026

Copy link
Copy Markdown

Summary

Fixes the test diff --dry-run path so it preserves the documented CI-gate behavior when the emitted sample shows a verdict change.

Root cause

The real test diff path prints the diff and throws CLIError(..., 1) when verdictChanged is true. The dry-run path prints a canned sample with verdictChanged: true, but returned successfully before reaching the same exit-1 gate.

Change

  • After printing the dry-run sample, throw the same verdict-difference CLIError when sample.verdictChanged is true.
  • Update the dry-run unit test to assert both behaviors: JSON output is still emitted offline, and the command rejects with exitCode: 1.

Workflow note

Issue #286 has been claimed with /assign per the contribution workflow.

Validation

  • npm test -- src/commands/test.test.ts -t "runDiff"
  • npm run typecheck
  • npm run lint
  • npm run format:check

All checks passed locally on Node v24.15.0 / npm 11.12.1.

Closes #286

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

test diff --dry-run now throws exit code 1 when its sample changes verdicts. The regression test captures and validates the emitted JSON, including run identifiers, verdict changes, and changed-step status transitions.

Changes

Dry-run verdict gate

Layer / File(s) Summary
Enforce and validate dry-run exit behavior
src/commands/test.ts, src/commands/test.test.ts
The dry-run path throws CLIError with exit code 1 when verdictChanged is true, while tests validate the JSON output and rejection behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: ruili-testsprite, zeshi-du

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR implements the requested dry-run exit-1 gate and adds a regression test for the verdict-change case.
Out of Scope Changes check ✅ Passed The changes stay focused on the dry-run exit behavior and its test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: enforcing the diff dry-run exit code behavior.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown

✅ This PR is linked to an issue assigned to @zengxm1979 — thanks! The needs-issue label has been removed.

@github-actions github-actions Bot added the needs-issue PR not linked to an issue yet — please open one first and claim it (see CONTRIBUTING) label Jul 26, 2026
@zengxm1979
zengxm1979 marked this pull request as ready for review July 26, 2026 07:52
@github-actions github-actions Bot removed the needs-issue PR not linked to an issue yet — please open one first and claim it (see CONTRIBUTING) label Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dry-run emits a verdict-changing diff but returns before the documented exit-1 gate

1 participant