Skip to content

Commit a015465

Browse files
committed
fix(test): add #[serial] to env-var tests to prevent concurrent access races
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.
1 parent 3fc8005 commit a015465

6 files changed

Lines changed: 15 additions & 0 deletions

File tree

CLAUDE.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,12 @@ All user-facing output must go through shared output modules instead of raw prin
106106
- Run `cargo test` to execute all tests
107107
- 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`.
108108

109+
### Environment Variables in Tests
110+
111+
- **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
112+
- **`#[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
113+
- **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
114+
109115
## Build
110116

111117
- Run `pnpm bootstrap-cli` from the project root to build all packages and install the global CLI

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/vite_global_cli/src/shim/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ mod tests {
255255
// SAFETY: We're in a test
256256
unsafe {
257257
std::env::remove_var(SHIM_TOOL_ENV_VAR);
258+
std::env::remove_var(LEGACY_SHIM_TOOL_ENV_VAR);
258259
}
259260
let result = detect_shim_tool("vpx");
260261
assert_eq!(result, Some("vpx".to_string()));
@@ -269,11 +270,13 @@ mod tests {
269270
}
270271

271272
#[test]
273+
#[serial]
272274
fn test_detect_shim_tool_vpr() {
273275
// vpr should be detected via the argv0 check, same pattern as vpx
274276
// SAFETY: We're in a test
275277
unsafe {
276278
std::env::remove_var(SHIM_TOOL_ENV_VAR);
279+
std::env::remove_var(LEGACY_SHIM_TOOL_ENV_VAR);
277280
}
278281
let result = detect_shim_tool("vpr");
279282
assert_eq!(result, Some("vpr".to_string()));

crates/vite_shared/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,8 @@ which = { workspace = true }
2222
[target.'cfg(not(target_os = "windows"))'.dependencies]
2323
rustls = { workspace = true }
2424

25+
[dev-dependencies]
26+
serial_test = { workspace = true }
27+
2528
[lints]
2629
workspace = true

crates/vite_shared/src/home.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ mod tests {
6464
}
6565

6666
#[test]
67+
#[serial_test::serial]
6768
fn test_get_vite_plus_without_home() {
6869
use std::path::PathBuf;
6970

crates/vite_shared/src/path_env.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ mod tests {
143143

144144
#[test]
145145
#[ignore]
146+
#[serial_test::serial]
146147
fn test_format_path_prepended_always_prepends() {
147148
// Even if the directory exists somewhere in PATH, it should be prepended
148149
let test_dir = "/test/node/bin";

0 commit comments

Comments
 (0)