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/src/util.rs b/src/util.rs index a87eee2..46924bb 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,6 +1,6 @@ /// Formats a number of bytes into the closest binary SI unit, i.e KiB, MiB etc. pub fn format_bytes(bytes: u64) -> String { - let prefixes = ["bytes", "KiB", "MiB", "GiB", "TiB"]; + let prefixes = ["B", "KiB", "MiB", "GiB", "TiB"]; let mut bytes = bytes as f64; for prefix in prefixes.iter() { if bytes < 1024. { @@ -18,7 +18,26 @@ mod tests { #[test] fn test_format_bytes() { assert_eq!(format_bytes(1024), "1.00 KiB"); - assert_eq!(format_bytes(1023), "1023.00 bytes"); + assert_eq!(format_bytes(1023), "1023.00 B"); assert_eq!(format_bytes(500 * 1024 * 1024), "500.00 MiB"); + + // Assert that the human-size crate can parse the output from cargo-sweep + assert_eq!("1.00 B".parse::().unwrap().to_bytes(), 1); + assert_eq!( + "1.00 KiB".parse::().unwrap().to_bytes(), + 1024 + ); + assert_eq!( + "1.00 MiB".parse::().unwrap().to_bytes(), + 1024 * 1024 + ); + assert_eq!( + "1.00 GiB".parse::().unwrap().to_bytes(), + 1024 * 1024 * 1024 + ); + assert_eq!( + "1.00 TiB".parse::().unwrap().to_bytes(), + 1024 * 1024 * 1024 * 1024 + ); } } diff --git a/tests/integration.rs b/tests/integration.rs index 6313006..794a419 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 { @@ -36,9 +39,9 @@ fn project_dir(project: &str) -> PathBuf { test_dir().join(project) } -fn cargo(project: &str) -> Command { +fn cargo(cmd_current_dir: impl AsRef) -> Command { let mut cmd = Command::new(env!("CARGO")); - cmd.current_dir(project_dir(project)); + cmd.current_dir(cmd_current_dir); cmd } @@ -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_dir(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,13 +228,14 @@ 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(); + // Please note: The output from the sweep command is platform dependent. The regular + // expression tries to take that into account by letting the file output order vary. let pattern = unindent( r#"\[DEBUG\] cleaning: ".+" with remove_older_until_fits \[DEBUG\] size_to_remove: .+ @@ -219,7 +250,7 @@ fn empty_project_output() -> TestResult { \[DEBUG\] Successfully removed: ".+debug.+deps.+sample_project.+" \[DEBUG\] Successfully removed: ".+debug.+deps.+sample_project.+" \[DEBUG\] Successfully removed: ".+debug.+deps.+sample_project.+" - \[DEBUG\] Successfully removed: ".+.fingerprint.+sample-project-.+" + \[DEBUG\] Successfully removed: ".+debug.+fingerprint.+sample-project.+" \[INFO\] Cleaned .+ from ".+""#, ); @@ -237,10 +268,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 +302,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 +328,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 +341,115 @@ 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. +/// * 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. +/// +/// Please note that there is some ceremony involved, namely correctly setting +/// `CARGO_TARGET_DIR` to `unset`, so as not to clash with users who use `CARGO_TARGET_DIR` +/// when they invoke `cargo test` for running these tests. +#[test] +fn recursive_multiple_root_workspaces() -> TestResult { + let temp_workspace_dir = 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], + temp_workspace_dir.path(), + &options, + )?; + + let old_size = get_size(temp_workspace_dir.path())?; + + // Build bin-crate + run(cargo( + temp_workspace_dir + .path() + .join("nested-root-workspace/bin-crate"), + ) + // If someone has built & run these tests with CARGO_TARGET_DIR, + // we need to override that. + .env_remove("CARGO_TARGET_DIR") + .arg("build")); + + let intermediate_build_size = get_size(temp_workspace_dir.path())?; + assert!(intermediate_build_size > old_size); + + // Build workspace crates + run( + cargo(temp_workspace_dir.path().join("nested-root-workspace")) + // If someone has built & run these tests with CARGO_TARGET_DIR, + // we need to override that. + .env_remove("CARGO_TARGET_DIR") + .arg("build"), + ); + + let final_build_size = get_size(temp_workspace_dir.path())?; + assert!(final_build_size > intermediate_build_size); + + // Measure the size of the nested root workspace and the bin crate before cargo-sweep is invoked. + let pre_clean_size_nested_root_workspace = + get_size(temp_workspace_dir.path().join("nested-root-workspace"))?; + let pre_clean_size_bin_create = get_size( + temp_workspace_dir + .path() + .join("nested-root-workspace/bin-crate"), + )?; + + // 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| { + // If someone has built & run these tests with CARGO_TARGET_DIR, + // we need to override that. + cmd.env_remove("CARGO_TARGET_DIR") + .current_dir(temp_workspace_dir.path()) + })?; + assert!(expected_cleaned > 0); + let size_after_dry_run_clean = get_size(temp_workspace_dir.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| { + // If someone has built & run these tests with CARGO_TARGET_DIR, + // we need to override that. + cmd.env_remove("CARGO_TARGET_DIR") + .current_dir(temp_workspace_dir.path()) + })?; + assert_sweeped_size(temp_workspace_dir.path(), actual_cleaned, final_build_size)?; + assert_eq!(actual_cleaned, expected_cleaned); + + // Measure the size of the nested root workspace and the bin crate after cargo-sweep is invoked, + // and assert that both of their sizes have been reduced. + // This works because by default cargo generates the `target/` directory in a sub-directory + // of the package root. + let post_clean_size_nested_root_workspace = + get_size(temp_workspace_dir.path().join("nested-root-workspace"))?; + let post_clean_size_bin_crate = get_size( + temp_workspace_dir + .path() + .join("nested-root-workspace/bin-crate"), + )?; + assert!(post_clean_size_nested_root_workspace < pre_clean_size_nested_root_workspace, "The size of the nested root workspace create has not been reduced after running cargo-sweep."); + assert!( + post_clean_size_bin_crate < pre_clean_size_bin_create, + "The size of the bin create has not been reduced after running cargo-sweep." + ); + + 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()); + } +}