diff --git a/CLAUDE.md b/CLAUDE.md index 50a2c4ad0f..3c17fe0e26 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -106,6 +106,12 @@ All user-facing output must go through shared output modules instead of raw prin - Run `cargo test` to execute all tests - You never need to run `pnpm install` in the test fixtures dir, vite-plus should able to load and parse the workspace without `pnpm install`. +### Environment Variables in Tests + +- **Prefer `EnvConfig::test_scope()`**: For tests needing custom config values (VP_HOME, npm registry, etc.), use thread-local `EnvConfig::test_scope()` or `EnvConfig::test_guard()` from `vite_shared` — no `unsafe`, no `#[serial]`, full parallelism +- **`#[serial]` required for `std::env::set_var`/`remove_var`**: Any test that directly modifies process env vars (PATH, VP_SHIM_TOOL, etc.) MUST have `#[serial_test::serial]` to prevent concurrent access races +- **Clean up ALL related env vars**: When clearing env vars before a test, clear ALL vars that the function under test reads — not just the one being tested + ## Build - Run `pnpm bootstrap-cli` from the project root to build all packages and install the global CLI diff --git a/Cargo.lock b/Cargo.lock index 3cbddd5ac0..65f6dd67a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7419,6 +7419,7 @@ dependencies = [ "rustls", "serde", "serde_json", + "serial_test", "supports-color 3.0.2", "tracing-subscriber", "vite_path", diff --git a/crates/vite_global_cli/src/shim/mod.rs b/crates/vite_global_cli/src/shim/mod.rs index 90e12466b5..e93d730847 100644 --- a/crates/vite_global_cli/src/shim/mod.rs +++ b/crates/vite_global_cli/src/shim/mod.rs @@ -217,10 +217,18 @@ mod tests { assert!(!is_potential_package_binary("another-fake-tool")); } + /// Clear both shim env vars to isolate tests. + /// SAFETY: caller must be `#[serial]` since this mutates process-global state. + unsafe fn clear_shim_env_vars() { + unsafe { + std::env::remove_var(SHIM_TOOL_ENV_VAR); + std::env::remove_var(LEGACY_SHIM_TOOL_ENV_VAR); + } + } + #[test] #[serial] fn test_detect_shim_tool_from_env_var() { - // SAFETY: We're in a test at startup, no other threads unsafe { std::env::set_var(SHIM_TOOL_ENV_VAR, "node"); std::env::remove_var(LEGACY_SHIM_TOOL_ENV_VAR); @@ -236,7 +244,6 @@ mod tests { fn test_detect_shim_tool_from_legacy_env_var() { // When only VITE_PLUS_SHIM_TOOL is set (older trampoline), it should // fall back to reading the legacy env var. - // SAFETY: We're in a test at startup, no other threads unsafe { std::env::remove_var(SHIM_TOOL_ENV_VAR); std::env::set_var(LEGACY_SHIM_TOOL_ENV_VAR, "npm"); @@ -247,43 +254,28 @@ mod tests { assert!(std::env::var(LEGACY_SHIM_TOOL_ENV_VAR).is_err()); } + /// Tests that argv0-based tool detection works for a given tool name, + /// including full path and .exe extension variants. + fn assert_detect_shim_tool_from_argv0(tool: &str) { + unsafe { clear_shim_env_vars() }; + + assert_eq!(detect_shim_tool(tool), Some(tool.to_string())); + assert_eq!( + detect_shim_tool(&vite_str::format!("/home/user/.vite-plus/bin/{tool}")), + Some(tool.to_string()), + ); + assert_eq!(detect_shim_tool(&vite_str::format!("{tool}.exe")), Some(tool.to_string()),); + } + #[test] #[serial] fn test_detect_shim_tool_vpx() { - // vpx should be detected via the argv0 check, before the env var check - // and before is_shim_tool (which would incorrectly match it as a package binary) - // SAFETY: We're in a test - unsafe { - std::env::remove_var(SHIM_TOOL_ENV_VAR); - } - let result = detect_shim_tool("vpx"); - assert_eq!(result, Some("vpx".to_string())); - - // Also works with full path - let result = detect_shim_tool("/home/user/.vite-plus/bin/vpx"); - assert_eq!(result, Some("vpx".to_string())); - - // Also works with .exe extension (Windows) - let result = detect_shim_tool("vpx.exe"); - assert_eq!(result, Some("vpx".to_string())); + assert_detect_shim_tool_from_argv0("vpx"); } #[test] + #[serial] fn test_detect_shim_tool_vpr() { - // vpr should be detected via the argv0 check, same pattern as vpx - // SAFETY: We're in a test - unsafe { - std::env::remove_var(SHIM_TOOL_ENV_VAR); - } - let result = detect_shim_tool("vpr"); - assert_eq!(result, Some("vpr".to_string())); - - // Also works with full path - let result = detect_shim_tool("/home/user/.vite-plus/bin/vpr"); - assert_eq!(result, Some("vpr".to_string())); - - // Also works with .exe extension (Windows) - let result = detect_shim_tool("vpr.exe"); - assert_eq!(result, Some("vpr".to_string())); + assert_detect_shim_tool_from_argv0("vpr"); } } diff --git a/crates/vite_shared/Cargo.toml b/crates/vite_shared/Cargo.toml index 4614724285..ff814d571b 100644 --- a/crates/vite_shared/Cargo.toml +++ b/crates/vite_shared/Cargo.toml @@ -22,5 +22,8 @@ which = { workspace = true } [target.'cfg(not(target_os = "windows"))'.dependencies] rustls = { workspace = true } +[dev-dependencies] +serial_test = { workspace = true } + [lints] workspace = true diff --git a/crates/vite_shared/src/home.rs b/crates/vite_shared/src/home.rs index 6b705180ad..65070b607c 100644 --- a/crates/vite_shared/src/home.rs +++ b/crates/vite_shared/src/home.rs @@ -64,6 +64,7 @@ mod tests { } #[test] + #[serial_test::serial] fn test_get_vite_plus_without_home() { use std::path::PathBuf; diff --git a/crates/vite_shared/src/path_env.rs b/crates/vite_shared/src/path_env.rs index 3cd905b778..f57d25db51 100644 --- a/crates/vite_shared/src/path_env.rs +++ b/crates/vite_shared/src/path_env.rs @@ -142,7 +142,7 @@ mod tests { } #[test] - #[ignore] + #[serial_test::serial] fn test_format_path_prepended_always_prepends() { // Even if the directory exists somewhere in PATH, it should be prepended let test_dir = "/test/node/bin";