From c64aa50c0e4219ef623a6ec3b939ac10a4568563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 11 Sep 2024 16:19:45 +0100 Subject: [PATCH] feat(upgrade): better error message on failure (#25503) Co-authored-by: crowlkats --- cli/args/flags.rs | 20 ++++++---- cli/tools/upgrade.rs | 37 +++++++++++++++---- .../upgrade/invalid_version/__test__.jsonc | 19 ++++++++++ .../specs/upgrade/invalid_version/canary.out | 17 +++++++++ .../upgrade/invalid_version/shorthand.out | 17 +++++++++ .../specs/upgrade/invalid_version/version.out | 17 +++++++++ 6 files changed, 112 insertions(+), 15 deletions(-) create mode 100644 tests/specs/upgrade/invalid_version/__test__.jsonc create mode 100644 tests/specs/upgrade/invalid_version/canary.out create mode 100644 tests/specs/upgrade/invalid_version/shorthand.out create mode 100644 tests/specs/upgrade/invalid_version/version.out diff --git a/cli/args/flags.rs b/cli/args/flags.rs index fbf58fff2f..6d26117499 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -2986,12 +2986,8 @@ The declaration file could be saved and used for typing information.", ) } -fn upgrade_subcommand() -> Command { - command( - "upgrade", - cstr!("Upgrade deno executable to the given version. - -Latest +pub static UPGRADE_USAGE: &str = cstr!( + "Latest deno upgrade Specific version @@ -3002,7 +2998,15 @@ fn upgrade_subcommand() -> Command { Channel deno upgrade stable deno upgrade rc - deno upgrade canary + deno upgrade canary" +); + +fn upgrade_subcommand() -> Command { + command( + "upgrade", + color_print::cformat!("Upgrade deno executable to the given version. + +{} The version is downloaded from https://dl.deno.land and is used to replace the current executable. @@ -3010,7 +3014,7 @@ If you want to not replace the current Deno executable but instead download an u different location, use the --output flag: deno upgrade --output $HOME/my_deno -Read more: https://docs.deno.com/go/cmd/upgrade"), +Read more: https://docs.deno.com/go/cmd/upgrade", UPGRADE_USAGE), UnstableArgsConfig::None, ) .hide(cfg!(not(feature = "upgrade"))) diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs index 46c12ba81c..826bce40ae 100644 --- a/cli/tools/upgrade.rs +++ b/cli/tools/upgrade.rs @@ -4,6 +4,7 @@ use crate::args::Flags; use crate::args::UpgradeFlags; +use crate::args::UPGRADE_USAGE; use crate::colors; use crate::factory::CliFactory; use crate::http_util::HttpClient; @@ -15,7 +16,6 @@ use crate::util::progress_bar::ProgressBarStyle; use crate::version; use async_trait::async_trait; -use color_print::cstr; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; @@ -38,8 +38,6 @@ const RELEASE_URL: &str = "https://github.com/denoland/deno/releases"; const CANARY_URL: &str = "https://dl.deno.land/canary"; const RC_URL: &str = "https://dl.deno.land/release"; -static EXAMPLE_USAGE: &str = cstr!("Example usage:\n deno upgrade | deno upgrade 1.46 | deno upgrade canary"); - pub static ARCHIVE_NAME: Lazy = Lazy::new(|| format!("deno-{}.zip", env!("TARGET"))); @@ -625,6 +623,7 @@ impl RequestedVersion { }; let mut maybe_passed_version = upgrade_flags.version.clone(); + // TODO(bartlomieju): prefer flags first? This whole logic could be cleaned up... if let Some(val) = &upgrade_flags.version_or_hash_or_channel { if let Ok(channel) = ReleaseChannel::deserialize(&val.to_lowercase()) { // TODO(bartlomieju): print error if any other flags passed? @@ -651,18 +650,19 @@ impl RequestedVersion { let (channel, passed_version) = if is_canary { if !re_hash.is_match(&passed_version) { bail!( - "Invalid commit hash passed ({})\n\n{}", + "Invalid commit hash passed ({})\n\nPass a semver, or a full 40 character git commit hash, or a release channel name.\n\nUsage:\n{}", colors::gray(passed_version), - EXAMPLE_USAGE + UPGRADE_USAGE ); } + (ReleaseChannel::Canary, passed_version) } else { let Ok(semver) = Version::parse_standard(&passed_version) else { bail!( - "Invalid version passed ({})\n\n{}", + "Invalid version passed ({})\n\nPass a semver, or a full 40 character git commit hash, or a release channel name.\n\nUsage:\n{}", colors::gray(passed_version), - EXAMPLE_USAGE + UPGRADE_USAGE ); }; @@ -1123,6 +1123,8 @@ mod test { use std::cell::RefCell; use std::rc::Rc; + use test_util::assert_contains; + use super::*; #[test] @@ -1221,6 +1223,27 @@ mod test { "5c69b4861b52ab406e73b9cd85c254f0505cb20f".to_string() ) ); + + upgrade_flags.version_or_hash_or_channel = + Some("5c69b4861b52a".to_string()); + let err = RequestedVersion::from_upgrade_flags(upgrade_flags.clone()) + .unwrap_err() + .to_string(); + assert_contains!(err, "Invalid version passed"); + assert_contains!( + err, + "Pass a semver, or a full 40 character git commit hash, or a release channel name." + ); + + upgrade_flags.version_or_hash_or_channel = Some("11.asd.1324".to_string()); + let err = RequestedVersion::from_upgrade_flags(upgrade_flags.clone()) + .unwrap_err() + .to_string(); + assert_contains!(err, "Invalid version passed"); + assert_contains!( + err, + "Pass a semver, or a full 40 character git commit hash, or a release channel name." + ); } #[test] diff --git a/tests/specs/upgrade/invalid_version/__test__.jsonc b/tests/specs/upgrade/invalid_version/__test__.jsonc new file mode 100644 index 0000000000..1fa81c1075 --- /dev/null +++ b/tests/specs/upgrade/invalid_version/__test__.jsonc @@ -0,0 +1,19 @@ +{ + "tests": { + "canary": { + "args": "upgrade --canary asdfasdf", + "output": "canary.out", + "exitCode": 1 + }, + "version": { + "args": "upgrade --version asdfasdf", + "output": "version.out", + "exitCode": 1 + }, + "shorthand": { + "args": "upgrade asdfasdf", + "output": "shorthand.out", + "exitCode": 1 + } + } +} diff --git a/tests/specs/upgrade/invalid_version/canary.out b/tests/specs/upgrade/invalid_version/canary.out new file mode 100644 index 0000000000..60a7b9f092 --- /dev/null +++ b/tests/specs/upgrade/invalid_version/canary.out @@ -0,0 +1,17 @@ +error: Invalid commit hash passed (asdfasdf) + +Pass a semver, or a full 40 character git commit hash, or a release channel name. + +Usage: +Latest + deno upgrade + +Specific version + deno upgrade 1.45.0 + deno upgrade 1.46.0-rc.1 + deno upgrade 9bc2dd29ad6ba334fd57a20114e367d3c04763d4 + +Channel + deno upgrade stable + deno upgrade rc + deno upgrade canary diff --git a/tests/specs/upgrade/invalid_version/shorthand.out b/tests/specs/upgrade/invalid_version/shorthand.out new file mode 100644 index 0000000000..134284cab9 --- /dev/null +++ b/tests/specs/upgrade/invalid_version/shorthand.out @@ -0,0 +1,17 @@ +error: Invalid version passed (asdfasdf) + +Pass a semver, or a full 40 character git commit hash, or a release channel name. + +Usage: +Latest + deno upgrade + +Specific version + deno upgrade 1.45.0 + deno upgrade 1.46.0-rc.1 + deno upgrade 9bc2dd29ad6ba334fd57a20114e367d3c04763d4 + +Channel + deno upgrade stable + deno upgrade rc + deno upgrade canary diff --git a/tests/specs/upgrade/invalid_version/version.out b/tests/specs/upgrade/invalid_version/version.out new file mode 100644 index 0000000000..134284cab9 --- /dev/null +++ b/tests/specs/upgrade/invalid_version/version.out @@ -0,0 +1,17 @@ +error: Invalid version passed (asdfasdf) + +Pass a semver, or a full 40 character git commit hash, or a release channel name. + +Usage: +Latest + deno upgrade + +Specific version + deno upgrade 1.45.0 + deno upgrade 1.46.0-rc.1 + deno upgrade 9bc2dd29ad6ba334fd57a20114e367d3c04763d4 + +Channel + deno upgrade stable + deno upgrade rc + deno upgrade canary