fix(test): add #[serial] to env-var tests to prevent concurrent access races (#1223)

Tests that manipulate process env vars (VP_SHIM_TOOL, VITE_PLUS_SHIM_TOOL,
PATH) without #[serial] can race with other tests running in parallel,
causing flaky CI failures. Ensure all such tests are serialized and clear
all related env vars before running.

Also document env var testing guidelines in CLAUDE.md.

Author: fengmk2

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

Cargo.lock

@@ -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");
     }
 }

crates/vite_global_cli/src/shim/mod.rs

@@ -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

crates/vite_shared/Cargo.toml

@@ -64,6 +64,7 @@ mod tests {
     }
 
     #[test]
+    #[serial_test::serial]
     fn test_get_vite_plus_without_home() {
         use std::path::PathBuf;
 

crates/vite_shared/src/home.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";