fix(cargo-codspeed): handle working-directory when using workspaces

Author: art049

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);

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<P>(dir: P) -> Result<Vec<PathBuf>>
-where
-    P: AsRef<Path>,
-{
-    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<P>(dir: P) -> Result<()>
 where
     P: AsRef<Path>,

crates/cargo-codspeed/src/run.rs

@@ -1,80 +1,120 @@
+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<Vec<String>>,
+    selected_bench_names: Option<Vec<String>>,
     package: Option<String>,
 ) -> 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::<Vec<_>>()
+    };
+    let mut benches: Vec<BenchToRun> = 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",
         format!("{} benchmark suite(s) to run", to_run.len()),
         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| {

crates/cargo-codspeed/tests/crates_working_directory.in/.gitignore

@@ -0,0 +1,2 @@
+Cargo.lock
+target/

crates/cargo-codspeed/tests/crates_working_directory.in/Cargo.toml

@@ -0,0 +1,2 @@
+[workspace]
+members = ["the_crate"]

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

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);

crates/cargo-codspeed/tests/crates_working_directory.in/the_crate/input.txt

@@ -0,0 +1 @@
+Hello world!

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);
+}

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()
 }