diff --git a/AGENTS.md b/AGENTS.md index 3bc9f3e9..fcc3e902 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,16 +16,21 @@ cargo build # Build in release mode cargo build --release -# Run tests -cargo test +# Run tests (prefer nextest if available) +cargo nextest run # preferred if installed +cargo test # fallback if nextest is not available # Run specific test -cargo test +cargo nextest run # with nextest +cargo test # with cargo test # Run tests with output -cargo test -- --nocapture +cargo nextest run --nocapture # with nextest +cargo test -- --nocapture # with cargo test ``` +**Note**: Always check if `cargo nextest` is available first (with `cargo nextest --version` or `which cargo-nextest`). If available, use it instead of `cargo test` as it provides faster and more reliable test execution. + ### Running the Application ```bash # Build and run @@ -85,9 +90,13 @@ The core functionality for running benchmarks: ## Testing The project uses: -- Standard Rust `cargo test` +- `cargo nextest` (preferred) or standard Rust `cargo test` - `insta` for snapshot testing - `rstest` for parameterized tests - `temp-env` for environment variable testing Test files include snapshots in `snapshots/` directories for various run environment providers. + +**Important**: +- Always prefer `cargo nextest run` over `cargo test` when running tests, as it provides better performance and reliability. +- Some walltime executor tests require `sudo` access and will fail in non-interactive environments (e.g., `test_walltime_executor::*`). These failures are expected if sudo is not available. diff --git a/crates/runner-shared/src/lib.rs b/crates/runner-shared/src/lib.rs index fe655090..a5d0237a 100644 --- a/crates/runner-shared/src/lib.rs +++ b/crates/runner-shared/src/lib.rs @@ -2,3 +2,4 @@ pub mod debug_info; pub mod fifo; pub mod metadata; pub mod unwind_data; +pub mod walltime_results; diff --git a/crates/runner-shared/src/walltime_results.rs b/crates/runner-shared/src/walltime_results.rs new file mode 100644 index 00000000..ad7a6e73 --- /dev/null +++ b/crates/runner-shared/src/walltime_results.rs @@ -0,0 +1,65 @@ +// WARN: Keep in sync with codspeed-rust + +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Serialize, Deserialize, Clone)] +pub struct BenchmarkMetadata { + pub name: String, + pub uri: String, +} + +#[derive(Debug, Serialize, Deserialize, Clone)] +pub struct BenchmarkStats { + pub min_ns: f64, + pub max_ns: f64, + pub mean_ns: f64, + pub stdev_ns: f64, + + pub q1_ns: f64, + pub median_ns: f64, + pub q3_ns: f64, + + pub rounds: u64, + pub total_time: f64, + pub iqr_outlier_rounds: u64, + pub stdev_outlier_rounds: u64, + pub iter_per_round: u64, + pub warmup_iters: u64, +} + +#[derive(Debug, Serialize, Deserialize, Default, Clone)] +struct BenchmarkConfig { + warmup_time_ns: Option, + min_round_time_ns: Option, + max_time_ns: Option, + max_rounds: Option, +} + +#[derive(Debug, Serialize, Deserialize, Clone)] +pub struct WalltimeBenchmark { + #[serde(flatten)] + pub metadata: BenchmarkMetadata, + + config: BenchmarkConfig, + pub stats: BenchmarkStats, +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct Instrument { + #[serde(rename = "type")] + type_: String, +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct Creator { + name: String, + version: String, + pub pid: u32, +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct WalltimeResults { + pub creator: Creator, + pub instrument: Instrument, + pub benchmarks: Vec, +} diff --git a/src/run/runner/helpers/run_command_with_log_pipe.rs b/src/run/runner/helpers/run_command_with_log_pipe.rs index 2b034450..b4e43037 100644 --- a/src/run/runner/helpers/run_command_with_log_pipe.rs +++ b/src/run/runner/helpers/run_command_with_log_pipe.rs @@ -32,20 +32,48 @@ where ) -> Result<()> { let prefix = log_prefix.unwrap_or(""); let mut buffer = [0; 1024]; + let mut line_buffer = Vec::new(); + loop { let bytes_read = reader.read(&mut buffer)?; if bytes_read == 0 { + // Flush any remaining data in the line buffer + if !line_buffer.is_empty() { + suspend_progress_bar(|| { + writer.write_all(&line_buffer).unwrap(); + trace!( + target: EXECUTOR_TARGET, + "{}{}", + prefix, + String::from_utf8_lossy(&line_buffer) + ); + }); + } break; } - suspend_progress_bar(|| { - writer.write_all(&buffer[..bytes_read]).unwrap(); - trace!( - target: EXECUTOR_TARGET, - "{}{}", - prefix, - String::from_utf8_lossy(&buffer[..bytes_read]) - ); - }); + + // Add the chunk to our line buffer + line_buffer.extend_from_slice(&buffer[..bytes_read]); + + // Check if we have any complete lines (ending with \n or \r) + if let Some(last_newline_pos) = + line_buffer.iter().rposition(|&b| b == b'\n' || b == b'\r') + { + // We have at least one complete line, flush up to and including the last newline + let to_flush = &line_buffer[..=last_newline_pos]; + suspend_progress_bar(|| { + writer.write_all(to_flush).unwrap(); + trace!( + target: EXECUTOR_TARGET, + "{}{}", + prefix, + String::from_utf8_lossy(to_flush) + ); + }); + + // Keep the remainder in the buffer + line_buffer = line_buffer[last_newline_pos + 1..].to_vec(); + } } Ok(()) } diff --git a/src/run/runner/valgrind/executor.rs b/src/run/runner/valgrind/executor.rs index 596227ea..da970794 100644 --- a/src/run/runner/valgrind/executor.rs +++ b/src/run/runner/valgrind/executor.rs @@ -50,6 +50,13 @@ impl Executor for ValgrindExecutor { ) -> Result<()> { harvest_perf_maps(&run_data.profile_folder).await?; + // No matter the command in input, at this point valgrind will have been run and have produced output files. + // + // Contrary to walltime, checking that benchmarks have been detected here would require + // parsing the valgrind output files, which is not ideal at this stage. + // A comprehensive message will be sent to the user if no benchmarks are detected, + // even if it's later in the process than technically possible. + Ok(()) } } diff --git a/src/run/runner/wall_time/executor.rs b/src/run/runner/wall_time/executor.rs index f26334d0..fd49ed29 100644 --- a/src/run/runner/wall_time/executor.rs +++ b/src/run/runner/wall_time/executor.rs @@ -1,3 +1,4 @@ +use super::helpers::validate_walltime_results; use super::perf::PerfRunner; use crate::prelude::*; use crate::run::RunnerMode; @@ -224,6 +225,8 @@ impl Executor for WallTimeExecutor { perf.save_files_to(&run_data.profile_folder).await?; } + validate_walltime_results(&run_data.profile_folder)?; + Ok(()) } } diff --git a/src/run/runner/wall_time/helpers.rs b/src/run/runner/wall_time/helpers.rs new file mode 100644 index 00000000..f10c4b6f --- /dev/null +++ b/src/run/runner/wall_time/helpers.rs @@ -0,0 +1,271 @@ +use crate::prelude::*; +use runner_shared::walltime_results::WalltimeResults; +use std::path::Path; + +fn add_empty_result_error_explanation(error_details: &str) -> String { + format!( + "The command did not produce any CodSpeed result to process, did you run your benchmarks using a compatible CodSpeed integration?\n\ + Check out https://codspeed.io/docs/benchmarks/overview for more information.\n\n\ + Details: {error_details}" + ) +} + +/// Validates that walltime results exist and contain at least one benchmark. +pub fn validate_walltime_results(profile_folder: &Path) -> Result<()> { + let results_dir = profile_folder.join("results"); + + if !results_dir.exists() { + bail!(add_empty_result_error_explanation(&format!( + "No walltime results found in profile folder: {results_dir:?}." + ))); + } + + debug!("Validating walltime results in {results_dir:?}"); + + let mut found_valid_results = false; + + for entry in std::fs::read_dir(&results_dir)? { + let entry = entry?; + let path = entry.path(); + + // Only process JSON files + if path.extension().and_then(|s| s.to_str()) != Some("json") { + continue; + } + + debug!("Parsing walltime results from {path:?}"); + let file = std::fs::File::open(&path) + .with_context(|| format!("Failed to open walltime results file: {path:?}"))?; + + let results: WalltimeResults = serde_json::from_reader(&file) + .with_context(|| format!("Failed to parse walltime results from: {path:?}"))?; + + if results.benchmarks.is_empty() { + bail!(add_empty_result_error_explanation(&format!( + "No benchmarks found in walltime results file: {path:?}." + ))); + } + + found_valid_results = true; + debug!( + "Found {} benchmark(s) in {path:?}", + results.benchmarks.len() + ); + } + + if !found_valid_results { + bail!(add_empty_result_error_explanation(&format!( + "No JSON result files found in: {results_dir:?}." + ))); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use std::io::Write; + use tempfile::TempDir; + + // Test helpers + struct TestProfileFolder { + temp_dir: TempDir, + } + + impl TestProfileFolder { + fn new() -> Self { + Self { + temp_dir: TempDir::new().unwrap(), + } + } + + fn path(&self) -> &Path { + self.temp_dir.path() + } + + fn results_dir(&self) -> std::path::PathBuf { + self.path().join("results") + } + + fn create_results_dir(&self) { + fs::create_dir_all(self.results_dir()).unwrap(); + } + + fn write_json_file(&self, filename: &str, content: &str) { + self.create_results_dir(); + let file_path = self.results_dir().join(filename); + let mut file = fs::File::create(file_path).unwrap(); + file.write_all(content.as_bytes()).unwrap(); + } + + fn write_text_file(&self, filename: &str, content: &str) { + self.create_results_dir(); + let file_path = self.results_dir().join(filename); + let mut file = fs::File::create(file_path).unwrap(); + file.write_all(content.as_bytes()).unwrap(); + } + } + + fn valid_walltime_results_json(benchmark_count: usize) -> String { + let benchmarks: Vec = (0..benchmark_count) + .map(|i| { + format!( + r#"{{ + "name": "bench_{i}", + "uri": "test.rs::bench_{i}", + "config": {{}}, + "stats": {{ + "min_ns": 100.0, + "max_ns": 200.0, + "mean_ns": 150.0, + "stdev_ns": 10.0, + "q1_ns": 140.0, + "median_ns": 150.0, + "q3_ns": 160.0, + "rounds": 100, + "total_time": 15000.0, + "iqr_outlier_rounds": 0, + "stdev_outlier_rounds": 0, + "iter_per_round": 1, + "warmup_iters": 10 + }} + }}"# + ) + }) + .collect(); + + format!( + r#"{{ + "creator": {{ + "name": "test", + "version": "1.0.0", + "pid": 12345 + }}, + "instrument": {{ + "type": "walltime" + }}, + "benchmarks": [{}] + }}"#, + benchmarks.join(",") + ) + } + + fn empty_benchmarks_json() -> String { + r#"{ + "creator": { + "name": "test", + "version": "1.0.0", + "pid": 12345 + }, + "instrument": { + "type": "walltime" + }, + "benchmarks": [] + }"# + .to_string() + } + + // Success cases + + #[test] + fn test_valid_single_result_file() { + let profile = TestProfileFolder::new(); + profile.write_json_file("results.json", &valid_walltime_results_json(1)); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_ok()); + } + + #[test] + fn test_valid_multiple_result_files() { + let profile = TestProfileFolder::new(); + profile.write_json_file("results1.json", &valid_walltime_results_json(2)); + profile.write_json_file("results2.json", &valid_walltime_results_json(3)); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_ok()); + } + + #[test] + fn test_ignores_non_json_files() { + let profile = TestProfileFolder::new(); + profile.write_json_file("results.json", &valid_walltime_results_json(1)); + profile.write_text_file("readme.txt", "This is a text file"); + profile.write_text_file("data.csv", "col1,col2"); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_ok()); + } + + // Failure cases + + #[test] + fn test_missing_results_directory() { + let profile = TestProfileFolder::new(); + // Don't create results directory + + let result = validate_walltime_results(profile.path()); + assert!(result.is_err()); + let error = result.unwrap_err().to_string(); + assert!(error.contains("No walltime results found in profile folder")); + } + + #[test] + fn test_empty_results_directory() { + let profile = TestProfileFolder::new(); + profile.create_results_dir(); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_err()); + let error = result.unwrap_err().to_string(); + assert!(error.contains("No JSON result files found in")); + } + + #[test] + fn test_no_json_files_in_directory() { + let profile = TestProfileFolder::new(); + profile.write_text_file("readme.txt", "some text"); + profile.write_text_file("data.csv", "col1,col2"); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_err()); + let error = result.unwrap_err().to_string(); + assert!(error.contains("No JSON result files found in")); + } + + #[test] + fn test_empty_benchmarks_array() { + let profile = TestProfileFolder::new(); + profile.write_json_file("results.json", &empty_benchmarks_json()); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_err()); + let error = result.unwrap_err().to_string(); + assert!(error.contains("No benchmarks found in walltime results file")); + } + + #[test] + fn test_invalid_json_format() { + let profile = TestProfileFolder::new(); + profile.write_json_file("results.json", "{ invalid json }"); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_err()); + let error = result.unwrap_err().to_string(); + assert!(error.contains("Failed to parse walltime results from")); + } + + #[test] + fn test_multiple_files_one_empty() { + let profile = TestProfileFolder::new(); + profile.write_json_file("results1.json", &valid_walltime_results_json(2)); + profile.write_json_file("results2.json", &empty_benchmarks_json()); + + let result = validate_walltime_results(profile.path()); + assert!(result.is_err()); + let error = result.unwrap_err().to_string(); + assert!(error.contains("No benchmarks found in walltime results file")); + } +} diff --git a/src/run/runner/wall_time/mod.rs b/src/run/runner/wall_time/mod.rs index ca152f25..b790b6ac 100644 --- a/src/run/runner/wall_time/mod.rs +++ b/src/run/runner/wall_time/mod.rs @@ -1,2 +1,3 @@ pub mod executor; +pub mod helpers; pub mod perf;