From f7f7abccf3d060a055628fbf5a66d97540f76c84 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 31 Oct 2024 16:25:55 -0500 Subject: [PATCH 1/2] test(gc): Update remaining unordered tests to snapbox --- tests/testsuite/global_cache_tracker.rs | 111 +++++++++++++----------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/tests/testsuite/global_cache_tracker.rs b/tests/testsuite/global_cache_tracker.rs index e7f9add7077..8e812fc7256 100644 --- a/tests/testsuite/global_cache_tracker.rs +++ b/tests/testsuite/global_cache_tracker.rs @@ -18,6 +18,7 @@ use cargo::core::global_cache_tracker::{self, DeferredGlobalLastUse, GlobalCache use cargo::util::cache_lock::CacheLockMode; use cargo::util::interning::InternedString; use cargo::GlobalContext; +use cargo_test_support::compare::assert_e2e; use cargo_test_support::paths; use cargo_test_support::prelude::*; use cargo_test_support::registry::{Package, RegistryBuilder}; @@ -749,7 +750,6 @@ fn both_git_and_http_index_cleans() { drop(lock); } -#[expect(deprecated)] #[cargo_test] fn clean_gc_dry_run() { // Basic `clean --gc --dry-run` test. @@ -772,16 +772,19 @@ fn clean_gc_dry_run() { let index = glob_registry("index").ls_r(); let src = glob_registry("src").ls_r(); let cache = glob_registry("cache").ls_r(); - let expected_files = index + let mut expected_files = index .iter() .chain(src.iter()) .chain(cache.iter()) .map(|p| p.to_str().unwrap()) .join("\n"); + expected_files.push_str("\n"); + let expected_files = snapbox::filter::normalize_paths(&expected_files); + let expected_files = assert_e2e().redactions().redact(&expected_files); p.cargo("clean gc --dry-run -v -Zgc") .masquerade_as_nightly_cargo(&["gc"]) - .with_stdout_unordered(&expected_files) + .with_stdout_data(expected_files.as_str().unordered()) .with_stderr_data(str![[r#" [SUMMARY] [FILE_NUM] files, [FILE_SIZE]B total [WARNING] no files deleted due to --dry-run @@ -792,7 +795,7 @@ fn clean_gc_dry_run() { // Again, make sure the information is still tracked. p.cargo("clean gc --dry-run -v -Zgc") .masquerade_as_nightly_cargo(&["gc"]) - .with_stdout_unordered(&expected_files) + .with_stdout_data(expected_files.as_str().unordered()) .with_stderr_data(str![[r#" [SUMMARY] [FILE_NUM] files, [FILE_SIZE]B total [WARNING] no files deleted due to --dry-run @@ -895,7 +898,6 @@ fn tracks_sizes() { assert!(db_sizes[1] > 26000); } -#[expect(deprecated)] #[cargo_test] fn max_size() { // Checks --max-crate-size and --max-src-size with various cleaning thresholds. @@ -924,20 +926,20 @@ fn max_size() { .collect(); // This exercises the different boundary conditions. - for (clean_size, files, bytes) in [ - (22, 0, 0), - (21, 1, 6), - (16, 1, 6), - (15, 2, 8), - (14, 2, 8), - (13, 3, 9), - (12, 4, 12), - (10, 4, 12), - (9, 5, 16), - (6, 5, 16), - (5, 6, 21), - (1, 6, 21), - (0, 7, 22), + for (clean_size, files) in [ + (22, 0), + (21, 1), + (16, 1), + (15, 2), + (14, 2), + (13, 3), + (12, 4), + (10, 4), + (9, 5), + (6, 5), + (5, 6), + (1, 6), + (0, 7), ] { let (removed, kept) = names_by_timestamp.split_at(files); // --max-crate-size @@ -947,19 +949,21 @@ fn max_size() { writeln!(stderr, "[REMOVING] [..]{name}.crate").unwrap(); } let total_display = if removed.is_empty() { - String::new() + "" } else { - format!(", {bytes}B total") + ", [FILE_SIZE]B total" }; - let files_display = if files == 1 { - format!("1 file") + let files_display = if files == 0 { + "0 files" + } else if files == 1 { + "1 file" } else { - format!("{files} files") + "[FILE_NUM] files" }; - write!(stderr, "[REMOVED] {files_display}{total_display}").unwrap(); + writeln!(stderr, "[REMOVED] {files_display}{total_display}").unwrap(); cargo_process(&format!("clean gc -Zgc -v --max-crate-size={clean_size}")) .masquerade_as_nightly_cargo(&["gc"]) - .with_stderr_unordered(&stderr) + .with_stderr_data(stderr.unordered()) .run(); for name in kept { assert!(cache_dir.join(format!("{name}.crate")).exists()); @@ -974,15 +978,15 @@ fn max_size() { for name in removed { writeln!(stderr, "[REMOVING] [..]{name}").unwrap(); } - let total_display = if files == 0 { - String::new() + let total_display = if removed.is_empty() { + "" } else { - format!(", {bytes}B total") + ", [FILE_SIZE]B total" }; - write!(stderr, "[REMOVED] {files_display}{total_display}").unwrap(); + writeln!(stderr, "[REMOVED] {files_display}{total_display}").unwrap(); cargo_process(&format!("clean gc -Zgc -v --max-src-size={clean_size}")) .masquerade_as_nightly_cargo(&["gc"]) - .with_stderr_unordered(&stderr) + .with_stderr_data(stderr.unordered()) .run(); for name in kept { assert!(src_dir.join(name).exists()); @@ -1122,7 +1126,6 @@ fn max_size_untracked_src_from_clean() { max_size_untracked_verify(&gctx); } -#[expect(deprecated)] #[cargo_test] fn max_download_size() { // --max-download-size @@ -1140,13 +1143,13 @@ fn max_download_size() { ("b-1.0.0", 1, 1, 7), ]; - for (max_size, num_deleted, files_deleted, bytes) in [ - (30, 0, 0, 0), - (29, 1, 1, 5), - (24, 2, 2, 9), - (20, 3, 3, 12), - (1, 7, 7, 29), - (0, 8, 8, 30), + for (max_size, num_deleted, files_deleted) in [ + (30, 0, 0), + (29, 1, 1), + (24, 2, 2), + (20, 3, 3), + (1, 7, 7), + (0, 8, 8), ] { populate_cache(&gctx, &test_crates); // Determine the order things will be deleted. @@ -1159,20 +1162,22 @@ fn max_download_size() { for name in removed { writeln!(stderr, "[REMOVING] [..]{name}").unwrap(); } - let files_display = if files_deleted == 1 { - format!("1 file") + let files_display = if files_deleted == 0 { + "0 files" + } else if files_deleted == 1 { + "1 file" } else { - format!("{files_deleted} files") + "[FILE_NUM] files" }; let total_display = if removed.is_empty() { - String::new() + "" } else { - format!(", {bytes}B total") + ", [FILE_SIZE]B total" }; - write!(stderr, "[REMOVED] {files_display}{total_display}",).unwrap(); + writeln!(stderr, "[REMOVED] {files_display}{total_display}",).unwrap(); cargo_process(&format!("clean gc -Zgc -v --max-download-size={max_size}")) .masquerade_as_nightly_cargo(&["gc"]) - .with_stderr_unordered(&stderr) + .with_stderr_data(stderr.unordered()) .run(); } } @@ -1683,7 +1688,6 @@ fn clean_max_src_crate_age() { .run(); } -#[expect(deprecated)] #[cargo_test] fn clean_max_git_size() { // clean --max-git-size @@ -1767,13 +1771,16 @@ fn clean_max_git_size() { // And then try cleaning everything. p.cargo("clean gc --max-git-size=0 -Zgc -v") .masquerade_as_nightly_cargo(&["gc"]) - .with_stderr_unordered(&format!( - "\ -[REMOVING] [ROOT]/home/.cargo/git/checkouts/{db_name}/{second_co_name} -[REMOVING] [ROOT]/home/.cargo/git/db/{db_name} + .with_stderr_data( + format!( + "\ +[REMOVING] [ROOT]/home/.cargo/git/checkouts/bar-[HASH]/{second_co_name} +[REMOVING] [ROOT]/home/.cargo/git/db/bar-[HASH] [REMOVED] [..] " - )) + ) + .unordered(), + ) .run(); } From bfa097e5d554ce8ffeb12da7f964813cd05bbcae Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 5 Nov 2024 10:39:30 -0600 Subject: [PATCH 2/2] fix(test)!: Remove unused with_stdout_unordered,with_stderr_unordered --- crates/cargo-test-support/src/compare.rs | 52 +----------------------- crates/cargo-test-support/src/diff.rs | 49 ---------------------- crates/cargo-test-support/src/lib.rs | 50 ----------------------- 3 files changed, 1 insertion(+), 150 deletions(-) delete mode 100644 crates/cargo-test-support/src/diff.rs diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index 36bbb2aedab..028b58ce2a6 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -43,7 +43,7 @@ use crate::cross_compile::try_alternate; use crate::paths; -use crate::{diff, rustc_host}; +use crate::rustc_host; use anyhow::{bail, Result}; use snapbox::Data; use snapbox::IntoData; @@ -431,56 +431,6 @@ fn substitute_macros(input: &str) -> String { result } -/// Checks that the given string contains the given lines, ignoring the order -/// of the lines. -/// -/// See [Patterns](index.html#patterns) for more information on pattern matching. -pub(crate) fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { - let expected = normalize_expected(expected, cwd); - let actual = normalize_actual(actual, cwd); - let e: Vec<_> = expected.lines().map(|line| WildStr::new(line)).collect(); - let mut a: Vec<_> = actual.lines().map(|line| WildStr::new(line)).collect(); - // match more-constrained lines first, although in theory we'll - // need some sort of recursive match here. This handles the case - // that you expect "a\n[..]b" and two lines are printed out, - // "ab\n"a", where technically we do match unordered but a naive - // search fails to find this. This simple sort at least gets the - // test suite to pass for now, but we may need to get more fancy - // if tests start failing again. - a.sort_by_key(|s| s.line.len()); - let mut changes = Vec::new(); - let mut a_index = 0; - let mut failure = false; - - use crate::diff::Change; - for (e_i, e_line) in e.into_iter().enumerate() { - match a.iter().position(|a_line| e_line == *a_line) { - Some(index) => { - let a_line = a.remove(index); - changes.push(Change::Keep(e_i, index, a_line)); - a_index += 1; - } - None => { - failure = true; - changes.push(Change::Remove(e_i, e_line)); - } - } - } - for unmatched in a { - failure = true; - changes.push(Change::Add(a_index, unmatched)); - a_index += 1; - } - if failure { - bail!( - "Expected lines did not match (ignoring order):\n{}\n", - diff::render_colored_changes(&changes) - ); - } else { - Ok(()) - } -} - /// Checks that the given string contains the given contiguous lines /// somewhere. /// diff --git a/crates/cargo-test-support/src/diff.rs b/crates/cargo-test-support/src/diff.rs deleted file mode 100644 index 8b5e109be45..00000000000 --- a/crates/cargo-test-support/src/diff.rs +++ /dev/null @@ -1,49 +0,0 @@ -//! A simple Myers diff implementation. -//! -//! This focuses on being short and simple, and the expense of being -//! inefficient. A key characteristic here is that this supports cargotest's -//! `[..]` wildcard matching. That means things like hashing can't be used. -//! Since Cargo's output tends to be small, this should be sufficient. - -use std::fmt; -use std::io::Write; - -/// A single line change to be applied to the original. -#[derive(Debug, Eq, PartialEq)] -pub enum Change { - Add(usize, T), - Remove(usize, T), - Keep(usize, usize, T), -} - -pub fn render_colored_changes(changes: &[Change]) -> String { - // anstyle is not very ergonomic, but I don't want to bring in another dependency. - let red = anstyle::AnsiColor::Red.on_default().render(); - let green = anstyle::AnsiColor::Green.on_default().render(); - let dim = (anstyle::Style::new() | anstyle::Effects::DIMMED).render(); - let bold = (anstyle::Style::new() | anstyle::Effects::BOLD).render(); - let reset = anstyle::Reset.render(); - - let choice = if crate::is_ci() { - // Don't use color on CI. Even though GitHub can display colors, it - // makes reading the raw logs more difficult. - anstream::ColorChoice::Never - } else { - anstream::AutoStream::choice(&std::io::stdout()) - }; - let mut buffer = anstream::AutoStream::new(Vec::new(), choice); - - for change in changes { - let (nums, sign, color, text) = match change { - Change::Add(i, s) => (format!(" {:<4} ", i), '+', green, s), - Change::Remove(i, s) => (format!("{:<4} ", i), '-', red, s), - Change::Keep(x, y, s) => (format!("{:<4}{:<4} ", x, y), ' ', dim, s), - }; - writeln!( - buffer, - "{dim}{nums}{reset}{bold}{sign}{reset}{color}{text}{reset}" - ) - .unwrap(); - } - String::from_utf8(buffer.into_inner()).unwrap() -} diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index b779f5605e7..55ca175710e 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -105,7 +105,6 @@ pub use cargo_test_macro::cargo_test; pub mod compare; pub mod containers; pub mod cross_compile; -mod diff; pub mod git; pub mod install; pub mod paths; @@ -651,8 +650,6 @@ pub struct Execs { expect_stderr_contains: Vec, expect_stdout_not_contains: Vec, expect_stderr_not_contains: Vec, - expect_stdout_unordered: Vec, - expect_stderr_unordered: Vec, expect_stderr_with_without: Vec<(Vec, Vec)>, stream_output: bool, assert: snapbox::Assert, @@ -751,43 +748,6 @@ impl Execs { self } - /// Verifies that all of the stdout output is equal to the given lines, - /// ignoring the order of the lines. - /// - /// See [`Execs::with_stderr_unordered`] for more details. - #[deprecated(note = "replaced with `Execs::with_stdout_data(expected.unordered())`")] - pub fn with_stdout_unordered(&mut self, expected: S) -> &mut Self { - self.expect_stdout_unordered.push(expected.to_string()); - self - } - - /// Verifies that all of the stderr output is equal to the given lines, - /// ignoring the order of the lines. - /// - /// See [`compare`] for supported patterns. - /// - /// This is useful when checking the output of `cargo build -v` since - /// the order of the output is not always deterministic. - /// Recommend use `with_stderr_contains` instead unless you really want to - /// check *every* line of output. - /// - /// Be careful when using patterns such as `[..]`, because you may end up - /// with multiple lines that might match, and this is not smart enough to - /// do anything like longest-match. For example, avoid something like: - /// - /// ```text - /// [RUNNING] `rustc [..] - /// [RUNNING] `rustc --crate-name foo [..] - /// ``` - /// - /// This will randomly fail if the other crate name is `bar`, and the - /// order changes. - #[deprecated(note = "replaced with `Execs::with_stderr_data(expected.unordered())`")] - pub fn with_stderr_unordered(&mut self, expected: S) -> &mut Self { - self.expect_stderr_unordered.push(expected.to_string()); - self - } - /// Verify that a particular line appears in stderr with and without the /// given substrings. Exactly one line must match. /// @@ -993,8 +953,6 @@ impl Execs { && self.expect_stderr_contains.is_empty() && self.expect_stdout_not_contains.is_empty() && self.expect_stderr_not_contains.is_empty() - && self.expect_stdout_unordered.is_empty() - && self.expect_stderr_unordered.is_empty() && self.expect_stderr_with_without.is_empty() { panic!( @@ -1107,12 +1065,6 @@ impl Execs { for expect in self.expect_stderr_not_contains.iter() { compare::match_does_not_contain(expect, stderr, cwd)?; } - for expect in self.expect_stdout_unordered.iter() { - compare::match_unordered(expect, stdout, cwd)?; - } - for expect in self.expect_stderr_unordered.iter() { - compare::match_unordered(expect, stderr, cwd)?; - } for (with, without) in self.expect_stderr_with_without.iter() { compare::match_with_without(stderr, with, without, cwd)?; } @@ -1141,8 +1093,6 @@ pub fn execs() -> Execs { expect_stderr_contains: Vec::new(), expect_stdout_not_contains: Vec::new(), expect_stderr_not_contains: Vec::new(), - expect_stdout_unordered: Vec::new(), - expect_stderr_unordered: Vec::new(), expect_stderr_with_without: Vec::new(), stream_output: false, assert: compare::assert_e2e(),