Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(test): Make redactions consistent with snapbox #14790

Merged
merged 1 commit into from
Nov 7, 2024
Merged

fix(test): Make redactions consistent with snapbox #14790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
240 changes: 27 additions & 213 deletions crates/cargo-test-support/src/compare.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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', "<tab>");
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 {
Expand Down Expand Up @@ -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\
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
15 changes: 5 additions & 10 deletions crates/cargo-test-support/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: AsRef<OsStr>>(&mut self, key: &str, val: T) -> &mut Self {
if let Some(ref mut p) = self.process_builder {
p.env(key, val);
Expand Down Expand Up @@ -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 => {}
Expand Down Expand Up @@ -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(())
}
Expand Down
Loading