Skip to content

Commit 613f1c5

Browse files
committed
feat: skip upload if --allow-empty is set and no results were produced
Note: this will only work for `walltime` and `memory` instruments
1 parent 06d99f2 commit 613f1c5

5 files changed

Lines changed: 59 additions & 19 deletions

File tree

src/executor/memory/executor.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::binary_installer::ensure_binary_installed;
2-
use crate::executor::ExecutorName;
32
use crate::executor::helpers::command::CommandBuilder;
43
use crate::executor::helpers::env::get_base_injected_env;
54
use crate::executor::helpers::get_bench_command::get_bench_command;
@@ -8,6 +7,7 @@ use crate::executor::helpers::run_with_env::wrap_with_env;
87
use crate::executor::helpers::run_with_sudo::wrap_with_sudo;
98
use crate::executor::shared::fifo::RunnerFifo;
109
use crate::executor::{ExecutionContext, Executor};
10+
use crate::executor::{ExecutorName, ExecutorTeardownResult};
1111
use crate::instruments::mongo_tracer::MongoTracer;
1212
use crate::prelude::*;
1313
use crate::run::check_system::SystemInfo;
@@ -118,7 +118,10 @@ impl Executor for MemoryExecutor {
118118
Ok(())
119119
}
120120

121-
async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> {
121+
async fn teardown(
122+
&self,
123+
execution_context: &ExecutionContext,
124+
) -> Result<ExecutorTeardownResult> {
122125
let files: Vec<_> = std::fs::read_dir(execution_context.profile_folder.join("results"))?
123126
.filter_map(Result::ok)
124127
// Filter out non-memtrack files:
@@ -131,13 +134,22 @@ impl Executor for MemoryExecutor {
131134
.flat_map(|f| std::fs::File::open(f.path()))
132135
.filter(|file| !MemtrackArtifact::is_empty(file))
133136
.collect();
137+
134138
if files.is_empty() {
135-
bail!(
136-
"No memtrack artifact files found. Does the integration support memory profiling?"
137-
);
139+
if !execution_context.config.allow_empty {
140+
bail!(
141+
"No memtrack artifact files found. Does the integration support memory profiling?"
142+
);
143+
} else {
144+
warn!(
145+
"No memtrack artifact files found. Does the integration support memory profiling?"
146+
);
147+
}
138148
}
139149

140-
Ok(())
150+
Ok(ExecutorTeardownResult {
151+
has_results: !files.is_empty(),
152+
})
141153
}
142154
}
143155

src/executor/mod.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@ pub fn get_all_executors() -> Vec<Box<dyn Executor>> {
7171
]
7272
}
7373

74+
pub struct ExecutorTeardownResult {
75+
/// Whether the executor has produced results
76+
/// Normally, the executor should fail if it does not produce results
77+
/// Unless `allow_empty` option is set
78+
has_results: bool,
79+
}
80+
7481
#[async_trait(?Send)]
7582
pub trait Executor {
7683
fn name(&self) -> ExecutorName;
@@ -91,7 +98,10 @@ pub trait Executor {
9198
mongo_tracer: &Option<MongoTracer>,
9299
) -> Result<()>;
93100

94-
async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()>;
101+
async fn teardown(
102+
&self,
103+
execution_context: &ExecutionContext,
104+
) -> Result<ExecutorTeardownResult>;
95105
}
96106

97107
/// Execute benchmarks with the given configuration
@@ -119,6 +129,8 @@ where
119129
end_group!();
120130
}
121131

132+
let mut has_results = false;
133+
122134
if !execution_context.config.skip_run {
123135
start_opened_group!("Running the benchmarks");
124136

@@ -141,7 +153,8 @@ where
141153
}
142154
end_group!();
143155
start_opened_group!("Tearing down environment");
144-
executor.teardown(execution_context).await?;
156+
let teardown_result = executor.teardown(execution_context).await?;
157+
has_results = teardown_result.has_results;
145158

146159
execution_context
147160
.logger
@@ -153,7 +166,7 @@ where
153166
};
154167

