Revert "feat: skip upload if --allow-empty is set and no results were produced"

src/executor/memory/executor.rs

@@ -1,4 +1,5 @@
 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;
@@ -7,7 +8,6 @@ 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,10 +118,7 @@ impl Executor for MemoryExecutor {
         Ok(())
     }
 
-    async fn teardown(
-        &self,
-        execution_context: &ExecutionContext,
-    ) -> Result<ExecutorTeardownResult> {
+    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:
@@ -134,22 +131,13 @@ impl Executor for MemoryExecutor {
             .flat_map(|f| std::fs::File::open(f.path()))
             .filter(|file| !MemtrackArtifact::is_empty(file))
             .collect();
-
         if files.is_empty() {
-            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?"
-                );
-            }
+            bail!(
+                "No memtrack artifact files found. Does the integration support memory profiling?"
+            );
         }
 
-        Ok(ExecutorTeardownResult {
-            has_results: !files.is_empty(),
-        })
+        Ok(())
     }
 }
 

src/executor/mod.rs

@@ -71,13 +71,6 @@ pub fn get_all_executors() -> Vec<Box<dyn Executor>> {
     ]
 }
 
-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;
@@ -98,10 +91,7 @@ pub trait Executor {
         mongo_tracer: &Option<MongoTracer>,
     ) -> Result<()>;
 
-    async fn teardown(
-        &self,
-        execution_context: &ExecutionContext,
-    ) -> Result<ExecutorTeardownResult>;
+    async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()>;
 }
 
 /// Execute benchmarks with the given configuration
@@ -129,8 +119,6 @@ where
         end_group!();
     }
 
-    let mut has_results = false;
-
     if !execution_context.config.skip_run {
         start_opened_group!("Running the benchmarks");
 
@@ -153,8 +141,7 @@ where
         }
         end_group!();
         start_opened_group!("Tearing down environment");
-        let teardown_result = executor.teardown(execution_context).await?;
-        has_results = teardown_result.has_results;
+        executor.teardown(execution_context).await?;
 
         execution_context
             .logger
@@ -166,7 +153,7 @@ where
     };
 
     // Handle upload and polling
-    if has_results && !execution_context.config.skip_upload {
+    if !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

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,10 +45,7 @@ impl Executor for ValgrindExecutor {
         Ok(())
     }
 
-    async fn teardown(
-        &self,
-        execution_context: &ExecutionContext,
-    ) -> Result<ExecutorTeardownResult> {
+    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.
@@ -58,6 +55,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(ExecutorTeardownResult { has_results: true })
+        Ok(())
     }
 }

src/executor/wall_time/executor.rs

@@ -2,7 +2,6 @@ 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;
@@ -196,10 +195,7 @@ impl Executor for WallTimeExecutor {
         Ok(())
     }
 
-    async fn teardown(
-        &self,
-        execution_context: &ExecutionContext,
-    ) -> Result<ExecutorTeardownResult> {
+    async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> {
         debug!("Copying files to the profile folder");
 
         if let Some(perf) = &self.perf
@@ -209,12 +205,12 @@ impl Executor for WallTimeExecutor {
                 .await?;
         }
 
-        let has_results = validate_walltime_results(
+        validate_walltime_results(
             &execution_context.profile_folder,
             execution_context.config.allow_empty,
         )?;
 
-        Ok(ExecutorTeardownResult { has_results })
+        Ok(())
     }
 }
 

src/executor/wall_time/helpers.rs

@@ -12,15 +12,13 @@ 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.
-///
-/// 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<bool> {
+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(false);
+            return Ok(());
         }
         bail!(add_empty_result_error_explanation(&format!(
             "No walltime results found in profile folder: {results_dir:?}."
@@ -54,26 +52,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(false);
+            return Ok(());
         }
         bail!(add_empty_result_error_explanation(&format!(
             "No JSON result files found in: {results_dir:?}."
         )));
     }
 
-    Ok(true)
+    Ok(())
 }
 
 #[cfg(test)]
@@ -190,7 +188,6 @@ mod tests {
 
         let result = validate_walltime_results(profile.path(), false);
         assert!(result.is_ok());
-        assert!(result.unwrap());
     }
 
     #[test]
@@ -201,7 +198,6 @@ mod tests {
 
         let result = validate_walltime_results(profile.path(), false);
         assert!(result.is_ok());
-        assert!(result.unwrap());
     }
 
     #[test]
@@ -213,7 +209,6 @@ mod tests {
 
         let result = validate_walltime_results(profile.path(), false);
         assert!(result.is_ok());
-        assert!(result.unwrap());
     }
 
     // Failure cases
@@ -295,7 +290,6 @@ mod tests {
 
         let result = validate_walltime_results(profile.path(), true);
         assert!(result.is_ok());
-        assert!(!result.unwrap());
     }
 
     #[test]
@@ -305,7 +299,6 @@ mod tests {
 
         let result = validate_walltime_results(profile.path(), true);
         assert!(result.is_ok());
-        assert!(!result.unwrap());
     }
 
     #[test]
@@ -315,6 +308,5 @@ mod tests {
 
         let result = validate_walltime_results(profile.path(), true);
         assert!(result.is_ok());
-        assert!(!result.unwrap());
     }
 }