fix: delete some flaky and not-useful tests (#4861)

This commit is contained in:
Jack Amadeo
2025-09-26 16:46:10 -04:00
committed by GitHub
parent 8662e0b745
commit 4af47fee09
-261
View File
@@ -22,35 +22,6 @@ fn get_log_directory() -> Result<PathBuf> {
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<String>) -> Result<PathBuf> {
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::<u32>()
);
// 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::<u32>() % 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::<Result<Vec<_>, _>>()
.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::<Result<Vec<_>, _>>()
.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<Local>
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();