diff --git a/Cargo.lock b/Cargo.lock index 05c67bef..3000e12d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -459,6 +459,7 @@ dependencies = [ "serde", "serde_json", "termcolor", + "toml", "uuid", ] @@ -1669,6 +1670,15 @@ dependencies = [ "serde_core", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + [[package]] name = "shlex" version = "1.3.0" @@ -1842,11 +1852,26 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "toml" +version = "0.8.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd87a5cdd6ffab733b2f74bc4fd7ee5fff6634124999ac278c35fc78c6120148" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + [[package]] name = "toml_datetime" version = "0.6.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0dd7358ecb8fc2f8d014bf86f6f638ce72ba252a2c3a2572f2a795f1d23efb41" +dependencies = [ + "serde", +] [[package]] name = "toml_edit" @@ -1855,6 +1880,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "02a8b472d1a3d7c18e2d61a489aee3453fd9031c33e4f55bd533f4a7adca1bee" dependencies = [ "indexmap", + "serde", + "serde_spanned", "toml_datetime", "winnow", ] diff --git a/crates/cargo-codspeed/Cargo.toml b/crates/cargo-codspeed/Cargo.toml index 6a17d2d1..6e0ad634 100644 --- a/crates/cargo-codspeed/Cargo.toml +++ b/crates/cargo-codspeed/Cargo.toml @@ -27,6 +27,7 @@ itertools = { workspace = true } anstyle = "1.0.8" serde = { workspace = true } serde_json = { workspace = true } +toml = "0.8" codspeed = { path = "../codspeed", version = "=4.0.5" } [dev-dependencies] diff --git a/crates/cargo-codspeed/src/build.rs b/crates/cargo-codspeed/src/build.rs index fbca0e6c..70e88a66 100644 --- a/crates/cargo-codspeed/src/build.rs +++ b/crates/cargo-codspeed/src/build.rs @@ -4,7 +4,9 @@ use crate::{ measurement_mode::MeasurementMode, prelude::*, }; +use anyhow::Context; use cargo_metadata::{camino::Utf8PathBuf, Message, Metadata, TargetKind}; +use std::collections::HashMap; use std::process::{exit, Command, Stdio}; struct BuildOptions<'a> { @@ -31,6 +33,50 @@ pub struct BuildConfig { pub passthrough_flags: Vec, } +fn get_bench_harness_value( + manifest_path: &Utf8PathBuf, + bench_name: &str, + cache: &mut HashMap, +) -> Result { + let manifest_table = if let Some(table) = cache.get(manifest_path) { + table + } else { + // Read and parse the Cargo.toml file + let manifest_content = std::fs::read_to_string(manifest_path) + .with_context(|| format!("Failed to read manifest at {manifest_path}"))?; + let table: toml::Table = toml::from_str(&manifest_content) + .with_context(|| format!("Failed to parse TOML in {manifest_path}"))?; + cache.insert(manifest_path.clone(), table); + cache.get(manifest_path).unwrap() + }; + + // Look for [[bench]] sections + let Some(benches) = manifest_table.get("bench").and_then(|v| v.as_array()) else { + // If no [[bench]] sections, it's not an error, benches present in /benches/.rs + // are still collected with harness = true + return Ok(true); + }; + + // Find the bench entry with matching name + let matching_bench = benches + .iter() + .filter_map(|bench| bench.as_table()) + .find(|bench_table| { + bench_table + .get("name") + .and_then(|v| v.as_str()) + .is_some_and(|name| name == bench_name) + }); + + // Check if harness is enabled (defaults to true) + let harness_enabled = matching_bench + .and_then(|bench_table| bench_table.get("harness")) + .and_then(|v| v.as_bool()) + .unwrap_or(true); + + Ok(harness_enabled) +} + impl BuildOptions<'_> { /// Builds the benchmarks by invoking cargo /// Returns a list of built benchmarks, with path to associated executables @@ -60,6 +106,8 @@ impl BuildOptions<'_> { ); let mut built_benches = Vec::new(); + let mut bench_targets_with_default_harness = Vec::new(); + let mut manifest_cache = HashMap::new(); let package_names = self .package_filters @@ -95,6 +143,15 @@ impl BuildOptions<'_> { let add_bench_to_codspeed_dir = package_names.iter().contains(&package.name); if add_bench_to_codspeed_dir { + if get_bench_harness_value( + &package.manifest_path, + &bench_target_name, + &mut manifest_cache, + )? { + bench_targets_with_default_harness + .push((package.name.to_string(), bench_target_name.clone())); + } + built_benches.push(BuiltBench { package: package.name.to_string(), bench: bench_target_name, @@ -114,6 +171,25 @@ impl BuildOptions<'_> { exit(status.code().expect("Could not get exit code")); } + if !bench_targets_with_default_harness.is_empty() { + let targets_list = bench_targets_with_default_harness + .into_iter() + .map(|(package, bench)| format!(" - `{bench}` in package `{package}`")) + .join("\n"); + + bail!("\ +CodSpeed will not work with the following benchmark targets: +{targets_list} + +CodSpeed requires benchmark targets to disable the default test harness because benchmark frameworks handle harnessing themselves. + +Either disable the default harness by adding `harness = false` to the corresponding \ +`[[bench]]` section in the Cargo.toml, or specify which targets to build by using \ +`cargo codspeed build -p package_name --bench first_target --bench second_target`. + +See `cargo codspeed build --help` for more information."); + } + for built_bench in &built_benches { eprintln!( "Built benchmark `{}` in package `{}`", diff --git a/crates/cargo-codspeed/src/run.rs b/crates/cargo-codspeed/src/run.rs index 2b52cb54..7fc6268a 100644 --- a/crates/cargo-codspeed/src/run.rs +++ b/crates/cargo-codspeed/src/run.rs @@ -134,7 +134,7 @@ pub fn run_benches( command .status() - .map_err(|e| anyhow!("failed to execute the benchmark process: {}", e)) + .map_err(|e| anyhow!("failed to execute the benchmark process: {e}")) .and_then(|status| { if status.success() { Ok(()) diff --git a/crates/cargo-codspeed/tests/default_harness_error.in/.gitignore b/crates/cargo-codspeed/tests/default_harness_error.in/.gitignore new file mode 100644 index 00000000..2c96eb1b --- /dev/null +++ b/crates/cargo-codspeed/tests/default_harness_error.in/.gitignore @@ -0,0 +1,2 @@ +target/ +Cargo.lock diff --git a/crates/cargo-codspeed/tests/default_harness_error.in/Cargo.toml b/crates/cargo-codspeed/tests/default_harness_error.in/Cargo.toml new file mode 100644 index 00000000..bc149a25 --- /dev/null +++ b/crates/cargo-codspeed/tests/default_harness_error.in/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "default-harness-error-test" +version = "0.1.0" +edition = "2021" +publish = false + +[dependencies] +bencher = "0.1.5" +codspeed = { path = "../../../codspeed" } +codspeed-bencher-compat = { path = "../../../bencher_compat" } + +[[bench]] +name = "bencher_example" +# Missing harness = false to trigger error + +[[bench]] +name = "bencher_correct" +harness = false diff --git a/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_correct.rs b/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_correct.rs new file mode 100644 index 00000000..8fee2c6a --- /dev/null +++ b/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_correct.rs @@ -0,0 +1,9 @@ +use codspeed::codspeed::black_box; +use codspeed_bencher_compat::{benchmark_group, benchmark_main, Bencher}; + +pub fn b(bench: &mut Bencher) { + bench.iter(|| (0..50).fold(0, |x, y| black_box(x + y))) +} + +benchmark_group!(benches, b); +benchmark_main!(benches); diff --git a/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_example.rs b/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_example.rs new file mode 100644 index 00000000..a5430914 --- /dev/null +++ b/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_example.rs @@ -0,0 +1,9 @@ +use codspeed::codspeed::black_box; +use codspeed_bencher_compat::{benchmark_group, benchmark_main, Bencher}; + +pub fn a(bench: &mut Bencher) { + bench.iter(|| (0..100).fold(0, |x, y| black_box(x + y))) +} + +benchmark_group!(benches, a); +benchmark_main!(benches); diff --git a/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_no_section.rs b/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_no_section.rs new file mode 100644 index 00000000..2c8f7ab3 --- /dev/null +++ b/crates/cargo-codspeed/tests/default_harness_error.in/benches/bencher_no_section.rs @@ -0,0 +1,9 @@ +use codspeed::codspeed::black_box; +use codspeed_bencher_compat::{benchmark_group, benchmark_main, Bencher}; + +pub fn c(bench: &mut Bencher) { + bench.iter(|| (0..75).fold(0, |x, y| black_box(x + y))) +} + +benchmark_group!(benches, c); +benchmark_main!(benches); diff --git a/crates/cargo-codspeed/tests/default_harness_error.rs b/crates/cargo-codspeed/tests/default_harness_error.rs new file mode 100644 index 00000000..78ec4cf0 --- /dev/null +++ b/crates/cargo-codspeed/tests/default_harness_error.rs @@ -0,0 +1,33 @@ +use predicates::prelude::*; +use predicates::str::contains; + +mod helpers; +use helpers::*; + +const DIR: &str = "tests/default_harness_error.in"; + +#[test] +fn test_default_harness_error() { + let dir = setup(DIR, Project::DefaultHarnessError); + cargo_codspeed(&dir) + .arg("build") + .assert() + .failure() + .stderr(contains( + "Error: CodSpeed will not work with the following benchmark targets:", + )) + // Should report bencher_example (explicit bench with missing harness = false) + .stderr(contains( + "`bencher_example` in package `default-harness-error-test`", + )) + // Should report bencher_no_section (no [[bench]] section means default harness) + .stderr(contains( + "`bencher_no_section` in package `default-harness-error-test`", + )) + .stderr(contains( + "CodSpeed requires benchmark targets to disable the default test harness", + )) + // Ensure the correct benchmark with harness = false is NOT reported + .stderr(contains("bencher_correct").not()); + teardown(dir); +} diff --git a/crates/cargo-codspeed/tests/helpers.rs b/crates/cargo-codspeed/tests/helpers.rs index f9259d14..1bc93c47 100644 --- a/crates/cargo-codspeed/tests/helpers.rs +++ b/crates/cargo-codspeed/tests/helpers.rs @@ -22,6 +22,7 @@ pub enum Project { Workspace, PackageInDeps, CratesWorkingDirectory, + DefaultHarnessError, } pub fn setup(dir: &str, project: Project) -> String { @@ -38,7 +39,7 @@ pub fn setup(dir: &str, project: Project) -> String { let package_root = PathBuf::from_str(env!("CARGO_MANIFEST_DIR")).unwrap(); let workspace_root = package_root.parent().unwrap().parent().unwrap(); match project { - Project::Simple | Project::Features => { + Project::Simple | Project::Features | Project::DefaultHarnessError => { replace_in_file( tmp_dir.join("Cargo.toml").to_str().unwrap(), "../../..", diff --git a/crates/cargo-codspeed/tests/workspace.in/b/Cargo.toml b/crates/cargo-codspeed/tests/workspace.in/b/Cargo.toml index 5fb96d32..d9297c79 100644 --- a/crates/cargo-codspeed/tests/workspace.in/b/Cargo.toml +++ b/crates/cargo-codspeed/tests/workspace.in/b/Cargo.toml @@ -12,3 +12,7 @@ codspeed-bencher-compat = { path = "../../../../bencher_compat" } [[bench]] name = "bencher_example" harness = false + +[[bench]] +name = "another_bencher_example" +harness = false diff --git a/crates/criterion_compat/Cargo.toml b/crates/criterion_compat/Cargo.toml index 54b9f96c..5b130ef0 100644 --- a/crates/criterion_compat/Cargo.toml +++ b/crates/criterion_compat/Cargo.toml @@ -11,9 +11,9 @@ repository = "https://github.com/CodSpeedHQ/codspeed-rust" homepage = "https://codspeed.io" license = "MIT OR Apache-2.0" categories = [ - "development-tools", - "development-tools::profiling", - "development-tools::testing", + "development-tools", + "development-tools::profiling", + "development-tools::testing", ] keywords = ["codspeed", "benchmark", "criterion"] [dependencies] @@ -26,7 +26,7 @@ regex = { version = "1.5", default-features = false, features = ["std"] } futures = { version = "0.3", default-features = false, optional = true } smol = { version = "2.0", default-features = false, optional = true } tokio = { version = "1.39", default-features = false, features = [ - "rt", + "rt", ], optional = true } async-std = { version = "1.12", optional = true } diff --git a/crates/divan_compat/Cargo.toml b/crates/divan_compat/Cargo.toml index b45effe7..f9ce305a 100644 --- a/crates/divan_compat/Cargo.toml +++ b/crates/divan_compat/Cargo.toml @@ -11,9 +11,9 @@ repository = "https://github.com/CodSpeedHQ/codspeed-rust" homepage = "https://codspeed.io" license = "MIT OR Apache-2.0" categories = [ - "development-tools", - "development-tools::profiling", - "development-tools::testing", + "development-tools", + "development-tools::profiling", + "development-tools::testing", ] keywords = ["codspeed", "benchmark", "divan"]