155168
// Handle upload and polling
156-
if !execution_context.config.skip_upload {
169+
if has_results && !execution_context.config.skip_upload {
157170
if !execution_context.is_local() {
158171
// If relevant, set the OIDC token for authentication
159172
// Note: OIDC tokens can expire quickly, so we set it just before the upload

src/executor/valgrind/executor.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use async_trait::async_trait;
22
use std::path::Path;
33

4-
use crate::executor::Executor;
54
use crate::executor::{ExecutionContext, ExecutorName};
5+
use crate::executor::{Executor, ExecutorTeardownResult};
66
use crate::instruments::mongo_tracer::MongoTracer;
77
use crate::prelude::*;
88
use crate::run::check_system::SystemInfo;
@@ -45,7 +45,10 @@ impl Executor for ValgrindExecutor {
4545
Ok(())
4646
}
4747

48-
async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> {
48+
async fn teardown(
49+
&self,
50+
execution_context: &ExecutionContext,
51+
) -> Result<ExecutorTeardownResult> {
4952
harvest_perf_maps(&execution_context.profile_folder).await?;
5053

5154
// 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 {
5558
// A comprehensive message will be sent to the user if no benchmarks are detected,
5659
// even if it's later in the process than technically possible.
5760

58-
Ok(())
61+
Ok(ExecutorTeardownResult { has_results: true })
5962
}
6063
}

src/executor/wall_time/executor.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use super::helpers::validate_walltime_results;
22
use super::perf::PerfRunner;
33
use crate::executor::Config;
44
use crate::executor::Executor;
5+
use crate::executor::ExecutorTeardownResult;
56
use crate::executor::helpers::command::CommandBuilder;
67
use crate::executor::helpers::env::{get_base_injected_env, is_codspeed_debug_enabled};
78
use crate::executor::helpers::get_bench_command::get_bench_command;
@@ -195,7 +196,10 @@ impl Executor for WallTimeExecutor {
195196
Ok(())
196197
}
197198

198-
async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> {
199+
async fn teardown(
200+
&self,
201+
execution_context: &ExecutionContext,
202+
) -> Result<ExecutorTeardownResult> {
199203
debug!("Copying files to the profile folder");
200204

201205
if let Some(perf) = &self.perf
@@ -205,12 +209,12 @@ impl Executor for WallTimeExecutor {
205209
.await?;
206210
}
207211

208-
validate_walltime_results(
212+
let has_results = validate_walltime_results(
209213
&execution_context.profile_folder,
210214
execution_context.config.allow_empty,
211215
)?;
212216

213-
Ok(())
217+
Ok(ExecutorTeardownResult { has_results })
214218
}
215219
}
216220

src/executor/wall_time/helpers.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ fn add_empty_result_error_explanation(error_details: &str) -> String {
1212

1313
/// Validates that walltime results exist and contain at least one benchmark.
1414
/// When `allow_empty` is true, empty benchmark results are allowed.
15-
pub fn validate_walltime_results(profile_folder: &Path, allow_empty: bool) -> Result<()> {
15+
///
16+
/// Returns `Ok(true)` if valid results are found, `Ok(false)` if no results are found but `allow_empty` is true
17+
pub fn validate_walltime_results(profile_folder: &Path, allow_empty: bool) -> Result<bool> {
1618
let results_dir = profile_folder.join("results");
1719

1820
if !results_dir.exists() {
1921
if allow_empty {
2022
warn!("No walltime results found in profile folder: {results_dir:?}.");
21-
return Ok(());
23+
return Ok(false);
2224
}
2325
bail!(add_empty_result_error_explanation(&format!(
2426
"No walltime results found in profile folder: {results_dir:?}."
@@ -64,14 +66,14 @@ pub fn validate_walltime_results(profile_folder: &Path, allow_empty: bool) -> Re
6466
if !found_benchmark_results {
6567
if allow_empty {
6668
warn!("No JSON result files found in: {results_dir:?}.");
67-
return Ok(());
69+
return Ok(false);
6870
}
6971
bail!(add_empty_result_error_explanation(&format!(
7072
"No JSON result files found in: {results_dir:?}."
7173
)));
7274
}
7375

74-
Ok(())
76+
Ok(true)
7577
}
7678

7779
#[cfg(test)]
@@ -188,6 +190,7 @@ mod tests {
188190

189191
let result = validate_walltime_results(profile.path(), false);
190192
assert!(result.is_ok());
193+
assert!(result.unwrap());
191194
}
192195

193196
#[test]
@@ -198,6 +201,7 @@ mod tests {
198201

199202
let result = validate_walltime_results(profile.path(), false);
200203
assert!(result.is_ok());
204+
assert!(result.unwrap());
201205
}
202206

203207
#[test]
@@ -209,6 +213,7 @@ mod tests {
209213

210214
let result = validate_walltime_results(profile.path(), false);
211215
assert!(result.is_ok());
216+
assert!(result.unwrap());
212217
}
213218

214219
// Failure cases
@@ -290,6 +295,7 @@ mod tests {
290295

291296
let result = validate_walltime_results(profile.path(), true);
292297
assert!(result.is_ok());
298+
assert!(!result.unwrap());
293299
}
294300

295301
#[test]
@@ -299,6 +305,7 @@ mod tests {
299305

300306
let result = validate_walltime_results(profile.path(), true);
301307
assert!(result.is_ok());
308+
assert!(!result.unwrap());
302309
}
303310

304311
#[test]
@@ -308,5 +315,6 @@ mod tests {
308315

309316
let result = validate_walltime_results(profile.path(), true);
310317
assert!(result.is_ok());
318+
assert!(!result.unwrap());
311319
}
312320
}

0 commit comments

Comments
 (0)