Skip to content

fix(cli/rustup-mode): improve exit code of rustup check #4694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rami3l merged 2 commits into from
Jan 31, 2026
Merged

fix(cli/rustup-mode): improve exit code of rustup check #4694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ab48ff
refactor(test)!: pass status code directly to `SanitizedOutput`
rami3l Jan 28, 2026
9becf3b
fix(cli/rustup-mode): improve exit code of `rustup check`
rami3l Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/cli/rustup_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,8 +927,7 @@ async fn check_updates(cfg: &Cfg<'_>, opts: CheckOpts) -> Result<ExitCode> {
update_available = true;
}

let exit_status = if update_available { 0 } else { 1 };
Ok(ExitCode(exit_status))
Ok(ExitCode(if update_available { 100 } else { 0 }))
Copy link
Member

@ChrisDenton ChrisDenton Jan 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think we should start using constants for exit codes (e.g. EXIT_SUCCESS, EXIT_UPDATE_AVAILABLE, etc). If not now then soon. Especially if we want to make any more commands return special exit codes.

Copy link
Member Author

@rami3l rami3l Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisDenton Nice catch! I'd say that might belong to a different patch though.

@FranciscoTGouveia would you be interested in trying this one?

}

async fn update(
Expand Down
29 changes: 19 additions & 10 deletions src/test/clitools.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,18 @@ impl Assert {

/// Asserts that the command exited with an ok status.
pub fn is_ok(&self) -> &Self {
assert!(self.output.ok);
self
self.has_code(0)
}

/// Asserts that the command exited with an error.
pub fn is_err(&self) -> &Self {
assert!(!self.output.ok);
assert_ne!(self.output.status, Some(0));
self
}

/// Asserts that the command exited with the specific code.
pub fn has_code(&self, code: i32) -> &Self {
assert_eq!(self.output.status, Some(code));
self
}

Expand Down Expand Up @@ -350,7 +355,7 @@ impl Config {

pub async fn expect_ok_contains(&self, args: &[&str], stdout: &str, stderr: &str) {
let out = self.run(args[0], &args[1..], &[]).await;
if !out.ok || !out.stdout.contains(stdout) || !out.stderr.contains(stderr) {
if out.status != Some(0) || !out.stdout.contains(stdout) || !out.stderr.contains(stderr) {
print_command(args, &out);
println!("expected.ok: true");
print_indented("expected.stdout.contains", stdout);
Expand All @@ -362,7 +367,11 @@ impl Config {
pub async fn expect_ok_eq(&self, args1: &[&str], args2: &[&str]) {
let out1 = self.run(args1[0], &args1[1..], &[]).await;
let out2 = self.run(args2[0], &args2[1..], &[]).await;
if !out1.ok || !out2.ok || out1.stdout != out2.stdout || out1.stderr != out2.stderr {
if out1.status != Some(0)
|| out2.status != Some(0)
|| out1.stdout != out2.stdout
|| out1.stderr != out2.stderr
{
print_command(args1, &out1);
println!("expected.ok: true");
print_command(args2, &out2);
Expand All @@ -373,7 +382,7 @@ impl Config {

pub async fn expect_component_executable(&self, cmd: &str) {
let out1 = self.run(cmd, ["--version"], &[]).await;
if !out1.ok {
if out1.status != Some(0) {
print_command(&[cmd, "--version"], &out1);
println!("expected.ok: true");
panic!()
Expand All @@ -382,7 +391,7 @@ impl Config {

pub async fn expect_component_not_executable(&self, cmd: &str) {
let out1 = self.run(cmd, ["--version"], &[]).await;
if out1.ok {
if out1.status == Some(0) {
print_command(&[cmd, "--version"], &out1);
println!("expected.ok: false");
panic!()
Expand Down Expand Up @@ -1092,7 +1101,7 @@ pub fn print_command(args: &[&str], out: &SanitizedOutput) {
}
}
println!();
println!("out.ok: {}", out.ok);
println!("out.status: {:?}", out.status);
print_indented("out.stdout", &out.stdout);
print_indented("out.stderr", &out.stderr);
}
Expand Down Expand Up @@ -1120,7 +1129,7 @@ pub struct Output {

#[derive(Debug, Clone)]
pub struct SanitizedOutput {
pub ok: bool,
pub status: Option<i32>,
pub stdout: String,
pub stderr: String,
}
Expand All @@ -1130,7 +1139,7 @@ impl TryFrom<Output> for SanitizedOutput {

fn try_from(out: Output) -> Result<Self, Self::Error> {
Ok(Self {
ok: matches!(out.status, Some(0)),
status: out.status,
stdout: String::from_utf8(out.stdout)?,
stderr: String::from_utf8(out.stderr)?,
})
Expand Down
14 changes: 7 additions & 7 deletions tests/suite/cli_exact.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ async fn check_updates_none() {
cx.config
.expect(["rustup", "check"])
.await
.is_err()
.is_ok()
.with_stdout(snapbox::str![[r#"
stable-[HOST_TRIPLE] - up to date: 1.1.0 (hash-stable-1.1.0)
beta-[HOST_TRIPLE] - up to date: 1.2.0 (hash-beta-1.2.0)
Expand All @@ -205,7 +205,7 @@ async fn check_updates_some() {
cx.config
.expect(["rustup", "check"])
.await
.is_ok()
.has_code(100)
.with_stdout(snapbox::str![[r#"
stable-[HOST_TRIPLE] - update available: 1.0.0 (hash-stable-1.0.0) -> 1.1.0 (hash-stable-1.1.0)
beta-[HOST_TRIPLE] - update available: 1.1.0 (hash-beta-1.1.0) -> 1.2.0 (hash-beta-1.2.0)
Expand All @@ -230,7 +230,7 @@ async fn check_updates_self() {
.expect(["rustup", "check"])
.await
.extend_redactions([("[TEST_VERSION]", test_version)])
.is_ok()
.has_code(100)
.with_stdout(snapbox::str![[r#"
rustup - Update available : [CURRENT_VERSION] -> [TEST_VERSION]

Expand All @@ -252,7 +252,7 @@ async fn check_updates_self_no_change() {
cx.config
.expect(["rustup", "check"])
.await
.is_err()
.is_ok()
.with_stdout(snapbox::str![[r#"
rustup - Up to date : [CURRENT_VERSION]

Expand All @@ -272,7 +272,7 @@ async fn check_updates_with_update() {
cx.config
.expect(["rustup", "check"])
.await
.is_err()
.is_ok()
.with_stdout(snapbox::str![[r#"
stable-[HOST_TRIPLE] - up to date: 1.0.0 (hash-stable-1.0.0)
beta-[HOST_TRIPLE] - up to date: 1.1.0 (hash-beta-1.1.0)
Expand All @@ -285,7 +285,7 @@ nightly-[HOST_TRIPLE] - up to date: 1.2.0 (hash-nightly-1)
cx.config
.expect(["rustup", "check"])
.await
.is_ok()
.has_code(100)
.with_stdout(snapbox::str![[r#"
stable-[HOST_TRIPLE] - update available: 1.0.0 (hash-stable-1.0.0) -> 1.1.0 (hash-stable-1.1.0)
beta-[HOST_TRIPLE] - update available: 1.1.0 (hash-beta-1.1.0) -> 1.2.0 (hash-beta-1.2.0)
Expand All @@ -296,7 +296,7 @@ nightly-[HOST_TRIPLE] - update available: 1.2.0 (hash-nightly-1) -> 1.3.0 (hash-
cx.config
.expect(["rustup", "check"])
.await
.is_ok()
.has_code(100)
.with_stdout(snapbox::str![[r#"
stable-[HOST_TRIPLE] - update available: 1.0.0 (hash-stable-1.0.0) -> 1.1.0 (hash-stable-1.1.0)
beta-[HOST_TRIPLE] - up to date: 1.2.0 (hash-beta-1.2.0)
Expand Down
6 changes: 3 additions & 3 deletions tests/suite/cli_inst_interactive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn run_input_with_env(config: &Config, args: &[&str], input: &str, env: &[(&str,
let out = child.wait_with_output().unwrap();

Assert::new(SanitizedOutput {
ok: out.status.success(),
status: out.status.code(),
stdout: String::from_utf8(out.stdout).unwrap(),
stderr: String::from_utf8(out.stderr).unwrap(),
})
Expand Down Expand Up @@ -276,7 +276,7 @@ async fn with_no_toolchain_doesnt_hang() {
)
.is_ok();

cx.config.expect(["rustup", "check"]).await.is_err();
cx.config.expect(["rustup", "check"]).await.is_ok();
}

#[tokio::test]
Expand All @@ -296,7 +296,7 @@ async fn with_no_toolchain_doesnt_hang_with_concurrent_downloads_override() {
cx.config
.expect_with_env(["rustup", "check"], [("RUSTUP_CONCURRENT_DOWNLOADS", "2")])
.await
.is_err();
.is_ok();
}

#[tokio::test]
Expand Down
4 changes: 2 additions & 2 deletions tests/suite/cli_rustup_ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async fn rustup_check_updates_none() {
None,
))
.with_stderr("")
.is_err();
.is_ok();
}

#[tokio::test]
Expand All @@ -108,7 +108,7 @@ async fn rustup_check_updates_some() {
None,
))
.with_stderr("")
.is_ok();
.has_code(100);
}

#[test]
Expand Down