From 01fcd3febb8bcf40be8e7141fa586a354411c2c4 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 14 Jan 2026 15:17:06 +0100 Subject: [PATCH 1/5] feat(exec-harness): add the full command to the uri and handle hyphens better --- crates/exec-harness/src/main.rs | 17 ++++++-------- crates/exec-harness/src/uri.rs | 26 +++++++++++++++++++++ src/exec/mod.rs | 17 +++++++++++++- src/executor/config.rs | 9 ++------ src/executor/tests.rs | 41 +++++++++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 18 deletions(-) create mode 100644 crates/exec-harness/src/uri.rs diff --git a/crates/exec-harness/src/main.rs b/crates/exec-harness/src/main.rs index d755e3cb..e0e7984b 100644 --- a/crates/exec-harness/src/main.rs +++ b/crates/exec-harness/src/main.rs @@ -6,6 +6,7 @@ use runner_shared::walltime_results::WalltimeBenchmark; use std::path::PathBuf; mod prelude; +mod uri; mod walltime; #[derive(Parser, Debug)] @@ -15,7 +16,7 @@ mod walltime; about = "CodSpeed exec harness - wraps commands with performance instrumentation" )] struct Args { - /// Optional benchmark name (defaults to command filename) + /// Optional benchmark name, else the command will be used as the name #[arg(long)] name: Option, @@ -23,6 +24,7 @@ struct Args { execution_args: walltime::WalltimeExecutionArgs, /// The command and arguments to execute + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] command: Vec, } @@ -38,15 +40,10 @@ fn main() -> Result<()> { bail!("Error: No command provided"); } - // Derive benchmark name from command if not provided - let bench_name = args.name.unwrap_or_else(|| { - // Extract filename from command path - let cmd = &args.command[0]; - std::path::Path::new(cmd).to_string_lossy().into_owned() - }); - - // TODO: Better URI generation - let bench_uri = format!("standalone_run::{bench_name}"); + let uri::NameAndUri { + name: bench_name, + uri: bench_uri, + } = uri::generate_name_and_uri(&args.name, &args.command); let hooks = InstrumentHooks::instance(); diff --git a/crates/exec-harness/src/uri.rs b/crates/exec-harness/src/uri.rs new file mode 100644 index 00000000..1e387a02 --- /dev/null +++ b/crates/exec-harness/src/uri.rs @@ -0,0 +1,26 @@ +use crate::prelude::*; + +pub(crate) struct NameAndUri { + pub(crate) name: String, + pub(crate) uri: String, +} + +/// Maximum length for benchmark name to avoid excessively long URIs +/// Should be removed once we have structured metadata around benchmarks +const MAX_NAME_LENGTH: usize = 1024 - 100; + +pub(crate) fn generate_name_and_uri(name: &Option, command: &[String]) -> NameAndUri { + let mut name = name.clone().unwrap_or_else(|| command.join(" ")); + let uri = format!("exec_harness::{name}"); + + if name.len() > MAX_NAME_LENGTH { + warn!( + "Benchmark name is too long ({} characters). Truncating to {} characters.", + name.len(), + MAX_NAME_LENGTH + ); + name.truncate(MAX_NAME_LENGTH); + } + + NameAndUri { name, uri } +} diff --git a/src/exec/mod.rs b/src/exec/mod.rs index c3340bcf..36599193 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -14,9 +14,24 @@ mod poll_results; /// We temporarily force this name for all exec runs pub const DEFAULT_REPOSITORY_NAME: &str = "local-runs"; -pub const EXEC_HARNESS_COMMAND: &str = "exec-harness"; +const EXEC_HARNESS_COMMAND: &str = "exec-harness"; const EXEC_HARNESS_VERSION: &str = "1.0.0"; +/// Wraps a command with exec-harness and the given walltime arguments. +/// +/// This produces a shell command string like: +/// `exec-harness --warmup-time 1s --max-rounds 10 sleep 0.1` +pub fn wrap_with_exec_harness( + walltime_args: &exec_harness::walltime::WalltimeExecutionArgs, + command: &[String], +) -> String { + shell_words::join( + std::iter::once(EXEC_HARNESS_COMMAND) + .chain(walltime_args.to_cli_args().iter().map(|s| s.as_str())) + .chain(command.iter().map(|s| s.as_str())), + ) +} + #[derive(Args, Debug)] pub struct ExecArgs { #[command(flatten)] diff --git a/src/executor/config.rs b/src/executor/config.rs index 1eee92a0..362a6c1f 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -1,4 +1,4 @@ -use crate::exec::EXEC_HARNESS_COMMAND; +use crate::exec::wrap_with_exec_harness; use crate::instruments::Instruments; use crate::prelude::*; use crate::run::{RunArgs, UnwindingMode}; @@ -135,12 +135,7 @@ impl TryFrom for Config { let upload_url = Url::parse(&raw_upload_url) .map_err(|e| anyhow!("Invalid upload URL: {raw_upload_url}, {e}"))?; - let wrapped_command = std::iter::once(EXEC_HARNESS_COMMAND.to_owned()) - // Forward exec-harness arguments - .chain(args.walltime_args.to_cli_args()) - .chain(args.command) - .collect::>() - .join(" "); + let wrapped_command = wrap_with_exec_harness(&args.walltime_args, &args.command); Ok(Self { upload_url, diff --git a/src/executor/tests.rs b/src/executor/tests.rs index 70f46437..8cf30fe1 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -109,6 +109,16 @@ const ENV_TESTS: [(&str, &str); 8] = [ #[case(TESTS[5])] fn test_cases(#[case] cmd: &str) {} +// Exec-harness currently does not support the inline multi command scripts +#[template] +#[rstest::rstest] +#[case(TESTS[0])] +#[case(TESTS[1])] +#[case(TESTS[2])] +fn exec_harness_test_cases() -> Vec<&'static str> { + EXEC_HARNESS_COMMANDS.to_vec() +} + #[template] #[rstest::rstest] #[case(ENV_TESTS[0])] @@ -346,6 +356,37 @@ fi }) .await; } + + // Ensure that the walltime executor works with the exec-harness + #[apply(exec_harness_test_cases)] + #[rstest::rstest] + #[test_log::test(tokio::test)] + async fn test_exec_harness(#[case] cmd: &str) { + use crate::exec::wrap_with_exec_harness; + use exec_harness::walltime::WalltimeExecutionArgs; + + let (_permit, executor) = get_walltime_executor().await; + + let walltime_args = WalltimeExecutionArgs { + warmup_time: Some("0s".to_string()), + max_time: None, + min_time: None, + max_rounds: Some(3), + min_rounds: None, + }; + + let cmd = cmd.split(" ").map(|s| s.to_owned()).collect::>(); + let wrapped_command = wrap_with_exec_harness(&walltime_args, &cmd); + + // Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override + temp_env::async_with_vars(&[("GITHUB_ACTIONS", None::<&str>)], async { + let config = walltime_config(&wrapped_command, true); + dbg!(&config); + let (execution_context, _temp_dir) = create_test_setup(config).await; + executor.run(&execution_context, &None).await.unwrap(); + }) + .await; + } } mod memory { From c1d91afe95bba5be5e4a6844a64ca8f80becda85 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 14 Jan 2026 11:34:09 +0100 Subject: [PATCH 2/5] chore: remove double metadata information Currently, the codspeed rust core library automatically sends the integration metadata message to the runner. This will be changed later, but in the meantime, remove the second call. --- crates/exec-harness/src/main.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/exec-harness/src/main.rs b/crates/exec-harness/src/main.rs index e0e7984b..df574795 100644 --- a/crates/exec-harness/src/main.rs +++ b/crates/exec-harness/src/main.rs @@ -1,7 +1,6 @@ use crate::prelude::*; use crate::walltime::WalltimeResults; use clap::Parser; -use codspeed::instrument_hooks::InstrumentHooks; use runner_shared::walltime_results::WalltimeBenchmark; use std::path::PathBuf; @@ -45,13 +44,6 @@ fn main() -> Result<()> { uri: bench_uri, } = uri::generate_name_and_uri(&args.name, &args.command); - let hooks = InstrumentHooks::instance(); - - // TODO(COD-1736): Stop impersonating codspeed-rust 🥸 - hooks - .set_integration("codspeed-rust", env!("CARGO_PKG_VERSION")) - .unwrap(); - // Build execution options from CLI args let execution_options: walltime::ExecutionOptions = args.execution_args.try_into()?; From 7a24f1dd281099561ddbb27de7c806e579958e6d Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 14 Jan 2026 11:59:34 +0100 Subject: [PATCH 3/5] feat(exec-harness): add integration tests for complex cli commands --- crates/exec-harness/tests/integration_test.rs | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/crates/exec-harness/tests/integration_test.rs b/crates/exec-harness/tests/integration_test.rs index 0d2e541a..b9f8839b 100644 --- a/crates/exec-harness/tests/integration_test.rs +++ b/crates/exec-harness/tests/integration_test.rs @@ -321,3 +321,126 @@ fn test_single_long_execution() -> Result<()> { Ok(()) } + +/// Test that a command with shell operators (&&) works correctly when passed as a single argument +#[test] +fn test_command_with_shell_operators() -> Result<()> { + let exec_opts = exec_harness::walltime::ExecutionOptions::try_from( + exec_harness::walltime::WalltimeExecutionArgs { + warmup_time: Some("0s".to_string()), + max_time: None, + min_time: None, + max_rounds: Some(1), + min_rounds: None, + }, + )?; + + let tmpdir = TempDir::new()?; + let marker_file = tmpdir.path().join("marker.txt"); + + // This simulates: bash -c "echo first && echo second > marker.txt" + // The entire "echo first && echo second > marker.txt" should be passed as one argument to -c + let cmd = format!("echo first && echo second > {}", marker_file.display()); + + let times = exec_harness::walltime::perform( + "test::shell_operators".to_string(), + vec!["bash".to_string(), "-c".to_string(), cmd], + &exec_opts, + )?; + + assert_eq!(times.len(), 1, "Expected exactly 1 iteration"); + + // Verify that the second command (after &&) was executed + assert!( + marker_file.exists(), + "Marker file should exist - the second part of && was not executed" + ); + + let content = std::fs::read_to_string(&marker_file)?; + assert_eq!( + content.trim(), + "second", + "Marker file should contain 'second'" + ); + + Ok(()) +} + +/// Test that a command with pipes works correctly +#[test] +fn test_command_with_pipes() -> Result<()> { + let exec_opts = exec_harness::walltime::ExecutionOptions::try_from( + exec_harness::walltime::WalltimeExecutionArgs { + warmup_time: Some("0s".to_string()), + max_time: None, + min_time: None, + max_rounds: Some(1), + min_rounds: None, + }, + )?; + + let tmpdir = TempDir::new()?; + let output_file = tmpdir.path().join("output.txt"); + + // This simulates: bash -c "echo 'hello world' | tr 'a-z' 'A-Z' > output.txt" + let cmd = format!( + "echo 'hello world' | tr 'a-z' 'A-Z' > {}", + output_file.display() + ); + + let times = exec_harness::walltime::perform( + "test::pipes".to_string(), + vec!["bash".to_string(), "-c".to_string(), cmd], + &exec_opts, + )?; + + assert_eq!(times.len(), 1, "Expected exactly 1 iteration"); + + // Verify that the pipe worked correctly + let content = std::fs::read_to_string(&output_file)?; + assert_eq!( + content.trim(), + "HELLO WORLD", + "Pipe should have transformed text to uppercase" + ); + + Ok(()) +} + +/// Test that a command with quotes in the argument works correctly +#[test] +fn test_command_with_embedded_quotes() -> Result<()> { + let exec_opts = exec_harness::walltime::ExecutionOptions::try_from( + exec_harness::walltime::WalltimeExecutionArgs { + warmup_time: Some("0s".to_string()), + max_time: None, + min_time: None, + max_rounds: Some(1), + min_rounds: None, + }, + )?; + + let tmpdir = TempDir::new()?; + let output_file = tmpdir.path().join("output.txt"); + + // This simulates: bash -c "echo 'hello world' > output.txt" + let cmd = format!("echo 'hello world' > {}", output_file.display()); + + let times = exec_harness::walltime::perform( + "test::embedded_quotes".to_string(), + vec!["bash".to_string(), "-c".to_string(), cmd], + &exec_opts, + )?; + + assert_eq!(times.len(), 1, "Expected exactly 1 iteration"); + + // Verify that the quoted string was preserved + let content = std::fs::read_to_string(&output_file)?; + assert_eq!( + content.trim(), + "hello world", + "Quoted string should be preserved" + ); + + Ok(()) +} From 1b0b0f3c729c29bd5c53b6b7952e709f3098e9ca Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 14 Jan 2026 17:11:28 +0100 Subject: [PATCH 4/5] ci: install exec-harness before runner tests tests --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0cd3fdfe..349575ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,9 @@ jobs: - name: Install memtrack run: | cargo install --path crates/memtrack --locked + - name: Install exec-harness + run: | + cargo install --path crates/exec-harness --locked - run: cargo test --all --exclude memtrack --exclude exec-harness From 1ac1eae4dedec87d66f24c58ba67228e2fd97a5b Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 14 Jan 2026 17:22:33 +0100 Subject: [PATCH 5/5] ci: switch to rust-cache to cache builds of the installed workspace binaries --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 349575ec..a3b552d6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,11 @@ jobs: - uses: actions/checkout@v3 with: lfs: true - - uses: moonrepo/setup-rust@v1 + + - name: 'Install rust-toolchain.toml' + run: rustup toolchain install + # We use Swatinem/rust-cache to cache cargo registry, index and target in this job + - uses: Swatinem/rust-cache@v2 # Install memtrack for the memory integration tests - name: Install dependencies required for libbpf-sys (vendored feature)