fix: forward exit codes in err, test, and summary commands#559
Open
FlorianBruniaux wants to merge 2 commits intodevelopfrom
Open
fix: forward exit codes in err, test, and summary commands#559FlorianBruniaux wants to merge 2 commits intodevelopfrom
FlorianBruniaux wants to merge 2 commits intodevelopfrom
Conversation
Commands `rtk err`, `rtk test`, and `rtk summary` were always returning exit code 0, even when the wrapped command failed. The exit code was computed but never forwarded via `std::process::exit`. Fixes #557 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
Sync module count with actual main.rs (64 modules) to pass pre-push validation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
There was a problem hiding this comment.
Pull request overview
Fixes RTK’s err, test, and summary wrappers so they propagate the wrapped command’s non-zero exit status to the caller (addressing #557), aligning RTK behavior with typical CLI/CI expectations.
Changes:
- Forward non-zero exit codes from
runner::run_err,runner::run_test, andsummary::runviastd::process::exit(exit_code). - Add ignored integration-style tests that assert exit code forwarding for
rtk errandrtk test. - Update module counts in
ARCHITECTURE.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/summary.rs |
Computes exit code and exits non-zero after printing/tracking summary output. |
src/runner.rs |
Exits non-zero for err/test modes after printing/tracking; adds ignored integration tests for exit code forwarding. |
ARCHITECTURE.md |
Updates documented module totals. |
Comments suppressed due to low confidence (1)
ARCHITECTURE.md:301
- The updated module totals ("64 modules" / "42 command modules + 22 infrastructure modules") conflict with the breakdown immediately below (currently 34 command / 20 infrastructure). Please reconcile these numbers so the architecture doc is internally consistent.
**Total: 64 modules** (42 command modules + 22 infrastructure modules)
### Module Count Breakdown
- **Command Modules**: 34 (directly exposed to users)
- **Infrastructure Modules**: 20 (utils, filter, tracking, tee, config, init, gain, toml_filter, verify_cmd, etc.)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+278
to
+305
| #[test] | ||
| #[ignore] | ||
| fn test_err_forwards_exit_code_nonzero() { | ||
| // Requires rtk to be installed on PATH (cargo install --path .) | ||
| let output = std::process::Command::new("rtk") | ||
| .args(["err", "sh", "-c", "exit 2"]) | ||
| .output() | ||
| .expect("failed to run rtk — install with: cargo install --path ."); | ||
| assert_eq!(output.status.code(), Some(2)); | ||
| } | ||
|
|
||
| #[test] | ||
| #[ignore] | ||
| fn test_err_forwards_exit_code_zero() { | ||
| let output = std::process::Command::new("rtk") | ||
| .args(["err", "sh", "-c", "exit 0"]) | ||
| .output() | ||
| .expect("failed to run rtk — install with: cargo install --path ."); | ||
| assert_eq!(output.status.code(), Some(0)); | ||
| } | ||
|
|
||
| #[test] | ||
| #[ignore] | ||
| fn test_test_forwards_exit_code_nonzero() { | ||
| let output = std::process::Command::new("rtk") | ||
| .args(["test", "sh", "-c", "exit 1"]) | ||
| .output() | ||
| .expect("failed to run rtk — install with: cargo install --path ."); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
rtk err,rtk test, andrtk summaryalways returned exit code 0, even when the wrapped command failed. The exit code was computed but never forwarded.Reproduction from #557:
Root cause
All three commands called
std::process::exitnowhere after computingexit_code. They returnedOk(())unconditionally.Fix
Added
if exit_code != 0 { std::process::exit(exit_code); }before returning inrunner::run_err,runner::run_test, andsummary::run.Verification
Integration tests added (marked
#[ignore], requirertkon PATH):test_err_forwards_exit_code_nonzerotest_err_forwards_exit_code_zerotest_test_forwards_exit_code_nonzeroCloses #557