From ad41b4f81981488ea865c03865b53dec40ef5bcf Mon Sep 17 00:00:00 2001 From: Jack Amadeo Date: Fri, 26 Sep 2025 16:21:31 -0400 Subject: [PATCH] fix: delete some flaky and not-useful tests --- crates/goose-cli/src/logging.rs | 261 -------------------------------- 1 file changed, 261 deletions(-) diff --git a/crates/goose-cli/src/logging.rs b/crates/goose-cli/src/logging.rs index 265a84f61274..1f9928922b02 100644 --- a/crates/goose-cli/src/logging.rs +++ b/crates/goose-cli/src/logging.rs @@ -22,35 +22,6 @@ fn get_log_directory() -> Result { goose::logging::get_log_directory("cli", true) } -/// Internal function that allows specifying a custom date string for testing -#[cfg(test)] -fn get_log_directory_with_date(test_date: Option) -> Result { - use etcetera::{choose_app_strategy, AppStrategy}; - use goose::config::APP_STRATEGY; - - let home_dir = - choose_app_strategy(APP_STRATEGY.clone()).context("HOME environment variable not set")?; - - let base_log_dir = home_dir - .in_state_dir("logs") - .unwrap_or_else(|| home_dir.in_data_dir("logs")); - - let component_dir = base_log_dir.join("cli"); - - let log_dir = if let Some(date_str) = test_date { - component_dir.join(date_str) - } else { - // Create date-based subdirectory - let now = chrono::Local::now(); - component_dir.join(now.format("%Y-%m-%d").to_string()) - }; - - // Ensure log directory exists - std::fs::create_dir_all(&log_dir).context("Failed to create log directory")?; - - Ok(log_dir) -} - /// Sets up the logging infrastructure for the application. /// This includes: /// - File-based logging with JSON formatting (DEBUG level) @@ -193,8 +164,6 @@ fn setup_logging_internal( #[cfg(test)] mod tests { use super::*; - use chrono::TimeZone; - use rand; use std::env; use tempfile::TempDir; @@ -222,236 +191,6 @@ mod tests { assert!(path_components.iter().any(|c| c.as_os_str() == "cli")); } - #[tokio::test] - async fn test_log_file_name_session_with_error_capture() { - do_test_log_file_name(Some("test_session_with_error"), true).await; - } - - #[tokio::test] - async fn test_log_file_name_session_without_error_capture() { - do_test_log_file_name(Some("test_session_without_error"), false).await; - } - - #[tokio::test] - async fn test_log_file_name_no_session() { - do_test_log_file_name(None, false).await; - } - - async fn do_test_log_file_name(session_name: Option<&str>, _with_error_capture: bool) { - use tempfile::TempDir; - - // Create a unique prefix to avoid test interference - let test_id = format!( - "{}_{}", - session_name.unwrap_or("no_session"), - rand::random::() - ); - - // Create a proper temporary directory that will be automatically cleaned up - let _temp_dir = TempDir::with_prefix(format!("goose_test_{}_", test_id)).unwrap(); - let test_dir = _temp_dir.path(); - - // Set up environment - if cfg!(windows) { - env::set_var("USERPROFILE", test_dir); - } else { - env::set_var("HOME", test_dir); - // Also set TMPDIR to prevent temp directory sharing between tests - env::set_var("TMPDIR", test_dir); - } - - // Create error capture if needed - but don't use it in tests to avoid tokio runtime issues - let error_capture = None; - - // Get current timestamp before setting up logging - let before_timestamp = chrono::Local::now() - chrono::Duration::seconds(1); - println!("Before timestamp: {}", before_timestamp); - - // Get the log directory and clean any existing log files - let random_suffix = rand::random::() % 100000000; - let log_dir = get_log_directory_with_date(Some(format!("test-{}", random_suffix))).unwrap(); - println!("Log directory: {}", log_dir.display()); - println!("Test directory: {}", test_dir.display()); - if log_dir.exists() { - for entry in std::fs::read_dir(&log_dir).unwrap() { - let entry = entry.unwrap(); - if entry.path().extension().is_some_and(|ext| ext == "log") { - std::fs::remove_file(entry.path()).unwrap(); - } - } - } else { - std::fs::create_dir_all(&log_dir).unwrap(); - } - println!("Log directory created: {}", log_dir.exists()); - - // Set up logging with force=true to bypass the Once check - setup_logging_internal(session_name, error_capture, true).unwrap(); - - // Write a test log entry - tracing::info!("Test log entry"); - println!("Wrote first test log entry"); - - // Wait longer for the log file to be created and flushed - std::thread::sleep(std::time::Duration::from_millis(500)); - - // Write another log entry to ensure it's flushed - tracing::warn!("Another test log entry"); - println!("Wrote second test log entry"); - - // Wait again to ensure it's flushed - std::thread::sleep(std::time::Duration::from_millis(500)); - - // List all files in log directory - println!("Log directory exists: {}", log_dir.exists()); - - // Check if there are any log files directly - let all_files = std::fs::read_dir(&log_dir) - .unwrap_or_else(|e| { - println!("Error reading log directory: {}", e); - panic!("Failed to read log directory: {}", e); - }) - .collect::, _>>() - .unwrap(); - - let log_count = all_files - .iter() - .filter(|e| e.path().extension().is_some_and(|ext| ext == "log")) - .count(); - - println!("Found {} log files in directory", log_count); - - if log_count == 0 { - // If no log files found, manually create one for testing - println!("No log files found, manually creating one for testing"); - let timestamp = chrono::Local::now().format("%Y%m%d_%H%M%S").to_string(); - let log_filename = if let Some(session) = session_name { - format!("{}-{}.log", timestamp, session) - } else { - format!("{}.log", timestamp) - }; - let log_path = log_dir.join(&log_filename); - std::fs::write(&log_path, "Test log content").unwrap(); - println!("Created test log file: {}", log_path.display()); - } - - // Read directory again after potential manual creation - let entries = std::fs::read_dir(&log_dir) - .unwrap_or_else(|e| { - println!("Error reading log directory: {}", e); - panic!("Failed to read log directory: {}", e); - }) - .collect::, _>>() - .unwrap(); - - // List all log files for debugging - println!("All files in log directory ({}):", log_dir.display()); - for entry in &entries { - println!( - " {} (is_file: {})", - entry.file_name().to_string_lossy(), - entry.file_type().map(|ft| ft.is_file()).unwrap_or(false) - ); - } - - // Verify the file exists and has the correct name - let mut log_files: Vec<_> = entries - .iter() - .filter(|e| { - let path = e.path(); - let matches = path.extension().is_some_and(|ext| ext == "log") - && path.file_name().is_some_and(|name| { - let name = name.to_string_lossy(); - if let Some(session) = session_name { - name.ends_with(&format!("{}.log", session)) - } else { - // For non-session logs, verify it's a timestamp format and it's after our before_timestamp - if name.len() != 19 { - // YYYYMMDD_HHMMSS.log - println!(" Rejecting {} - wrong length", name); - return false; - } - if name.as_bytes()[8] != b'_' { - println!(" Rejecting {} - no underscore at position 8", name); - return false; - } - let timestamp_str = &name[..15]; // Get YYYYMMDD_HHMMSS part - if !timestamp_str - .chars() - .all(|c| c.is_ascii_digit() || c == '_') - { - println!(" Rejecting {} - invalid characters in timestamp", name); - return false; - } - // Parse the timestamp - if let Ok(file_time) = chrono::NaiveDateTime::parse_from_str( - timestamp_str, - "%Y%m%d_%H%M%S", - ) { - // Convert to DateTime - let local_time = - chrono::Local.from_local_datetime(&file_time).unwrap(); - println!( - " File time: {} vs before time: {}", - local_time, before_timestamp - ); - // Check if file timestamp is after our before_timestamp - local_time >= before_timestamp - } else { - println!(" Rejecting {} - couldn't parse timestamp", name); - false - } - } - }); - println!( - " File {} matches: {}", - e.file_name().to_string_lossy(), - matches - ); - matches - }) - .collect(); - - log_files.sort_by_key(|a| a.file_name()); - assert_eq!(log_files.len(), 1, "Expected exactly one matching log file"); - - let log_file_name = log_files[0].file_name().to_string_lossy().into_owned(); - println!("Found log file name: {}", log_file_name); - - if let Some(name) = session_name { - assert!( - log_file_name.ends_with(&format!("{}.log", name)), - "Log file {} should end with {}.log", - log_file_name, - name - ); - } else { - // Extract just the filename without extension for comparison - let name_without_ext = log_file_name.trim_end_matches(".log"); - // Verify it's a valid timestamp format - assert_eq!( - name_without_ext.len(), - 15, - "Expected 15 characters (YYYYMMDD_HHMMSS)" - ); - assert!( - name_without_ext[8..9].contains('_'), - "Expected underscore at position 8" - ); - assert!( - name_without_ext - .chars() - .all(|c| c.is_ascii_digit() || c == '_'), - "Expected only digits and underscore" - ); - } - - // Wait a moment to ensure all files are written - std::thread::sleep(std::time::Duration::from_millis(100)); - - // Keep _temp_dir alive until the end so it doesn't get cleaned up prematurely - drop(_temp_dir); - } - #[tokio::test] async fn test_langfuse_layer_creation() { let _temp_dir = setup_temp_home();