From b0f25e13193876ae4ddb8a455ae1efeaa4758fed Mon Sep 17 00:00:00 2001 From: AdAstraAbyssoque Date: Thu, 29 Jan 2026 16:17:20 +0800 Subject: [PATCH 1/2] fix: exercise color helper tests Ensure color tests call the real formatting helpers so they actually validate the NO_COLOR behavior instead of only checking format! output. --- cli/src/color.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/cli/src/color.rs b/cli/src/color.rs index 07eb526b..7668380b 100644 --- a/cli/src/color.rs +++ b/cli/src/color.rs @@ -127,16 +127,30 @@ mod tests { #[test] fn test_red_contains_ansi_codes() { - // Test the format structure (actual color depends on NO_COLOR env) - let formatted = format!("\x1b[31m{}\x1b[0m", "error"); - assert!(formatted.contains("\x1b[31m")); - assert!(formatted.contains("\x1b[0m")); + let text = "error"; + let formatted = red(text); + + if is_enabled() { + assert!(formatted.contains("\x1b[31m")); + assert!(formatted.contains(text)); + assert!(formatted.contains("\x1b[0m")); + } else { + assert_eq!(formatted, text); + } } #[test] fn test_green_contains_ansi_codes() { - let formatted = format!("\x1b[32m{}\x1b[0m", "success"); - assert!(formatted.contains("\x1b[32m")); + let text = "success"; + let formatted = green(text); + + if is_enabled() { + assert!(formatted.contains("\x1b[32m")); + assert!(formatted.contains(text)); + assert!(formatted.contains("\x1b[0m")); + } else { + assert_eq!(formatted, text); + } } #[test] From 5532380886ef89baa66987f5b17a1160379451c0 Mon Sep 17 00:00:00 2001 From: AdAstraAbyssoque Date: Thu, 29 Jan 2026 16:36:13 +0800 Subject: [PATCH 2/2] fix: make color tests deterministic Avoid NO_COLOR caching during tests and add env-guarded coverage for both colored and non-colored paths. --- cli/src/color.rs | 80 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 66 insertions(+), 14 deletions(-) diff --git a/cli/src/color.rs b/cli/src/color.rs index 7668380b..8b11f494 100644 --- a/cli/src/color.rs +++ b/cli/src/color.rs @@ -9,6 +9,10 @@ use std::sync::OnceLock; /// Returns true if color output is enabled (NO_COLOR is NOT set) pub fn is_enabled() -> bool { static COLORS_ENABLED: OnceLock = OnceLock::new(); + if cfg!(test) { + // Avoid caching in tests so NO_COLOR can be toggled deterministically. + return env::var("NO_COLOR").is_err(); + } *COLORS_ENABLED.get_or_init(|| env::var("NO_COLOR").is_err()) } @@ -124,33 +128,81 @@ pub fn console_level_prefix(level: &str) -> String { #[cfg(test)] mod tests { use super::*; + use std::sync::{Mutex, MutexGuard}; + + // Mutex to prevent parallel tests from interfering with env vars + static ENV_MUTEX: Mutex<()> = Mutex::new(()); + + /// RAII guard that locks env mutex and restores env vars on drop + struct EnvGuard<'a> { + _lock: MutexGuard<'a, ()>, + vars: Vec<(String, Option)>, + } + + impl<'a> EnvGuard<'a> { + fn new(var_names: &[&str]) -> Self { + let lock = ENV_MUTEX.lock().unwrap(); + let vars = var_names + .iter() + .map(|&name| (name.to_string(), env::var(name).ok())) + .collect(); + Self { _lock: lock, vars } + } + } + + impl Drop for EnvGuard<'_> { + fn drop(&mut self) { + for (name, value) in &self.vars { + match value { + Some(v) => env::set_var(name, v), + None => env::remove_var(name), + } + } + } + } #[test] fn test_red_contains_ansi_codes() { + let _guard = EnvGuard::new(&["NO_COLOR"]); let text = "error"; + env::remove_var("NO_COLOR"); let formatted = red(text); - if is_enabled() { - assert!(formatted.contains("\x1b[31m")); - assert!(formatted.contains(text)); - assert!(formatted.contains("\x1b[0m")); - } else { - assert_eq!(formatted, text); - } + assert!(formatted.contains("\x1b[31m")); + assert!(formatted.contains(text)); + assert!(formatted.contains("\x1b[0m")); + } + + #[test] + fn test_red_no_color() { + let _guard = EnvGuard::new(&["NO_COLOR"]); + let text = "error"; + env::set_var("NO_COLOR", "1"); + let formatted = red(text); + + assert_eq!(formatted, text); } #[test] fn test_green_contains_ansi_codes() { + let _guard = EnvGuard::new(&["NO_COLOR"]); let text = "success"; + env::remove_var("NO_COLOR"); let formatted = green(text); - if is_enabled() { - assert!(formatted.contains("\x1b[32m")); - assert!(formatted.contains(text)); - assert!(formatted.contains("\x1b[0m")); - } else { - assert_eq!(formatted, text); - } + assert!(formatted.contains("\x1b[32m")); + assert!(formatted.contains(text)); + assert!(formatted.contains("\x1b[0m")); + } + + #[test] + fn test_green_no_color() { + let _guard = EnvGuard::new(&["NO_COLOR"]); + let text = "success"; + env::set_var("NO_COLOR", "1"); + let formatted = green(text); + + assert_eq!(formatted, text); } #[test]