Skip to content

Commit 66623cf

Browse files
feat: add exec subcommand and refactor run subcommand
Refactor aims to extract common behavior between exec and run. It mostly moves out the run::Config into the executor module, as well as moving the run_environment module to the root of the repo. The main aim is to offload the overloaded run module that has outgrown its initial purpose of a clap subcommand.
1 parent b982cbe commit 66623cf

File tree

68 files changed

+514
-70954
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+514
-70954
lines changed

src/app.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::{
44
api_client::CodSpeedAPIClient,
55
auth,
66
config::CodSpeedConfig,
7+
exec,
78
local_logger::{CODSPEED_U8_COLOR_CODE, init_local_logger},
89
prelude::*,
910
run, setup,
@@ -59,8 +60,11 @@ pub struct Cli {
5960

6061
#[derive(Subcommand, Debug)]
6162
enum Commands {
62-
/// Run the bench command and upload the results to CodSpeed
63+
/// Run a benchmark program that already contains the CodSpeed instrumentation and upload the results to CodSpeed
6364
Run(Box<run::RunArgs>),
65+
/// Run a command after adding CodSpeed instrumentation to it and upload the results to
66+
/// CodSpeed
67+
Exec(Box<exec::ExecArgs>),
6468
/// Manage the CLI authentication state
6569
Auth(auth::AuthArgs),
6670
/// Pre-install the codspeed executors
@@ -80,7 +84,7 @@ pub async fn run() -> Result<()> {
8084
let setup_cache_dir = setup_cache_dir.as_deref();
8185

8286
match cli.command {
83-
Commands::Run(_) => {} // Run is responsible for its own logger initialization
87+
Commands::Run(_) | Commands::Exec(_) => {} // Run and Exec are responsible for their own logger initialization
8488
_ => {
8589
init_local_logger()?;
8690
}
@@ -90,6 +94,9 @@ pub async fn run() -> Result<()> {
9094
Commands::Run(args) => {
9195
run::run(*args, &api_client, &codspeed_config, setup_cache_dir).await?
9296
}
97+
Commands::Exec(args) => {
98+
exec::run(*args, &api_client, &codspeed_config, setup_cache_dir).await?
99+
}
93100
Commands::Auth(args) => auth::run(args, &api_client, cli.config_name.as_deref()).await?,
94101
Commands::Setup => setup::setup(setup_cache_dir).await?,
95102
}

src/exec/mod.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
use crate::api_client::CodSpeedAPIClient;
2+
use crate::config::CodSpeedConfig;
3+
use crate::prelude::*;
4+
use clap::Args;
5+
use std::path::Path;
6+
7+
mod run_with_harness;
8+
use run_with_harness::wrap_command_with_harness;
9+
10+
#[derive(Args, Debug)]
11+
pub struct ExecArgs {
12+
#[command(flatten)]
13+
pub shared: crate::run::ExecAndRunSharedArgs,
14+
15+
/// Optional benchmark name (defaults to command filename)
16+
#[arg(long)]
17+
pub name: Option<String>,
18+
19+
/// The command to execute with the exec harness
20+
pub command: Vec<String>,
21+
}
22+
23+
pub async fn run(
24+
args: ExecArgs,
25+
api_client: &CodSpeedAPIClient,
26+
codspeed_config: &CodSpeedConfig,
27+
setup_cache_dir: Option<&Path>,
28+
) -> Result<()> {
29+
// Wrap the user's command with exec-harness BEFORE creating config
30+
let wrapped_command = wrap_command_with_harness(&args.command, args.name.as_deref())?;
31+
32+
info!("Executing: {}", wrapped_command.join(" "));
33+
34+
// Convert ExecArgs to executor::Config using shared args
35+
let config = crate::executor::Config {
36+
upload_url: args
37+
.shared
38+
.upload_url
39+
.as_ref()
40+
.map(|url| url.parse())
41+
.transpose()
42+
.map_err(|e| anyhow!("Invalid upload URL: {e}"))?
43+
.unwrap_or_else(|| {
44+
"https://api.codspeed.io/upload"
45+
.parse()
46+
.expect("Default URL should be valid")
47+
}),
48+
token: args.shared.token,
49+
repository_override: args
50+
.shared
51+
.repository
52+
.map(|repo| {
53+
crate::executor::config::RepositoryOverride::from_arg(repo, args.shared.provider)
54+
})
55+
.transpose()?,
56+
working_directory: args.shared.working_directory,
57+
command: wrapped_command.join(" "), // Use wrapped command
58+
mode: args.shared.mode,
59+
instruments: crate::instruments::Instruments { mongodb: None }, // exec doesn't support MongoDB
60+
enable_perf: args.shared.perf_run_args.enable_perf,
61+
perf_unwinding_mode: args.shared.perf_run_args.perf_unwinding_mode,
62+
profile_folder: args.shared.profile_folder,
63+
skip_upload: args.shared.skip_upload,
64+
skip_run: args.shared.skip_run,
65+
skip_setup: args.shared.skip_setup,
66+
allow_empty: args.shared.allow_empty,
67+
};
68+
69+
// Delegate to shared execution logic
70+
crate::executor::execute_benchmarks(config, api_client, codspeed_config, setup_cache_dir, false)
71+
.await
72+
}

src/exec/run_with_harness.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
use crate::prelude::*;
2+
use std::path::PathBuf;
3+
4+
/// Find the exec-harness binary in the standard locations
5+
fn find_harness_binary() -> Result<PathBuf> {
6+
// Try common locations where the harness might be installed
7+
let mut possible_paths = Vec::new();
8+
9+
// Look in the same directory as the runner binary
10+
if let Ok(current_exe) = std::env::current_exe() {
11+
if let Some(parent) = current_exe.parent() {
12+
possible_paths.push(parent.join("exec-harness"));
13+
}
14+
15+
// Look in cargo target directory (for development)
16+
for ancestor in current_exe.ancestors() {
17+
if ancestor.ends_with("target") {
18+
possible_paths.push(ancestor.join("release").join("exec-harness"));
19+
possible_paths.push(ancestor.join("debug").join("exec-harness"));
20+
break;
21+
}
22+
}
23+
}
24+
25+
for path in &possible_paths {
26+
if path.exists() && path.is_file() {
27+
debug!("Found exec-harness at: {path:?}");
28+
return Ok(path.clone());
29+
}
30+
}
31+
32+
bail!(
33+
"exec-harness binary not found. Please ensure it's built and in the same directory as the runner.\nSearched paths: {possible_paths:?}"
34+
)
35+
}
36+
37+
/// Wraps the user's command with the exec-harness binary
38+
pub fn wrap_command_with_harness(
39+
user_command: &[String],
40+
benchmark_name: Option<&str>,
41+
) -> Result<Vec<String>> {
42+
if user_command.is_empty() {
43+
bail!("Cannot wrap empty command");
44+
}
45+
46+
let harness_path = find_harness_binary()?;
47+
let harness_path_str = harness_path
48+
.to_str()
49+
.context("exec-harness path contains invalid UTF-8")?;
50+
51+
let mut wrapped_command = vec![harness_path_str.to_string()];
52+
53+
// Add --name if provided
54+
if let Some(name) = benchmark_name {
55+
wrapped_command.push("--name".to_string());
56+
wrapped_command.push(name.to_string());
57+
}
58+
59+
// Add -- separator (optional but clearer)
60+
wrapped_command.push("--".to_string());
61+
62+
// Add the user's command
63+
wrapped_command.extend_from_slice(user_command);
64+
65+
debug!("Wrapped command: {wrapped_command:?}");
66+
Ok(wrapped_command)
67+
}
68+
69+
#[cfg(test)]
70+
mod tests {
71+
use super::*;
72+
73+
#[test]
74+
fn test_wrap_command_without_name() {
75+
let command = vec!["./my_binary".to_string(), "arg1".to_string()];
76+
let wrapped = wrap_command_with_harness(&command, None);
77+
78+
// We can't test the exact path, but we can verify the structure
79+
if let Ok(wrapped) = wrapped {
80+
assert!(wrapped[0].contains("exec-harness"));
81+
assert_eq!(wrapped[wrapped.len() - 2], "./my_binary");
82+
assert_eq!(wrapped[wrapped.len() - 1], "arg1");
83+
}
84+
}
85+
86+
#[test]
87+
fn test_wrap_command_with_name() {
88+
let command = vec!["./my_binary".to_string()];
89+
let wrapped = wrap_command_with_harness(&command, Some("custom_name"));
90+
91+
if let Ok(wrapped) = wrapped {
92+
assert!(wrapped[0].contains("exec-harness"));
93+
assert!(wrapped.contains(&"--name".to_string()));
94+
assert!(wrapped.contains(&"custom_name".to_string()));
95+
assert_eq!(wrapped[wrapped.len() - 1], "./my_binary");
96+
}
97+
}
98+
99+
#[test]
100+
fn test_wrap_empty_command() {
101+
let command: Vec<String> = vec![];
102+
let wrapped = wrap_command_with_harness(&command, None);
103+
assert!(wrapped.is_err());
104+
}
105+
}

0 commit comments

Comments
 (0)