Skip to content

Commit 6dbc4b7

Browse files
committed
fix(validate): Fix test race condition
Extract pure parsing functions from read_parallel_options_from_env to enable testing without environment variable manipulation. Tests that modified env vars were racing with each other when run in parallel, causing intermittent CI failures.
1 parent c861a8d commit 6dbc4b7

File tree

1 file changed

+35
-41
lines changed

1 file changed

+35
-41
lines changed

src/commands/validate/analysis.rs

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,32 @@ pub struct ValidationAnalysisOptions {
2424
pub disable_context: Option<Vec<String>>,
2525
}
2626

27+
/// Parse a parallel flag value.
28+
///
29+
/// Returns `true` if the value is "true" or "1".
30+
fn parse_parallel_flag(value: &str) -> bool {
31+
value == "true" || value == "1"
32+
}
33+
34+
/// Parse a jobs value.
35+
///
36+
/// Returns the parsed number, or 0 if invalid.
37+
fn parse_jobs_value(value: &str) -> usize {
38+
value.parse::<usize>().unwrap_or(0)
39+
}
40+
2741
/// Read parallel processing settings from environment.
2842
///
2943
/// This reads DEBTMAP_PARALLEL and DEBTMAP_JOBS environment variables
3044
/// to determine parallel processing configuration.
3145
pub fn read_parallel_options_from_env() -> ValidationAnalysisOptions {
3246
let parallel = std::env::var("DEBTMAP_PARALLEL")
33-
.map(|v| v == "true" || v == "1")
47+
.map(|v| parse_parallel_flag(&v))
3448
.unwrap_or(false);
3549

3650
let jobs = std::env::var("DEBTMAP_JOBS")
3751
.ok()
38-
.and_then(|v| v.parse::<usize>().ok())
52+
.map(|v| parse_jobs_value(&v))
3953
.unwrap_or(0);
4054

4155
ValidationAnalysisOptions {
@@ -97,54 +111,34 @@ mod tests {
97111
}
98112

99113
#[test]
100-
fn test_read_parallel_options_not_set() {
101-
// Clear env vars
102-
std::env::remove_var("DEBTMAP_PARALLEL");
103-
std::env::remove_var("DEBTMAP_JOBS");
104-
105-
let options = read_parallel_options_from_env();
106-
assert!(!options.parallel);
107-
assert_eq!(options.jobs, 0);
114+
fn test_parse_parallel_flag_true() {
115+
assert!(parse_parallel_flag("true"));
108116
}
109117

110118
#[test]
111-
fn test_read_parallel_options_enabled() {
112-
// Test the parsing logic directly to avoid env var race conditions
113-
// between parallel tests. The actual env var reading is tested
114-
// implicitly by the integration tests.
115-
let parallel_from_true = "true" == "true" || "true" == "1";
116-
let parallel_from_1 = "1" == "true" || "1" == "1";
117-
let jobs_from_valid: Option<usize> = "4".parse().ok();
118-
119-
assert!(parallel_from_true);
120-
assert!(parallel_from_1);
121-
assert_eq!(jobs_from_valid, Some(4));
119+
fn test_parse_parallel_flag_1() {
120+
assert!(parse_parallel_flag("1"));
122121
}
123122

124123
#[test]
125-
fn test_read_parallel_options_with_1() {
126-
std::env::set_var("DEBTMAP_PARALLEL", "1");
127-
std::env::remove_var("DEBTMAP_JOBS");
128-
129-
let options = read_parallel_options_from_env();
130-
assert!(options.parallel);
131-
assert_eq!(options.jobs, 0);
132-
133-
// Cleanup
134-
std::env::remove_var("DEBTMAP_PARALLEL");
124+
fn test_parse_parallel_flag_false() {
125+
assert!(!parse_parallel_flag("false"));
126+
assert!(!parse_parallel_flag("0"));
127+
assert!(!parse_parallel_flag(""));
128+
assert!(!parse_parallel_flag("yes"));
135129
}
136130

137131
#[test]
138-
fn test_read_parallel_options_invalid_jobs() {
139-
std::env::set_var("DEBTMAP_PARALLEL", "true");
140-
std::env::set_var("DEBTMAP_JOBS", "invalid");
141-
142-
let options = read_parallel_options_from_env();
143-
assert!(options.parallel);
144-
assert_eq!(options.jobs, 0); // Falls back to 0
132+
fn test_parse_jobs_value_valid() {
133+
assert_eq!(parse_jobs_value("4"), 4);
134+
assert_eq!(parse_jobs_value("1"), 1);
135+
assert_eq!(parse_jobs_value("16"), 16);
136+
}
145137

146-
// Cleanup
147-
std::env::remove_var("DEBTMAP_PARALLEL");
148-
std::env::remove_var("DEBTMAP_JOBS");
138+
#[test]
139+
fn test_parse_jobs_value_invalid() {
140+
assert_eq!(parse_jobs_value("invalid"), 0);
141+
assert_eq!(parse_jobs_value(""), 0);
142+
assert_eq!(parse_jobs_value("-1"), 0);
149143
}
150144
}

0 commit comments

Comments
 (0)