From 690b815ae0889db49df4f1bfa45d1c04956810c2 Mon Sep 17 00:00:00 2001 From: orangels Date: Wed, 6 May 2026 16:40:37 +0800 Subject: [PATCH] test: avoid global env mutation in config tests --- src/config.rs | 110 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 74 insertions(+), 36 deletions(-) diff --git a/src/config.rs b/src/config.rs index a230f3e..199fe47 100644 --- a/src/config.rs +++ b/src/config.rs @@ -82,13 +82,28 @@ impl AppConfig { path: path.to_path_buf(), source, })?; + Self::load_from_str_with_env(&contents, |name| std::env::var(name)).map_err(|error| { + match error { + ConfigError::Parse { source, .. } => ConfigError::Parse { + path: path.to_path_buf(), + source, + }, + error => error, + } + }) + } + + fn load_from_str_with_env(contents: &str, env: F) -> Result + where + F: Fn(&str) -> Result, + { let mut config: Self = - serde_yaml::from_str(&contents).map_err(|source| ConfigError::Parse { - path: path.to_path_buf(), + serde_yaml::from_str(contents).map_err(|source| ConfigError::Parse { + path: PathBuf::from(""), source, })?; - config.expand_env_refs()?; + config.expand_env_refs_with(env)?; Ok(config) } @@ -109,24 +124,27 @@ impl AppConfig { }) } - fn expand_env_refs(&mut self) -> Result<(), ConfigError> { + fn expand_env_refs_with(&mut self, env: F) -> Result<(), ConfigError> + where + F: Fn(&str) -> Result, + { if let Some(default_provider) = &mut self.default_provider { - *default_provider = expand_env_refs(default_provider)?; + *default_provider = expand_env_refs_with(default_provider, &env)?; } for provider in self.providers.values_mut() { - provider.base_url = expand_env_refs(&provider.base_url)?; - provider.api_token = expand_env_refs(&provider.api_token)?; - provider.default_model = expand_env_refs(&provider.default_model)?; + provider.base_url = expand_env_refs_with(&provider.base_url, &env)?; + provider.api_token = expand_env_refs_with(&provider.api_token, &env)?; + provider.default_model = expand_env_refs_with(&provider.default_model, &env)?; } - self.benchmarks.data_dir = expand_env_refs(&self.benchmarks.data_dir)?; + self.benchmarks.data_dir = expand_env_refs_with(&self.benchmarks.data_dir, &env)?; if let Some(dataset) = &mut self.benchmarks.aime2026 { - dataset.expand_env_refs()?; + dataset.expand_env_refs_with(&env)?; } if let Some(dataset) = &mut self.benchmarks.gpqa_diamond { - dataset.expand_env_refs()?; + dataset.expand_env_refs_with(&env)?; } Ok(()) @@ -134,9 +152,12 @@ impl AppConfig { } impl DatasetConfig { - fn expand_env_refs(&mut self) -> Result<(), ConfigError> { - self.source = expand_env_refs(&self.source)?; - self.split = expand_env_refs(&self.split)?; + fn expand_env_refs_with(&mut self, env: F) -> Result<(), ConfigError> + where + F: Fn(&str) -> Result, + { + self.source = expand_env_refs_with(&self.source, &env)?; + self.split = expand_env_refs_with(&self.split, &env)?; Ok(()) } } @@ -145,7 +166,10 @@ fn default_benchmark_data_dir() -> String { "data/benchmarks".to_string() } -fn expand_env_refs(value: &str) -> Result { +fn expand_env_refs_with(value: &str, env: F) -> Result +where + F: Fn(&str) -> Result, +{ let regex = Regex::new(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}").expect("valid env regex"); let mut expanded = String::with_capacity(value.len()); let mut last_match = 0; @@ -153,7 +177,7 @@ fn expand_env_refs(value: &str) -> Result { for captures in regex.captures_iter(value) { let full_match = captures.get(0).expect("full match exists"); let env_name = captures.get(1).expect("env name exists").as_str(); - let env_value = std::env::var(env_name).map_err(|_| ConfigError::MissingEnv { + let env_value = env(env_name).map_err(|_| ConfigError::MissingEnv { name: env_name.to_string(), })?; @@ -172,14 +196,7 @@ mod tests { #[test] fn loads_provider_and_expands_env_token() { - unsafe { - std::env::set_var("LQ_TEST_TOKEN", "secret-token"); - } - - let temp_dir = tempfile::tempdir().expect("create temp dir"); - let config_path = temp_dir.path().join("config.yaml"); - std::fs::write( - &config_path, + let config = AppConfig::load_from_str_with_env( r#" default_provider: openai providers: @@ -191,10 +208,13 @@ providers: benchmarks: data_dir: data/benchmarks "#, + |name| match name { + "LQ_TEST_TOKEN" => Ok("secret-token".to_string()), + _ => Err(()), + }, ) - .expect("write config"); + .expect("load config"); - let config = AppConfig::load(&config_path).expect("load config"); let provider = config.provider(None).expect("default provider"); assert_eq!(provider.api_token, "secret-token"); @@ -204,14 +224,7 @@ benchmarks: #[test] fn rejects_missing_env_token() { - unsafe { - std::env::remove_var("LQ_MISSING_TOKEN"); - } - - let temp_dir = tempfile::tempdir().expect("create temp dir"); - let config_path = temp_dir.path().join("config.yaml"); - std::fs::write( - &config_path, + let error = AppConfig::load_from_str_with_env( r#" default_provider: openai providers: @@ -222,12 +235,37 @@ providers: default_model: gpt-test benchmarks: data_dir: data/benchmarks +"#, + |_| Err(()), + ) + .expect_err("missing env should fail"); + + assert!(error.to_string().contains("LQ_MISSING_TOKEN")); + } + + #[test] + fn load_reads_yaml_from_path_without_env_refs() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let config_path = temp_dir.path().join("config.yaml"); + std::fs::write( + &config_path, + r#" +default_provider: anthropic +providers: + anthropic: + protocol: anthropic + base_url: https://api.anthropic.test + api_token: literal-token + default_model: claude-test "#, ) .expect("write config"); - let error = AppConfig::load(&config_path).expect_err("missing env should fail"); + let config = AppConfig::load(&config_path).expect("load config"); + let provider = config.provider(None).expect("default provider"); - assert!(error.to_string().contains("LQ_MISSING_TOKEN")); + assert_eq!(provider.api_token, "literal-token"); + assert_eq!(provider.protocol, ProtocolKind::Anthropic); + assert_eq!(config.benchmarks.data_dir, "data/benchmarks"); } }