diff --git a/src/main.rs b/src/main.rs index 68b8312..0e5f7a9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -109,7 +109,8 @@ fn find_cargo_projects(root: &Path, include_hidden: bool) -> Vec { } if let Some(target_directory) = is_cargo_root(entry.path()) { target_paths.insert(target_directory); - iter.skip_current_dir(); // no reason to look at the src and such + // Previously cargo-sweep skipped subdirectories here, but it is valid for + // subdirectories to contain cargo roots. } } } diff --git a/tests/integration.rs b/tests/integration.rs index 6313006..407fb6e 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -7,7 +7,7 @@ use std::{ use anyhow::{Context, Result}; use assert_cmd::{assert::Assert, cargo::cargo_bin, Command}; -use fs_extra::dir::get_size; +use fs_extra::dir::{get_size, CopyOptions}; use predicates::{prelude::PredicateBooleanExt, str::contains}; #[allow(unused_imports)] use pretty_assertions::{assert_eq, assert_ne}; @@ -16,16 +16,19 @@ use tempfile::{tempdir, TempDir}; use unindent::unindent; struct AnyhowWithContext(anyhow::Error); + impl Debug for AnyhowWithContext { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{:#?}", self.0) } } + impl> From for AnyhowWithContext { fn from(err: T) -> Self { Self(err.into()) } } + type TestResult = Result<(), AnyhowWithContext>; fn test_dir() -> PathBuf { @@ -52,10 +55,7 @@ fn sweep(args: &[&str]) -> Command { } /// Sets the target folder and runs the given command -fn run<'a>(mut cmd: impl BorrowMut, target: impl Into>) -> Assert { - if let Some(target) = target.into() { - cmd.borrow_mut().env("CARGO_TARGET_DIR", target); - } +fn run(mut cmd: impl BorrowMut) -> Assert { let assert = cmd.borrow_mut().assert().success(); let out = assert.get_output(); let str = |s| std::str::from_utf8(s).unwrap(); @@ -64,17 +64,29 @@ fn run<'a>(mut cmd: impl BorrowMut, target: impl Into> assert } -/// Returns the size of the build directory. +/// Runs a cargo build of the named project (which is expected to exist as a direct +/// child of the `tests` directory. +/// Returns the size of the build directory, as well as the [TempDir] of the unique +/// temporary build target directory. fn build(project: &str) -> Result<(u64, TempDir)> { let target = tempdir()?; let old_size = get_size(target.path())?; - run(cargo(project).arg("build"), target.path()); + run(cargo(project) + .arg("build") + .env("CARGO_TARGET_DIR", target.path())); let new_size = get_size(target.path())?; assert!(new_size > old_size, "cargo didn't build anything"); Ok((new_size, target)) } -fn clean_and_parse(target: &TempDir, args: &[&str]) -> Result { +/// Run the sweep command, cleaning up target files, and then parse the results and +/// return the number of cleaned files. This function takes a `cmd_modifier` which can +/// be used to modify the `sweep` [Command], e.g. adding or changing environment variables. +fn clean_and_parse( + args: &[&str], + // Use this to modify the Command generated by the sweep function. + cmd_modifier: impl Fn(&mut Command) -> &mut Command, +) -> Result { let dry_run = args.iter().any(|&f| f == "-d" || f == "--dry-run"); let (remove_msg, clean_msg) = if dry_run { @@ -83,40 +95,59 @@ fn clean_and_parse(target: &TempDir, args: &[&str]) -> Result { ("Successfully removed", "Cleaned ") }; let assertion = - run(sweep(args), target.path()).stdout(contains(remove_msg).and(contains(clean_msg))); + run(cmd_modifier(&mut sweep(args))).stdout(contains(remove_msg).and(contains(clean_msg))); let output = assertion.get_output(); assert!(output.stderr.is_empty()); - // Extract the size from the last line of stdout, example: - // - stdout: "[INFO] Would clean: 9.43 KiB from "/home/user/project/target" - // - extracted amount: "9.43 KiB " - let amount = std::str::from_utf8(&output.stdout)? + // Collect all the lines that contain the "clean message". + let amount_lines: Vec = std::str::from_utf8(&output.stdout)? .lines() - .last() - .unwrap() - .split(clean_msg) - .nth(1) - .unwrap() - .split_inclusive(' ') - .take(2) - .collect::(); - - let cleaned = amount - .parse::() - .context(format!("failed to parse amount {amount}"))? - .to_bytes(); + .filter(|line| line.contains(clean_msg)) + .map(|line| { + line.split(clean_msg) + .nth(1) + .unwrap() + .split_inclusive(' ') + .take(2) + .collect::() + }) + .collect(); + + // Turn all those lines into a collected sum of bytes called `clean`. + let cleaned: u64 = amount_lines + .iter() + .map(|amount| { + amount + .parse::() + .context(format!("failed to parse amount {amount}")) + .unwrap() + .to_bytes() + }) + .sum(); Ok(cleaned) } fn count_cleaned(target: &TempDir, args: &[&str], old_size: u64) -> Result { - let cleaned = clean_and_parse(target, args)?; + let cleaned = clean_and_parse(args, |cmd| cmd.env("CARGO_TARGET_DIR", target.path()))?; + assert_sweeped_size(target.path(), cleaned, old_size)?; + Ok(cleaned) +} - // Make sure this is accurate. - let new_size = get_size(target.path())?; - // Due to rounding and truncation we might have an inexact result. Make sure these are within 1% of each other, - // but don't require an exact match. +/// Assert that the size of the target directory has the expected size +/// (within a small margin of error). +fn assert_sweeped_size( + path: impl AsRef, + // The number of bytes that were sweeped ("cleaned"). + cleaned: u64, + // The size of the target directory (in bytes) before the sweep. + old_size: u64, +) -> Result<()> { + // Make sure that the expected target directory size is accurate. + let new_size = get_size(path)?; + // Due to rounding and truncation we might have an inexact result. Make sure + // these are within 1% of each other, but don't require an exact match. let calculated_size = old_size - cleaned; let diff = new_size.abs_diff(calculated_size); let one_percent = old_size as f64 * 0.01; @@ -124,14 +155,13 @@ fn count_cleaned(target: &TempDir, args: &[&str], old_size: u64) -> Result diff <= one_percent as u64, "new_size={new_size}, old_size={old_size}, cleaned={cleaned}, diff={diff}, 1%={one_percent}", ); - - Ok(cleaned) + Ok(()) } fn count_cleaned_dry_run(target: &TempDir, args: &[&str], old_size: u64) -> Result { let mut args = args.to_vec(); args.push("--dry-run"); - let cleaned = clean_and_parse(target, &args)?; + let cleaned = clean_and_parse(&args, |cmd| cmd.env("CARGO_TARGET_DIR", target.path()))?; let new_size = get_size(target.path())?; assert_eq!(old_size, new_size); @@ -171,7 +201,7 @@ fn stamp_file() -> TestResult { let (size, target) = build("sample-project")?; // Create a stamp file for --file. - let assert = run(sweep(&["--stamp"]), target.path()); + let assert = run(sweep(&["--stamp"]).env("CARGO_TARGET_DIR", target.path())); println!( "{}", std::str::from_utf8(&assert.get_output().stdout).unwrap() @@ -186,7 +216,7 @@ fn stamp_file() -> TestResult { // For some reason, we delete the stamp file after `--file` :( // Recreate it. - run(sweep(&["--stamp"]), target.path()); + run(sweep(&["--stamp"]).env("CARGO_TARGET_DIR", target.path())); let actual_cleaned = count_cleaned(&target, args, size)?; assert_eq!(actual_cleaned, expected_cleaned); @@ -198,10 +228,9 @@ fn stamp_file() -> TestResult { fn empty_project_output() -> TestResult { let (_size, target) = build("sample-project")?; - let assert = run( - sweep(&["--maxsize", "0"]).current_dir(test_dir().join("sample-project")), - target.path(), - ); + let assert = run(sweep(&["--maxsize", "0"]) + .current_dir(test_dir().join("sample-project")) + .env("CARGO_TARGET_DIR", target.path())); let output = std::str::from_utf8(&assert.get_output().stdout).unwrap(); @@ -237,10 +266,9 @@ fn hidden() -> TestResult { // So we can't let cargo-sweep discover any other projects, or it will think they share the same directory as this hidden project. let (size, target) = build("fresh-prefix/.hidden/hidden-project")?; let run = |args| { - run( - sweep(args).current_dir(test_dir().join("fresh-prefix")), - target.path(), - ) + run(sweep(args) + .current_dir(test_dir().join("fresh-prefix")) + .env("CARGO_TARGET_DIR", target.path())) }; run(&["--maxsize", "0", "-r"]); @@ -272,10 +300,9 @@ fn error_output() -> TestResult { } let (_, tempdir) = build("sample-project")?; - let assert = run( - sweep(&["--installed"]).env("PATH", test_dir()), - tempdir.path(), - ); + let assert = run(sweep(&["--installed"]) + .env("PATH", test_dir()) + .env("CARGO_TARGET_DIR", tempdir.path())); assert.stdout(contains("oh no an error")); Ok(()) @@ -299,7 +326,9 @@ fn path() -> TestResult { cmd.arg("sweep").arg("--installed").current_dir(temp_dir()); // Pass `path` as an argument, instead of `current_dir` like it normally is. - let assert = run(cmd.arg(project_dir("sample-project")), target.path()); + let assert = run(cmd + .arg(project_dir("sample-project")) + .env("CARGO_TARGET_DIR", target.path())); assert.stdout(contains("Cleaned")); Ok(()) @@ -310,3 +339,66 @@ fn usage() -> TestResult { trycmd::TestCases::new().case("tests/*.trycmd"); Ok(()) } + +/// This test is a bit more verbose than the other tests because it uses a slightly +/// different mechanism for setting up and sweeping / cleaning. +/// +/// Step-by-step: +/// +/// * Create a new temporary directory. Other tests only output the built files into +/// the target directory, but this test copies the whole template project into the +/// target directory. +/// * Build the two different (nested) projects using cargo build, but without incremental +/// compilation, because cargo-sweep doesn't remove files built by incremental compilation, +/// which affects this test since this test doesn't use a separate target folder for just +/// the target files. +/// * Do a dry run sweep, asserting that nothing was actually cleaned. +/// * Do a proper sweep, asserting that the target directory size is back down to its +/// expected size. +#[test] +fn recursive_multiple_root_workspaces() -> TestResult { + let target = tempdir()?; + + let nested_workspace_dir = test_dir().join("nested-root-workspace"); + let options = CopyOptions::default(); + + // Copy the whole nested-root-workspace folder (and its content) into the new temp dir, + // and then `cargo build` and run the sweep tests inside that directory. + fs_extra::copy_items(&[&nested_workspace_dir], target.path(), &options)?; + + let old_size = get_size(target.path())?; + + // Build bin-crate + run(Command::new(env!("CARGO")) + .env("CARGO_INCREMENTAL", "0") + .arg("build") + .current_dir(target.path().join("nested-root-workspace/bin-crate"))); + + let intermediate_build_size = get_size(target.path())?; + assert!(intermediate_build_size > old_size); + + // Build workspace crates + run(Command::new(env!("CARGO")) + .env("CARGO_INCREMENTAL", "0") + .arg("build") + .current_dir(target.path().join("nested-root-workspace"))); + + let final_build_size = get_size(target.path())?; + assert!(final_build_size > intermediate_build_size); + + // Run a dry-run of cargo-sweep ("clean") in the target directory (recursive) + let args = &["-r", "--time", "0", "--dry-run"]; + let expected_cleaned = clean_and_parse(args, |cmd| cmd.current_dir(target.path()))?; + assert!(expected_cleaned > 0); + let size_after_dry_run_clean = get_size(target.path())?; + // Make sure that nothing was actually cleaned + assert_eq!(final_build_size, size_after_dry_run_clean); + + // Run a proper cargo-sweep ("clean") in the target directory (recursive) + let args = &["-r", "--time", "0"]; + let actual_cleaned = clean_and_parse(args, |cmd| cmd.current_dir(target.path()))?; + assert_sweeped_size(target.path(), actual_cleaned, final_build_size)?; + assert_eq!(actual_cleaned, expected_cleaned); + + Ok(()) +} diff --git a/tests/nested-root-workspace/Cargo.toml b/tests/nested-root-workspace/Cargo.toml new file mode 100644 index 0000000..b4d859d --- /dev/null +++ b/tests/nested-root-workspace/Cargo.toml @@ -0,0 +1,7 @@ +[workspace] +members = [ + "crates/*", +] +exclude = [ + "bin-crate", +] diff --git a/tests/nested-root-workspace/bin-crate/Cargo.toml b/tests/nested-root-workspace/bin-crate/Cargo.toml new file mode 100644 index 0000000..c5f9791 --- /dev/null +++ b/tests/nested-root-workspace/bin-crate/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "bin-crate" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/tests/nested-root-workspace/bin-crate/src/main.rs b/tests/nested-root-workspace/bin-crate/src/main.rs new file mode 100644 index 0000000..93efce5 --- /dev/null +++ b/tests/nested-root-workspace/bin-crate/src/main.rs @@ -0,0 +1,17 @@ +fn main() { + println!("{}", bin_string()); +} + +fn bin_string() -> String { + "This is bin-crate".to_string() +} + +#[cfg(test)] +mod test { + use super::bin_string; + + #[test] + fn test() { + assert_eq!("This is bin-crate".to_string(), bin_string()); + } +} diff --git a/tests/nested-root-workspace/crates/crate-one/Cargo.toml b/tests/nested-root-workspace/crates/crate-one/Cargo.toml new file mode 100644 index 0000000..df8e0fc --- /dev/null +++ b/tests/nested-root-workspace/crates/crate-one/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "crate-one" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/tests/nested-root-workspace/crates/crate-one/src/lib.rs b/tests/nested-root-workspace/crates/crate-one/src/lib.rs new file mode 100644 index 0000000..8f3bba6 --- /dev/null +++ b/tests/nested-root-workspace/crates/crate-one/src/lib.rs @@ -0,0 +1,13 @@ +pub fn crate_one_string() -> String { + "This is crate-one".to_string() +} + +#[cfg(test)] +mod test { + use super::crate_one_string; + + #[test] + fn test() { + assert_eq!("This is crate-one".to_string(), crate_one_string()); + } +} diff --git a/tests/nested-root-workspace/crates/crate-two/Cargo.toml b/tests/nested-root-workspace/crates/crate-two/Cargo.toml new file mode 100644 index 0000000..df1daae --- /dev/null +++ b/tests/nested-root-workspace/crates/crate-two/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "crate-two" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/tests/nested-root-workspace/crates/crate-two/src/lib.rs b/tests/nested-root-workspace/crates/crate-two/src/lib.rs new file mode 100644 index 0000000..f9aaf7e --- /dev/null +++ b/tests/nested-root-workspace/crates/crate-two/src/lib.rs @@ -0,0 +1,13 @@ +pub fn crate_two_string() -> String { + "This is crate-two".to_string() +} + +#[cfg(test)] +mod test { + use super::crate_two_string; + + #[test] + fn test() { + assert_eq!("This is crate-two".to_string(), crate_two_string()); + } +}