From b1ead82bc333b73e8ca5158fba393c8343fd8572 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Nov 2024 20:03:53 -0600 Subject: [PATCH] fix(test): Make redactions consistent with snapbox I'm unsure how we should be replacing these use cases, so I'm exploring keeping them but making them use snapbox under the hood. Part of the intent of snapbox is that it provides you the building blocks to make what you need. --- Cargo.lock | 8 +- Cargo.toml | 2 +- crates/cargo-test-support/src/compare.rs | 240 +++-------------------- crates/cargo-test-support/src/lib.rs | 15 +- tests/testsuite/artifact_dep.rs | 77 ++++---- tests/testsuite/check_cfg.rs | 2 +- 6 files changed, 71 insertions(+), 273 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1d3d0db6c69c..0834cb961d0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -987,9 +987,9 @@ dependencies = [ [[package]] name = "escargot" -version = "0.5.11" +version = "0.5.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "650eb5f6eeda986377996e9ed570cbc20cc16d30440696f82f129c863e4e3e83" +checksum = "05a3ac187a16b5382fef8c69fd1bad123c67b7cf3932240a2d43dcdd32cded88" dependencies = [ "log", "once_cell", @@ -3375,9 +3375,9 @@ checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" [[package]] name = "snapbox" -version = "0.6.18" +version = "0.6.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ba434818a8a9b1b106404288d6bd75a94348aae8fc9a518b211b609a36a54bc" +checksum = "1373ce406dfad473059bbc31d807715642182bbc952a811952b58d1c9e41dcfa" dependencies = [ "anstream", "anstyle", diff --git a/Cargo.toml b/Cargo.toml index 3a0bb6fe0fdf..3245a05ca0b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -96,7 +96,7 @@ sha2 = "0.10.8" shell-escape = "0.1.5" similar = "2.6.0" supports-hyperlinks = "3.0.0" -snapbox = { version = "0.6.18", features = ["diff", "dir", "term-svg", "regex", "json"] } +snapbox = { version = "0.6.20", features = ["diff", "dir", "term-svg", "regex", "json"] } tar = { version = "0.4.42", default-features = false } tempfile = "3.10.1" thiserror = "1.0.63" diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index 028b58ce2a66..848468cf97bd 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -51,7 +51,6 @@ use std::fmt; use std::path::Path; use std::path::PathBuf; use std::str; -use url::Url; /// This makes it easier to write regex replacements that are guaranteed to only /// get compiled once @@ -333,111 +332,37 @@ static E2E_LITERAL_REDACTIONS: &[(&str, &str)] = &[ ]; /// Normalizes the output so that it can be compared against the expected value. -fn normalize_actual(actual: &str, cwd: Option<&Path>) -> String { - // It's easier to read tabs in outputs if they don't show up as literal - // hidden characters - let actual = actual.replace('\t', ""); - if cfg!(windows) { - // Let's not deal with \r\n vs \n on windows... - let actual = actual.replace('\r', ""); - normalize_windows(&actual, cwd) - } else { - actual - } +fn normalize_actual(content: &str, redactions: &snapbox::Redactions) -> String { + use snapbox::filter::Filter as _; + let content = snapbox::filter::FilterPaths.filter(content.into_data()); + let content = snapbox::filter::FilterNewlines.filter(content); + let content = content.render().expect("came in as a String"); + let content = redactions.redact(&content); + content } /// Normalizes the expected string so that it can be compared against the actual output. -fn normalize_expected(expected: &str, cwd: Option<&Path>) -> String { - let expected = replace_dirty_msvc(expected); - let expected = substitute_macros(&expected); - - if cfg!(windows) { - normalize_windows(&expected, cwd) - } else { - let expected = match cwd { - None => expected, - Some(cwd) => expected.replace("[CWD]", &cwd.display().to_string()), - }; - let expected = expected.replace("[ROOT]", &paths::root().display().to_string()); - expected - } -} - -fn replace_dirty_msvc_impl(s: &str, is_msvc: bool) -> String { - if is_msvc { - s.replace("[DIRTY-MSVC]", "[DIRTY]") - } else { - use itertools::Itertools; - - let mut new = s - .lines() - .filter(|it| !it.starts_with("[DIRTY-MSVC]")) - .join("\n"); - - if s.ends_with("\n") { - new.push_str("\n"); - } - - new - } -} - -fn replace_dirty_msvc(s: &str) -> String { - replace_dirty_msvc_impl(s, cfg!(target_env = "msvc")) -} - -/// Normalizes text for both actual and expected strings on Windows. -fn normalize_windows(text: &str, cwd: Option<&Path>) -> String { - // Let's not deal with / vs \ (windows...) - let text = text.replace('\\', "/"); - - // Weirdness for paths on Windows extends beyond `/` vs `\` apparently. - // Namely paths like `c:\` and `C:\` are equivalent and that can cause - // issues. The return value of `env::current_dir()` may return a - // lowercase drive name, but we round-trip a lot of values through `Url` - // which will auto-uppercase the drive name. To just ignore this - // distinction we try to canonicalize as much as possible, taking all - // forms of a path and canonicalizing them to one. - let replace_path = |s: &str, path: &Path, with: &str| { - let path_through_url = Url::from_file_path(path).unwrap().to_file_path().unwrap(); - let path1 = path.display().to_string().replace('\\', "/"); - let path2 = path_through_url.display().to_string().replace('\\', "/"); - s.replace(&path1, with) - .replace(&path2, with) - .replace(with, &path1) - }; - - let text = match cwd { - None => text, - Some(p) => replace_path(&text, p, "[CWD]"), - }; - - // Similar to cwd above, perform similar treatment to the root path - // which in theory all of our paths should otherwise get rooted at. - let root = paths::root(); - let text = replace_path(&text, &root, "[ROOT]"); - - text -} - -fn substitute_macros(input: &str) -> String { - let mut result = input.to_owned(); - for &(pat, subst) in MIN_LITERAL_REDACTIONS { - result = result.replace(pat, subst) - } - for &(pat, subst) in E2E_LITERAL_REDACTIONS { - result = result.replace(pat, subst) - } - result +fn normalize_expected(content: &str, redactions: &snapbox::Redactions) -> String { + use snapbox::filter::Filter as _; + let content = snapbox::filter::FilterPaths.filter(content.into_data()); + let content = snapbox::filter::FilterNewlines.filter(content); + // Remove any conditionally absent redactions like `[EXE]` + let content = content.render().expect("came in as a String"); + let content = redactions.clear_unused(&content); + content.into_owned() } /// Checks that the given string contains the given contiguous lines /// somewhere. /// /// See [Patterns](index.html#patterns) for more information on pattern matching. -pub(crate) fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { - let expected = normalize_expected(expected, cwd); - let actual = normalize_actual(actual, cwd); +pub(crate) fn match_contains( + expected: &str, + actual: &str, + redactions: &snapbox::Redactions, +) -> Result<()> { + let expected = normalize_expected(expected, redactions); + let actual = normalize_actual(actual, redactions); let e: Vec<_> = expected.lines().map(|line| WildStr::new(line)).collect(); let a: Vec<_> = actual.lines().map(|line| WildStr::new(line)).collect(); if e.len() == 0 { @@ -465,9 +390,9 @@ pub(crate) fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) - pub(crate) fn match_does_not_contain( expected: &str, actual: &str, - cwd: Option<&Path>, + redactions: &snapbox::Redactions, ) -> Result<()> { - if match_contains(expected, actual, cwd).is_ok() { + if match_contains(expected, actual, redactions).is_ok() { bail!( "expected not to find:\n\ {}\n\n\ @@ -492,10 +417,10 @@ pub(crate) fn match_with_without( actual: &str, with: &[String], without: &[String], - cwd: Option<&Path>, + redactions: &snapbox::Redactions, ) -> Result<()> { - let actual = normalize_actual(actual, cwd); - let norm = |s: &String| format!("[..]{}[..]", normalize_expected(s, cwd)); + let actual = normalize_actual(actual, redactions); + let norm = |s: &String| format!("[..]{}[..]", normalize_expected(s, redactions)); let with: Vec<_> = with.iter().map(norm).collect(); let without: Vec<_> = without.iter().map(norm).collect(); let with_wild: Vec<_> = with.iter().map(|w| WildStr::new(w)).collect(); @@ -748,117 +673,6 @@ mod test { } } - #[test] - fn dirty_msvc() { - let case = |expected: &str, wild: &str, msvc: bool| { - assert_eq!(expected, &replace_dirty_msvc_impl(wild, msvc)); - }; - - // no replacements - case("aa", "aa", false); - case("aa", "aa", true); - - // with replacements - case( - "\ -[DIRTY] a", - "\ -[DIRTY-MSVC] a", - true, - ); - case( - "", - "\ -[DIRTY-MSVC] a", - false, - ); - case( - "\ -[DIRTY] a -[COMPILING] a", - "\ -[DIRTY-MSVC] a -[COMPILING] a", - true, - ); - case( - "\ -[COMPILING] a", - "\ -[DIRTY-MSVC] a -[COMPILING] a", - false, - ); - - // test trailing newline behavior - case( - "\ -A -B -", "\ -A -B -", true, - ); - - case( - "\ -A -B -", "\ -A -B -", false, - ); - - case( - "\ -A -B", "\ -A -B", true, - ); - - case( - "\ -A -B", "\ -A -B", false, - ); - - case( - "\ -[DIRTY] a -", - "\ -[DIRTY-MSVC] a -", - true, - ); - case( - "\n", - "\ -[DIRTY-MSVC] a -", - false, - ); - - case( - "\ -[DIRTY] a", - "\ -[DIRTY-MSVC] a", - true, - ); - case( - "", - "\ -[DIRTY-MSVC] a", - false, - ); - } - #[test] fn redact_elapsed_time() { let mut subs = snapbox::Redactions::new(); diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 55ca175710ea..964b72502a78 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -820,10 +820,6 @@ impl Execs { self } - fn get_cwd(&self) -> Option<&Path> { - self.process_builder.as_ref().and_then(|p| p.get_cwd()) - } - pub fn env>(&mut self, key: &str, val: T) -> &mut Self { if let Some(ref mut p) = self.process_builder { p.env(key, val); @@ -1021,7 +1017,6 @@ impl Execs { self.verify_checks_output(stdout, stderr); let stdout = std::str::from_utf8(stdout).expect("stdout is not utf8"); let stderr = std::str::from_utf8(stderr).expect("stderr is not utf8"); - let cwd = self.get_cwd(); match self.expect_exit_code { None => {} @@ -1054,19 +1049,19 @@ impl Execs { } } for expect in self.expect_stdout_contains.iter() { - compare::match_contains(expect, stdout, cwd)?; + compare::match_contains(expect, stdout, self.assert.redactions())?; } for expect in self.expect_stderr_contains.iter() { - compare::match_contains(expect, stderr, cwd)?; + compare::match_contains(expect, stderr, self.assert.redactions())?; } for expect in self.expect_stdout_not_contains.iter() { - compare::match_does_not_contain(expect, stdout, cwd)?; + compare::match_does_not_contain(expect, stdout, self.assert.redactions())?; } for expect in self.expect_stderr_not_contains.iter() { - compare::match_does_not_contain(expect, stderr, cwd)?; + compare::match_does_not_contain(expect, stderr, self.assert.redactions())?; } for (with, without) in self.expect_stderr_with_without.iter() { - compare::match_with_without(stderr, with, without, cwd)?; + compare::match_with_without(stderr, with, without, self.assert.redactions())?; } Ok(()) } diff --git a/tests/testsuite/artifact_dep.rs b/tests/testsuite/artifact_dep.rs index 7b438d310548..ffb4512ddda5 100644 --- a/tests/testsuite/artifact_dep.rs +++ b/tests/testsuite/artifact_dep.rs @@ -1120,23 +1120,19 @@ fn build_script_deps_adopt_specified_target_unconditionally() { #[expect(deprecated)] p.cargo("check -v -Z bindeps") .masquerade_as_nightly_cargo(&["bindeps"]) - .with_stderr_does_not_contain(format!( - "[RUNNING] `rustc --crate-name build_script_build --edition=2015 build.rs [..]--target {} [..]", - target - )) + .with_stderr_does_not_contain( + "[RUNNING] `rustc --crate-name build_script_build --edition=2015 build.rs [..]--target [ALT_TARGET] [..]", + ) .with_stderr_contains("[RUNNING] `rustc --crate-name build_script_build --edition=2015 build.rs [..]") - .with_stderr_contains(format!( - "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/lib.rs [..]--target {} [..]", - target - )) - .with_stderr_contains(format!( - "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/main.rs [..]--target {} [..]", - target - )) - .with_stderr_does_not_contain(format!( - "[RUNNING] `rustc --crate-name foo [..]--target {} [..]", - target - )) + .with_stderr_contains( + "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/lib.rs [..]--target [ALT_TARGET] [..]", + ) + .with_stderr_contains( + "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/main.rs [..]--target [ALT_TARGET] [..]", + ) + .with_stderr_does_not_contain( + "[RUNNING] `rustc --crate-name foo [..]--target [ALT_TARGET] [..]", + ) .with_stderr_contains("[RUNNING] `rustc --crate-name foo [..]") .run(); } @@ -1240,18 +1236,15 @@ fn non_build_script_deps_adopt_specified_target_unconditionally() { #[expect(deprecated)] p.cargo("check -v -Z bindeps") .masquerade_as_nightly_cargo(&["bindeps"]) - .with_stderr_contains(format!( - "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/lib.rs [..]--target {} [..]", - target - )) - .with_stderr_contains(format!( - "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/main.rs [..]--target {} [..]", - target - )) - .with_stderr_does_not_contain(format!( - "[RUNNING] `rustc --crate-name foo [..]--target {} [..]", - target - )) + .with_stderr_contains( + "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/lib.rs [..]--target [ALT_TARGET] [..]", + ) + .with_stderr_contains( + "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/main.rs [..]--target [ALT_TARGET] [..]", + ) + .with_stderr_does_not_contain( + "[RUNNING] `rustc --crate-name foo [..]--target [ALT_TARGET] [..]", + ) .with_stderr_contains("[RUNNING] `rustc --crate-name foo [..]") .run(); } @@ -1389,23 +1382,19 @@ fn build_script_deps_adopts_target_platform_if_target_equals_target() { p.cargo("check -v -Z bindeps --target") .arg(alternate_target) .masquerade_as_nightly_cargo(&["bindeps"]) - .with_stderr_does_not_contain(format!( - "[RUNNING] `rustc --crate-name build_script_build --edition=2015 build.rs [..]--target {} [..]", - alternate_target - )) + .with_stderr_does_not_contain( + "[RUNNING] `rustc --crate-name build_script_build --edition=2015 build.rs [..]--target [ALT_TARGET] [..]", + ) .with_stderr_contains("[RUNNING] `rustc --crate-name build_script_build --edition=2015 build.rs [..]") - .with_stderr_contains(format!( - "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/lib.rs [..]--target {} [..]", - alternate_target - )) - .with_stderr_contains(format!( - "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/main.rs [..]--target {} [..]", - alternate_target - )) - .with_stderr_contains(format!( - "[RUNNING] `rustc --crate-name foo [..]--target {} [..]", - alternate_target - )) + .with_stderr_contains( + "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/lib.rs [..]--target [ALT_TARGET] [..]", + ) + .with_stderr_contains( + "[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/main.rs [..]--target [ALT_TARGET] [..]", + ) + .with_stderr_contains( + "[RUNNING] `rustc --crate-name foo [..]--target [ALT_TARGET] [..]", + ) .run(); } diff --git a/tests/testsuite/check_cfg.rs b/tests/testsuite/check_cfg.rs index f60029e67ad7..a23d63353718 100644 --- a/tests/testsuite/check_cfg.rs +++ b/tests/testsuite/check_cfg.rs @@ -945,7 +945,7 @@ fn config_fingerprint() { p.cargo("check -v") // we check that the fingerprint is indeed dirty - .with_stderr_contains("[..]Dirty[..]the profile configuration changed") + .with_stderr_contains("[..][DIRTY][..]the profile configuration changed") // that cause rustc to be called again with the new check-cfg args .with_stderr_contains(x!("rustc" => "cfg" of "bar")) .with_stderr_contains(x!("rustc" => "cfg" of "foo"))