diff --git a/src/executor/memory/executor.rs b/src/executor/memory/executor.rs index 72ba4a05..c1677533 100644 --- a/src/executor/memory/executor.rs +++ b/src/executor/memory/executor.rs @@ -1,5 +1,4 @@ use crate::binary_installer::ensure_binary_installed; -use crate::executor::ExecutorName; use crate::executor::helpers::command::CommandBuilder; use crate::executor::helpers::env::get_base_injected_env; use crate::executor::helpers::get_bench_command::get_bench_command; @@ -8,6 +7,7 @@ use crate::executor::helpers::run_with_env::wrap_with_env; use crate::executor::helpers::run_with_sudo::wrap_with_sudo; use crate::executor::shared::fifo::RunnerFifo; use crate::executor::{ExecutionContext, Executor}; +use crate::executor::{ExecutorName, ExecutorTeardownResult}; use crate::instruments::mongo_tracer::MongoTracer; use crate::prelude::*; use crate::run::check_system::SystemInfo; @@ -118,7 +118,10 @@ impl Executor for MemoryExecutor { Ok(()) } - async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> { + async fn teardown( + &self, + execution_context: &ExecutionContext, + ) -> Result { let files: Vec<_> = std::fs::read_dir(execution_context.profile_folder.join("results"))? .filter_map(Result::ok) // Filter out non-memtrack files: @@ -131,13 +134,22 @@ impl Executor for MemoryExecutor { .flat_map(|f| std::fs::File::open(f.path())) .filter(|file| !MemtrackArtifact::is_empty(file)) .collect(); + if files.is_empty() { - bail!( - "No memtrack artifact files found. Does the integration support memory profiling?" - ); + if !execution_context.config.allow_empty { + bail!( + "No memtrack artifact files found. Does the integration support memory profiling?" + ); + } else { + warn!( + "No memtrack artifact files found. Does the integration support memory profiling?" + ); + } } - Ok(()) + Ok(ExecutorTeardownResult { + has_results: !files.is_empty(), + }) } } diff --git a/src/executor/mod.rs b/src/executor/mod.rs index 1204bc9c..9410cdb2 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -71,6 +71,13 @@ pub fn get_all_executors() -> Vec> { ] } +pub struct ExecutorTeardownResult { + /// Whether the executor has produced results + /// Normally, the executor should fail if it does not produce results + /// Unless `allow_empty` option is set + has_results: bool, +} + #[async_trait(?Send)] pub trait Executor { fn name(&self) -> ExecutorName; @@ -91,7 +98,10 @@ pub trait Executor { mongo_tracer: &Option, ) -> Result<()>; - async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()>; + async fn teardown( + &self, + execution_context: &ExecutionContext, + ) -> Result; } /// Execute benchmarks with the given configuration @@ -119,6 +129,8 @@ where end_group!(); } + let mut has_results = false; + if !execution_context.config.skip_run { start_opened_group!("Running the benchmarks"); @@ -141,7 +153,8 @@ where } end_group!(); start_opened_group!("Tearing down environment"); - executor.teardown(execution_context).await?; + let teardown_result = executor.teardown(execution_context).await?; + has_results = teardown_result.has_results; execution_context .logger @@ -153,7 +166,7 @@ where }; // Handle upload and polling - if !execution_context.config.skip_upload { + if has_results && !execution_context.config.skip_upload { if !execution_context.is_local() { // If relevant, set the OIDC token for authentication // Note: OIDC tokens can expire quickly, so we set it just before the upload diff --git a/src/executor/valgrind/executor.rs b/src/executor/valgrind/executor.rs index 53d76034..21efcc79 100644 --- a/src/executor/valgrind/executor.rs +++ b/src/executor/valgrind/executor.rs @@ -1,8 +1,8 @@ use async_trait::async_trait; use std::path::Path; -use crate::executor::Executor; use crate::executor::{ExecutionContext, ExecutorName}; +use crate::executor::{Executor, ExecutorTeardownResult}; use crate::instruments::mongo_tracer::MongoTracer; use crate::prelude::*; use crate::run::check_system::SystemInfo; @@ -45,7 +45,10 @@ impl Executor for ValgrindExecutor { Ok(()) } - async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> { + async fn teardown( + &self, + execution_context: &ExecutionContext, + ) -> Result { harvest_perf_maps(&execution_context.profile_folder).await?; // No matter the command in input, at this point valgrind will have been run and have produced output files. @@ -55,6 +58,6 @@ impl Executor for ValgrindExecutor { // 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(()) + Ok(ExecutorTeardownResult { has_results: true }) } } diff --git a/src/executor/wall_time/executor.rs b/src/executor/wall_time/executor.rs index 7bb4faac..59429a91 100644 --- a/src/executor/wall_time/executor.rs +++ b/src/executor/wall_time/executor.rs @@ -2,6 +2,7 @@ use super::helpers::validate_walltime_results; use super::perf::PerfRunner; use crate::executor::Config; use crate::executor::Executor; +use crate::executor::ExecutorTeardownResult; use crate::executor::helpers::command::CommandBuilder; use crate::executor::helpers::env::{get_base_injected_env, is_codspeed_debug_enabled}; use crate::executor::helpers::get_bench_command::get_bench_command; @@ -195,7 +196,10 @@ impl Executor for WallTimeExecutor { Ok(()) } - async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> { + async fn teardown( + &self, + execution_context: &ExecutionContext, + ) -> Result { debug!("Copying files to the profile folder"); if let Some(perf) = &self.perf @@ -205,12 +209,12 @@ impl Executor for WallTimeExecutor { .await?; } - validate_walltime_results( + let has_results = validate_walltime_results( &execution_context.profile_folder, execution_context.config.allow_empty, )?; - Ok(()) + Ok(ExecutorTeardownResult { has_results }) } } diff --git a/src/executor/wall_time/helpers.rs b/src/executor/wall_time/helpers.rs index 23c15acc..dc4102fb 100644 --- a/src/executor/wall_time/helpers.rs +++ b/src/executor/wall_time/helpers.rs @@ -12,13 +12,15 @@ fn add_empty_result_error_explanation(error_details: &str) -> String { /// Validates that walltime results exist and contain at least one benchmark. /// When `allow_empty` is true, empty benchmark results are allowed. -pub fn validate_walltime_results(profile_folder: &Path, allow_empty: bool) -> Result<()> { +/// +/// Returns `Ok(true)` if valid results are found, `Ok(false)` if no results are found but `allow_empty` is true +pub fn validate_walltime_results(profile_folder: &Path, allow_empty: bool) -> Result { let results_dir = profile_folder.join("results"); if !results_dir.exists() { if allow_empty { warn!("No walltime results found in profile folder: {results_dir:?}."); - return Ok(()); + return Ok(false); } bail!(add_empty_result_error_explanation(&format!( "No walltime results found in profile folder: {results_dir:?}." @@ -52,26 +54,26 @@ pub fn validate_walltime_results(profile_folder: &Path, allow_empty: bool) -> Re ))); } debug!("No benchmarks found in {path:?} (allowed)"); + } else { + found_benchmark_results = true; + debug!( + "Found {} benchmark(s) in {path:?}", + results.benchmarks.len() + ); } - - found_benchmark_results = true; - debug!( - "Found {} benchmark(s) in {path:?}", - results.benchmarks.len() - ); } if !found_benchmark_results { if allow_empty { warn!("No JSON result files found in: {results_dir:?}."); - return Ok(()); + return Ok(false); } bail!(add_empty_result_error_explanation(&format!( "No JSON result files found in: {results_dir:?}." ))); } - Ok(()) + Ok(true) } #[cfg(test)] @@ -188,6 +190,7 @@ mod tests { let result = validate_walltime_results(profile.path(), false); assert!(result.is_ok()); + assert!(result.unwrap()); } #[test] @@ -198,6 +201,7 @@ mod tests { let result = validate_walltime_results(profile.path(), false); assert!(result.is_ok()); + assert!(result.unwrap()); } #[test] @@ -209,6 +213,7 @@ mod tests { let result = validate_walltime_results(profile.path(), false); assert!(result.is_ok()); + assert!(result.unwrap()); } // Failure cases @@ -290,6 +295,7 @@ mod tests { let result = validate_walltime_results(profile.path(), true); assert!(result.is_ok()); + assert!(!result.unwrap()); } #[test] @@ -299,6 +305,7 @@ mod tests { let result = validate_walltime_results(profile.path(), true); assert!(result.is_ok()); + assert!(!result.unwrap()); } #[test] @@ -308,5 +315,6 @@ mod tests { let result = validate_walltime_results(profile.path(), true); assert!(result.is_ok()); + assert!(!result.unwrap()); } }