From 5efb015c817106342d6caa8d0cc2b36c0ff2557b Mon Sep 17 00:00:00 2001 From: Arthur Pastel Date: Sun, 21 Apr 2024 19:15:18 -0400 Subject: [PATCH] fix(cargo-codspeed): handle working-directory when using workspaces --- crates/cargo-codspeed/src/build.rs | 4 +- crates/cargo-codspeed/src/helpers.rs | 19 +-- crates/cargo-codspeed/src/run.rs | 118 ++++++++++++------ .../crates_working_directory.in/.gitignore | 2 + .../crates_working_directory.in/Cargo.toml | 2 + .../the_crate/Cargo.toml | 14 +++ .../the_crate/benches/bencher_example.rs | 12 ++ .../the_crate/input.txt | 1 + .../tests/crates_working_directory.rs | 36 ++++++ crates/cargo-codspeed/tests/helpers.rs | 12 ++ crates/cargo-codspeed/tests/simple.rs | 5 +- 11 files changed, 164 insertions(+), 61 deletions(-) create mode 100644 crates/cargo-codspeed/tests/crates_working_directory.in/.gitignore create mode 100644 crates/cargo-codspeed/tests/crates_working_directory.in/Cargo.toml create mode 100644 crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/Cargo.toml create mode 100644 crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/benches/bencher_example.rs create mode 100644 crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/input.txt create mode 100644 crates/cargo-codspeed/tests/crates_working_directory.rs diff --git a/crates/cargo-codspeed/src/build.rs b/crates/cargo-codspeed/src/build.rs index 654dbbd..9816fe5 100644 --- a/crates/cargo-codspeed/src/build.rs +++ b/crates/cargo-codspeed/src/build.rs @@ -1,5 +1,5 @@ use crate::{ - helpers::{clear_dir, get_codspeed_dir}, + helpers::{clear_dir, get_codspeed_target_dir}, prelude::*, }; @@ -125,7 +125,7 @@ pub fn build_benches( .shell() .status_with_color("Built", benches_names_str, Color::Green)?; - let mut codspeed_target_dir = get_codspeed_dir(ws); + let mut codspeed_target_dir = get_codspeed_target_dir(ws); create_dir_all(&codspeed_target_dir)?; if let Some(name) = package_name.as_ref() { codspeed_target_dir = codspeed_target_dir.join(name); diff --git a/crates/cargo-codspeed/src/helpers.rs b/crates/cargo-codspeed/src/helpers.rs index 13e8a08..34f4f13 100644 --- a/crates/cargo-codspeed/src/helpers.rs +++ b/crates/cargo-codspeed/src/helpers.rs @@ -1,30 +1,13 @@ use crate::prelude::*; use std::path::{Path, PathBuf}; -pub fn get_codspeed_dir(ws: &Workspace) -> PathBuf { +pub fn get_codspeed_target_dir(ws: &Workspace) -> PathBuf { ws.target_dir() .as_path_unlocked() .to_path_buf() .join("codspeed") } -pub fn read_dir_recursive

(dir: P) -> Result> -where - P: AsRef, -{ - let mut out = vec![]; - for entry in std::fs::read_dir(dir)? { - let entry = entry?; - let path = entry.path(); - if path.is_dir() { - out.extend(read_dir_recursive(&path)?); - } else { - out.push(path); - } - } - Ok(out) -} - pub fn clear_dir

(dir: P) -> Result<()> where P: AsRef, diff --git a/crates/cargo-codspeed/src/run.rs b/crates/cargo-codspeed/src/run.rs index de0f34a..52ec8cf 100644 --- a/crates/cargo-codspeed/src/run.rs +++ b/crates/cargo-codspeed/src/run.rs @@ -1,64 +1,101 @@ +use std::{io, path::PathBuf}; + use anyhow::anyhow; use termcolor::Color; -use crate::{ - helpers::{get_codspeed_dir, read_dir_recursive}, - prelude::*, -}; +use crate::{helpers::get_codspeed_target_dir, prelude::*}; + +struct BenchToRun { + bench_path: PathBuf, + bench_name: String, + working_directory: PathBuf, + package_name: String, +} pub fn run_benches( ws: &Workspace, - benches: Option>, + selected_bench_names: Option>, package: Option, ) -> Result<()> { - let mut codspeed_dir = get_codspeed_dir(ws); + let codspeed_target_dir = get_codspeed_target_dir(ws); - if let Some(package) = package { - codspeed_dir.push(package.clone()); - if !codspeed_dir.exists() { - return Err(anyhow!( + let packages_to_run = if let Some(package) = package.as_ref() { + let p = ws + .members() + .find(|m| m.manifest().name().to_string().as_str() == package); + if let Some(p) = p { + vec![p] + } else { + bail!("Package {} not found", package); + } + } else { + ws.default_members().collect::>() + }; + let mut benches: Vec = vec![]; + for p in packages_to_run { + let package_name = p.manifest().name().to_string(); + let is_root_package = p.root() == ws.root(); + let package_target_dir = if is_root_package { + codspeed_target_dir.clone() + } else { + codspeed_target_dir.join(&package_name) + }; + let working_directory = p.root().to_path_buf(); + if let io::Result::Ok(read_dir) = std::fs::read_dir(&package_target_dir) { + for entry in read_dir { + let entry = entry?; + let bench_path = entry.path(); + let bench_name = bench_path + .file_name() + .unwrap() + .to_str() + .unwrap() + .to_string(); + if !bench_path.is_dir() { + benches.push(BenchToRun { + package_name: package_name.clone(), + bench_path, + bench_name, + working_directory: working_directory.clone(), + }); + } + } + } + } + + if benches.is_empty() { + if let Some(package) = package.as_ref() { + bail!( "No benchmarks found. Run `cargo codspeed build -p {}` first.", package - )); + ); + } else { + bail!("No benchmarks found. Run `cargo codspeed build` first."); } } - if !codspeed_dir.exists() { - return Err(anyhow!( - "No benchmarks found. Run `cargo codspeed build` first." - )); - } - - let found_benches = read_dir_recursive(codspeed_dir)?; - if found_benches.is_empty() { - return Err(anyhow!( - "No benchmark target found. Run `cargo codspeed build` first." - )); - } let mut to_run = vec![]; - if let Some(benches) = benches { + if let Some(selected_bench_names) = selected_bench_names { // Make sure all benchmarks are found let mut not_found = vec![]; - for bench in benches.iter() { - let bench_path = found_benches - .iter() - .find(|b| b.file_name().unwrap().to_str().unwrap() == bench); + for bench_name in selected_bench_names.iter() { + let bench = benches.iter().find(|b| &b.bench_name == bench_name); - if let Some(bench_path) = bench_path { - to_run.push(bench_path.clone()); + if let Some(bench) = bench { + to_run.push(bench); } else { - not_found.push(bench); + not_found.push(bench_name); } } if !not_found.is_empty() { - return Err(anyhow!( + bail!( "The following benchmarks to run were not found: {}", not_found.iter().join(", ") - )); + ); } } else { - to_run = found_benches; + to_run = benches.iter().collect(); } ws.config().shell().status_with_color( "Collected", @@ -66,15 +103,18 @@ pub fn run_benches( Color::White, )?; for bench in to_run.iter() { - let bench_name = bench.file_name().unwrap().to_str().unwrap(); + let bench_name = &bench.bench_name; // workspace_root is needed since file! returns the path relatively to the workspace root // while CARGO_MANIFEST_DIR returns the path to the sub package let workspace_root = ws.root().to_string_lossy(); - ws.config() - .shell() - .status_with_color("Running", bench_name, Color::Yellow)?; - std::process::Command::new(bench) + ws.config().shell().status_with_color( + "Running", + format!("{} {}", &bench.package_name, bench_name), + Color::Yellow, + )?; + std::process::Command::new(&bench.bench_path) .env("CODSPEED_CARGO_WORKSPACE_ROOT", workspace_root.as_ref()) + .current_dir(&bench.working_directory) .status() .map_err(|_| anyhow!("failed to execute the benchmark process")) .and_then(|status| { diff --git a/crates/cargo-codspeed/tests/crates_working_directory.in/.gitignore b/crates/cargo-codspeed/tests/crates_working_directory.in/.gitignore new file mode 100644 index 0000000..1e7caa9 --- /dev/null +++ b/crates/cargo-codspeed/tests/crates_working_directory.in/.gitignore @@ -0,0 +1,2 @@ +Cargo.lock +target/ diff --git a/crates/cargo-codspeed/tests/crates_working_directory.in/Cargo.toml b/crates/cargo-codspeed/tests/crates_working_directory.in/Cargo.toml new file mode 100644 index 0000000..d499541 --- /dev/null +++ b/crates/cargo-codspeed/tests/crates_working_directory.in/Cargo.toml @@ -0,0 +1,2 @@ +[workspace] +members = ["the_crate"] diff --git a/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/Cargo.toml b/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/Cargo.toml new file mode 100644 index 0000000..be0d786 --- /dev/null +++ b/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "the_crate" +version = "0.0.0" +edition = "2021" +publish = false + +[dependencies] +bencher = "0.1.5" +codspeed = { path = "../../../../codspeed" } +codspeed-bencher-compat = { path = "../../../../bencher_compat" } + +[[bench]] +name = "bencher_example" +harness = false diff --git a/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/benches/bencher_example.rs b/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/benches/bencher_example.rs new file mode 100644 index 0000000..36c6b44 --- /dev/null +++ b/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/benches/bencher_example.rs @@ -0,0 +1,12 @@ +use std::hint::black_box; + +use codspeed_bencher_compat::{benchmark_group, benchmark_main, Bencher}; + +pub fn a(bench: &mut Bencher) { + // Open ./input.txt file + std::fs::read_to_string("./input.txt").expect("Failed to read file"); + 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/crates_working_directory.in/the_crate/input.txt b/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/input.txt new file mode 100644 index 0000000..6769dd6 --- /dev/null +++ b/crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/input.txt @@ -0,0 +1 @@ +Hello world! \ No newline at end of file diff --git a/crates/cargo-codspeed/tests/crates_working_directory.rs b/crates/cargo-codspeed/tests/crates_working_directory.rs new file mode 100644 index 0000000..6770e06 --- /dev/null +++ b/crates/cargo-codspeed/tests/crates_working_directory.rs @@ -0,0 +1,36 @@ +use predicates::str::contains; + +mod helpers; +use helpers::*; + +const DIR: &str = "tests/crates_working_directory.in"; + +#[test] +fn test_crates_working_directory_build_and_run_explicit() { + let dir = setup(DIR, Project::CratesWorkingDirectory); + cargo_codspeed(&dir) + .args(["build", "-p", "the_crate"]) + .assert() + .success(); + cargo_codspeed(&dir) + .args(["run", "-p", "the_crate"]) + .assert() + .success() + .stderr(contains("Finished running 1 benchmark suite(s)")); + teardown(dir); +} + +#[test] +fn test_crates_working_directory_build_and_run_implicit() { + let dir = setup(DIR, Project::CratesWorkingDirectory); + cargo_codspeed(&dir) + .args(["build", "-p", "the_crate"]) + .assert() + .success(); + cargo_codspeed(&dir) + .arg("run") + .assert() + .success() + .stderr(contains("Finished running 1 benchmark suite(s)")); + teardown(dir); +} diff --git a/crates/cargo-codspeed/tests/helpers.rs b/crates/cargo-codspeed/tests/helpers.rs index 9e0e849..3ab5103 100644 --- a/crates/cargo-codspeed/tests/helpers.rs +++ b/crates/cargo-codspeed/tests/helpers.rs @@ -21,6 +21,7 @@ pub enum Project { Features, Workspace, PackageInDeps, + CratesWorkingDirectory, } pub fn setup(dir: &str, project: Project) -> String { @@ -63,6 +64,17 @@ pub fn setup(dir: &str, project: Project) -> String { workspace_root.join("crates").to_str().unwrap(), ); } + Project::CratesWorkingDirectory => { + replace_in_file( + tmp_dir + .join("the_crate") + .join("Cargo.toml") + .to_str() + .unwrap(), + "../../../..", + workspace_root.join("crates").to_str().unwrap(), + ); + } } tmp_dir.to_str().unwrap().to_string() } diff --git a/crates/cargo-codspeed/tests/simple.rs b/crates/cargo-codspeed/tests/simple.rs index 0d6be80..78ce752 100644 --- a/crates/cargo-codspeed/tests/simple.rs +++ b/crates/cargo-codspeed/tests/simple.rs @@ -30,7 +30,7 @@ fn test_simple_build() { #[test] fn test_simple_build_and_run() { let dir = setup(DIR, Project::Simple); - cargo_codspeed(&dir).arg("build").assert(); + cargo_codspeed(&dir).arg("build").assert().success(); cargo_codspeed(&dir) .arg("run") .assert() @@ -58,7 +58,8 @@ fn test_simple_build_and_run_single() { cargo_codspeed(&dir) .arg("build") .arg("another_bencher_example") - .assert(); + .assert() + .success(); cargo_codspeed(&dir) .arg("run") .arg("another_bencher_example")