From 687935d7461f97f8d6cf53b62e2b9c718c23d413 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 18:06:44 +0000 Subject: [PATCH] test: remove tautological and duplicate Rust unit tests - Drop the `assert!(true)` placeholder test in `vp create`. - Drop four `detect_shell` tests in `env use` that duplicate the `commands::shell` tests of the same function. - Drop tests of test-only helpers (`resolution::test_utils` and the `list_args` helper); every resolution parser test already exercises them. - Fold the unrolled `is_newer_version` and `should_run_for_command` cases in `upgrade_check` into table-driven tests, keeping every row and comment. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf --- crates/vp_global_cli/src/commands/create.rs | 9 -- crates/vp_global_cli/src/commands/env/use.rs | 54 +------ crates/vp_global_cli/src/upgrade_check.rs | 140 +++++------------- .../vp_pm_cli/src/resolution/commands/list.rs | 10 -- crates/vp_pm_cli/src/resolution/test_utils.rs | 30 ---- 5 files changed, 42 insertions(+), 201 deletions(-) diff --git a/crates/vp_global_cli/src/commands/create.rs b/crates/vp_global_cli/src/commands/create.rs index a92751546e..f94f841775 100644 --- a/crates/vp_global_cli/src/commands/create.rs +++ b/crates/vp_global_cli/src/commands/create.rs @@ -14,12 +14,3 @@ pub async fn execute( ) -> Result { super::delegate::execute(cwd, "create", args, raw_subcommand).await } - -#[cfg(test)] -mod tests { - #[test] - fn test_create_command_module_exists() { - // Basic test to ensure the module compiles - assert!(true); - } -} diff --git a/crates/vp_global_cli/src/commands/env/use.rs b/crates/vp_global_cli/src/commands/env/use.rs index 7f3d31e381..d183a46054 100644 --- a/crates/vp_global_cli/src/commands/env/use.rs +++ b/crates/vp_global_cli/src/commands/env/use.rs @@ -333,24 +333,11 @@ fn current_override(session: Option, environment: Option) -> Opt #[cfg(test)] mod tests { + #[cfg(windows)] use vp_shared::env_vars; use super::*; - #[test] - fn test_detect_shell_vp_shell_powershell() { - vp_shared::EnvConfig::with_vars( - [ - (env_vars::VP_HOME, std::env::temp_dir().as_os_str()), - (env_vars::VP_SHELL, std::ffi::OsStr::new("powershell")), - ], - |_| { - let shell = detect_shell(); - assert_eq!(shell, Shell::PowerShell); - }, - ); - } - #[test] fn empty_environment_override_falls_back_to_session() { assert_eq!( @@ -359,45 +346,6 @@ mod tests { ); } - #[test] - fn test_detect_shell_vp_shell_fish() { - vp_shared::EnvConfig::with_vars( - [ - (env_vars::VP_HOME, std::env::temp_dir().as_os_str()), - (env_vars::VP_SHELL, std::ffi::OsStr::new("fish")), - ], - |_| { - let shell = detect_shell(); - assert_eq!(shell, Shell::Fish); - }, - ); - } - - #[test] - fn test_detect_shell_vp_shell_nu() { - vp_shared::EnvConfig::with_vars( - [ - (env_vars::VP_HOME, std::env::temp_dir().as_os_str()), - (env_vars::VP_SHELL, std::ffi::OsStr::new("nu")), - ], - |_| { - let shell = detect_shell(); - assert_eq!(shell, Shell::NuShell); - }, - ); - } - - #[test] - fn test_detect_shell_posix_default() { - vp_shared::EnvConfig::with_vars([(env_vars::VP_HOME, std::env::temp_dir())], |_| { - let shell = detect_shell(); - #[cfg(not(windows))] - assert_eq!(shell, Shell::Posix); - #[cfg(windows)] - assert_eq!(shell, Shell::Cmd); - }); - } - #[test] fn test_format_export_posix() { let result = format_export(&Shell::Posix, VERSION_ENV_VAR, "20.18.0"); diff --git a/crates/vp_global_cli/src/upgrade_check.rs b/crates/vp_global_cli/src/upgrade_check.rs index fd289b662b..164c26cd04 100644 --- a/crates/vp_global_cli/src/upgrade_check.rs +++ b/crates/vp_global_cli/src/upgrade_check.rs @@ -588,53 +588,26 @@ mod tests { } #[test] - fn is_newer_version_detects_upgrade() { - assert!(is_newer_version("0.1.0", "0.2.0")); - assert!(is_newer_version("0.1.0", "1.0.0")); - assert!(is_newer_version("1.0.0", "1.0.1")); - } - - #[test] - fn is_newer_version_rejects_same() { - assert!(!is_newer_version("0.2.0", "0.2.0")); - } - - #[test] - fn is_newer_version_rejects_downgrade() { - assert!(!is_newer_version("0.2.0", "0.1.0")); - } - - #[test] - fn is_newer_version_rejects_prerelease_downgrade_to_stable() { - // User on alpha, latest stable is older — don't prompt - assert!(!is_newer_version("0.3.0-alpha.1", "0.2.0")); - } - - #[test] - fn is_newer_version_prompts_prerelease_to_newer_stable() { - assert!(is_newer_version("0.1.0-alpha.1", "0.2.0")); - } - - #[test] - fn is_newer_version_prompts_prerelease_to_same_base_release() { - // 1.0.0 is newer than 1.0.0-alpha.1 per semver - assert!(is_newer_version("1.0.0-alpha.1", "1.0.0")); - } - - #[test] - fn is_newer_version_rejects_empty_latest() { - assert!(!is_newer_version("0.1.0", "")); - } - - #[test] - fn is_newer_version_skips_dev_build() { - assert!(!is_newer_version("0.0.0", "0.2.0")); - } - - #[test] - fn is_newer_version_rejects_invalid_versions() { - assert!(!is_newer_version("not-a-version", "0.2.0")); - assert!(!is_newer_version("0.1.0", "not-a-version")); + fn is_newer_version_compares_semver() { + for (current, latest, expected) in [ + ("0.1.0", "0.2.0", true), + ("0.1.0", "1.0.0", true), + ("1.0.0", "1.0.1", true), + ("0.2.0", "0.2.0", false), + ("0.2.0", "0.1.0", false), + // User on alpha, latest stable is older — don't prompt + ("0.3.0-alpha.1", "0.2.0", false), + ("0.1.0-alpha.1", "0.2.0", true), + // 1.0.0 is newer than 1.0.0-alpha.1 per semver + ("1.0.0-alpha.1", "1.0.0", true), + ("0.1.0", "", false), + // Dev builds (0.0.0) are never prompted + ("0.0.0", "0.2.0", false), + ("not-a-version", "0.2.0", false), + ("0.1.0", "not-a-version", false), + ] { + assert_eq!(is_newer_version(current, latest), expected, "{current} -> {latest}"); + } } fn parse_args(args: &[&str]) -> crate::cli::Args { @@ -644,58 +617,27 @@ mod tests { } #[test] - fn should_run_for_normal_command() { - assert!(should_run_for_command(&parse_args(&["build"]))); - } - - #[test] - fn should_not_run_for_upgrade() { - assert!(!should_run_for_command(&parse_args(&["upgrade"]))); - } - - #[test] - fn should_not_run_for_install_silent() { - assert!(!should_run_for_command(&parse_args(&["install", "--silent"]))); - } - - #[test] - fn should_not_run_for_dlx_short_silent() { - assert!(!should_run_for_command(&parse_args(&["dlx", "-s", "pkg"]))); - } - - #[test] - fn should_not_run_for_why_json() { - assert!(!should_run_for_command(&parse_args(&["why", "lodash", "--json"]))); - } - - #[test] - fn should_not_run_for_why_parseable() { - assert!(!should_run_for_command(&parse_args(&["why", "lodash", "--parseable"]))); - } - - #[test] - fn should_not_run_for_outdated_format_json() { - assert!(!should_run_for_command(&parse_args(&["outdated", "--format", "json"]))); - } - - #[test] - fn should_not_run_for_pm_list_parseable() { - assert!(!should_run_for_command(&parse_args(&["pm", "list", "--parseable"]))); - } - - #[test] - fn should_not_run_for_pm_list_json() { - assert!(!should_run_for_command(&parse_args(&["pm", "list", "--json"]))); - } - - #[test] - fn should_not_run_for_env_current_json() { - assert!(!should_run_for_command(&parse_args(&["env", "current", "--json"]))); - } - - #[test] - fn should_run_for_outdated_without_format() { - assert!(should_run_for_command(&parse_args(&["outdated"]))); + fn should_run_for_command_skips_excluded_and_machine_readable_commands() { + for (args, expected) in [ + (&["build"][..], true), + (&["upgrade"], false), + (&["install", "--silent"], false), + (&["dlx", "-s", "pkg"], false), + (&["why", "lodash", "--json"], false), + (&["why", "lodash", "--parseable"], false), + (&["outdated", "--format", "json"], false), + (&["pm", "list", "--parseable"], false), + (&["pm", "list", "--json"], false), + (&["env", "current", "--json"], false), + (&["outdated"], true), + ] { + assert_eq!( + should_run_for_command(&parse_args(args)), + expected, + "vp {}", + args.join(" ") + ); + } } #[tokio::test(flavor = "current_thread")] diff --git a/crates/vp_pm_cli/src/resolution/commands/list.rs b/crates/vp_pm_cli/src/resolution/commands/list.rs index b508951a0a..9bf6ddfe15 100644 --- a/crates/vp_pm_cli/src/resolution/commands/list.rs +++ b/crates/vp_pm_cli/src/resolution/commands/list.rs @@ -181,10 +181,6 @@ mod tests { test_utils::{bun, expect_run, expect_unsupported, npm, parse_args, pnpm, yarn}, }; - fn list_args(pattern: Option<&str>) -> ListArgs { - ListArgs { pattern: pattern.map(ToString::to_string), ..Default::default() } - } - #[test] fn test_parser_accepts_short_dev_and_global_flags() { let args = parse_args::(["react", "-D", "-g"]).unwrap(); @@ -665,10 +661,4 @@ mod tests { assert_eq!(command.program, "pnpm"); assert_eq!(command.args, vec!["list", "--json", "--long", "--parseable"]); } - - #[test] - fn list_args_helper_keeps_pattern() { - assert_eq!(list_args(Some("react")).pattern, Some("react".to_string())); - assert_eq!(list_args(None).pattern, None); - } } diff --git a/crates/vp_pm_cli/src/resolution/test_utils.rs b/crates/vp_pm_cli/src/resolution/test_utils.rs index 6ef96f033d..7051971025 100644 --- a/crates/vp_pm_cli/src/resolution/test_utils.rs +++ b/crates/vp_pm_cli/src/resolution/test_utils.rs @@ -68,33 +68,3 @@ fn test_argv( ) -> impl Iterator { std::iter::once(OsString::from("test")).chain(args.into_iter().map(Into::into)) } - -#[cfg(test)] -mod tests { - use super::*; - - #[derive(clap::Args, Debug, PartialEq, Eq)] - struct TestArgs { - #[arg(long)] - value: String, - } - - #[derive(clap::Subcommand, Debug, PartialEq, Eq)] - enum TestSubcommand { - Get { key: String }, - } - - #[test] - fn parses_args_without_a_program_name() { - let args = parse_args::(["--value", "hello"]).unwrap(); - - assert_eq!(args, TestArgs { value: "hello".to_string() }); - } - - #[test] - fn parses_subcommands_without_a_program_name() { - let args = parse_subcommand::(["get", "registry"]).unwrap(); - - assert_eq!(args, TestSubcommand::Get { key: "registry".to_string() }); - } -}