From 2021865d88e141a97a0b3e68438e96700c6f636a Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 15 Jun 2021 15:38:02 -0700 Subject: [PATCH 01/10] Remove some unused code. --- crates/cargo-test-support/src/lib.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index b99055e1e1b..fc0475ce956 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -1470,17 +1470,6 @@ pub fn execs() -> Execs { } } -pub trait Tap { - fn tap(self, callback: F) -> Self; -} - -impl Tap for T { - fn tap(mut self, callback: F) -> T { - callback(&mut self); - self - } -} - pub fn basic_manifest(name: &str, version: &str) -> String { format!( r#" From 0f5deb64f93dc54fccdb91c7cea551d79d60b7c0 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 15 Jun 2021 16:09:03 -0700 Subject: [PATCH 02/10] testsuite: Support anyhow error chains in error messages. This is intended to help with adding more usage of anyhow in the testsuite, which can help show context for errors. This also includes some small improvements to the error messages to provide more information. --- crates/cargo-test-support/src/lib.rs | 31 +++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index fc0475ce956..487d5910f74 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -8,6 +8,7 @@ use std::env; use std::ffi::OsStr; +use std::fmt::Write; use std::fs; use std::os; use std::path::{Path, PathBuf}; @@ -27,11 +28,26 @@ macro_rules! t { ($e:expr) => { match $e { Ok(e) => e, - Err(e) => panic!("{} failed with {}", stringify!($e), e), + Err(e) => $crate::panic_error(&format!("failed running {}", stringify!($e)), e), } }; } +#[track_caller] +pub fn panic_error(what: &str, err: impl Into) -> ! { + let err = err.into(); + pe(what, err); + #[track_caller] + fn pe(what: &str, err: anyhow::Error) -> ! { + let mut result = format!("{}\nerror: {}", what, err); + for cause in err.chain().skip(1) { + drop(writeln!(result, "\nCaused by:")); + drop(write!(result, "{}", cause)); + } + panic!("\n{}", result); + } +} + pub use cargo_test_macro::cargo_test; pub mod cross_compile; @@ -737,7 +753,7 @@ impl Execs { self.ran = true; let p = (&self.process_builder).clone().unwrap(); if let Err(e) = self.match_process(&p) { - panic!("\n{}", e) + panic_error(&format!("test failed running {}", p), e); } } @@ -748,7 +764,7 @@ impl Execs { self.ran = true; let p = (&self.process_builder).clone().unwrap(); match self.match_process(&p) { - Err(e) => panic!("\n{}", e), + Err(e) => panic_error(&format!("test failed running {}", p), e), Ok(output) => serde_json::from_slice(&output.stdout).unwrap_or_else(|e| { panic!( "\nfailed to parse JSON: {}\n\ @@ -764,7 +780,7 @@ impl Execs { pub fn run_output(&mut self, output: &Output) { self.ran = true; if let Err(e) = self.match_output(output) { - panic!("\n{}", e) + panic_error("process did not return the expected result", e) } } @@ -858,9 +874,10 @@ impl Execs { match self.expect_exit_code { None => Ok(()), Some(expected) if code == Some(expected) => Ok(()), - Some(_) => bail!( - "exited with {:?}\n--- stdout\n{}\n--- stderr\n{}", - code, + Some(expected) => bail!( + "process exited with code {} (expected {})\n--- stdout\n{}\n--- stderr\n{}", + code.unwrap_or(-1), + expected, String::from_utf8_lossy(stdout), String::from_utf8_lossy(stderr) ), From e132bb53abe06e53f83541f85808828c08689909 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 15 Jun 2021 16:23:06 -0700 Subject: [PATCH 03/10] Move comparison and diffing code to a new module. This includes various minor refactorings to try to clean things up and provide better error messages. --- crates/cargo-test-support/Cargo.toml | 1 + crates/cargo-test-support/src/compare.rs | 597 ++++++++++++++++++ crates/cargo-test-support/src/lib.rs | 754 +++-------------------- crates/cargo-test-support/src/publish.rs | 2 +- src/doc/contrib/src/tests/writing.md | 8 +- tests/testsuite/build.rs | 11 +- tests/testsuite/build_script.rs | 3 +- tests/testsuite/config.rs | 3 +- tests/testsuite/lockfile_compat.rs | 3 +- 9 files changed, 713 insertions(+), 669 deletions(-) create mode 100644 crates/cargo-test-support/src/compare.rs diff --git a/crates/cargo-test-support/Cargo.toml b/crates/cargo-test-support/Cargo.toml index 8fe6268a430..99f8d271785 100644 --- a/crates/cargo-test-support/Cargo.toml +++ b/crates/cargo-test-support/Cargo.toml @@ -16,6 +16,7 @@ filetime = "0.2" flate2 = { version = "1.0", default-features = false, features = ["zlib"] } git2 = "0.13.16" glob = "0.3" +itertools = "0.10.0" lazy_static = "1.0" remove_dir_all = "0.5" serde_json = "1.0" diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs new file mode 100644 index 00000000000..4fa21e9c266 --- /dev/null +++ b/crates/cargo-test-support/src/compare.rs @@ -0,0 +1,597 @@ +//! Routines for comparing and diffing output. +//! +//! # Patterns +//! +//! Many of these functions support special markup to assist with comparing +//! text that may vary or is otherwise uninteresting for the test at hand. The +//! supported patterns are: +//! +//! - `[..]` is a wildcard that matches 0 or more characters on the same line +//! (similar to `.*` in a regex). It is non-greedy. +//! - `[EXE]` optionally adds `.exe` on Windows (empty string on other +//! platforms). +//! - `[ROOT]` is the path to the test directory's root. +//! - `[CWD]` is the working directory of the process that was run. +//! - There is a wide range of substitutions (such as `[COMPILING]` or +//! `[WARNING]`) to match cargo's "status" output and allows you to ignore +//! the alignment. See the source of `substitute_macros` for a complete list +//! of substitutions. +//! +//! # Normalization +//! +//! In addition to the patterns described above, the strings are normalized +//! in such a way to avoid unwanted differences. The normalizations are: +//! +//! - Raw tab characters are converted to the string ``. This is helpful +//! so that raw tabs do not need to be written in the expected string, and +//! to avoid confusion of tabs vs spaces. +//! - Backslashes are converted to forward slashes to deal with Windows paths. +//! This helps so that all tests can be written assuming forward slashes. +//! Other heuristics are applied to try to ensure Windows-style paths aren't +//! a problem. +//! - Carriage returns are removed, which can help when running on Windows. + +use crate::paths; +use anyhow::{bail, Context, Result}; +use serde_json::Value; +use std::env; +use std::path::Path; +use std::str; +use url::Url; + +/// 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', ""); + // Let's not deal with \r\n vs \n on windows... + let actual = actual.replace('\r', ""); + normalize_common(&actual, cwd) +} + +/// 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 = substitute_macros(expected); + normalize_common(&expected, cwd) +} + +/// Normalizes text for both actual and expected strings. +fn normalize_common(text: &str, cwd: Option<&Path>) -> String { + // Let's not deal with / vs \ (windows...) + // First replace backslash-escaped backslashes with forward slashes + // which can occur in, for example, JSON output + let text = text.replace("\\\\", "/").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 macros = [ + ("[RUNNING]", " Running"), + ("[COMPILING]", " Compiling"), + ("[CHECKING]", " Checking"), + ("[COMPLETED]", " Completed"), + ("[CREATED]", " Created"), + ("[FINISHED]", " Finished"), + ("[ERROR]", "error:"), + ("[WARNING]", "warning:"), + ("[NOTE]", "note:"), + ("[HELP]", "help:"), + ("[DOCUMENTING]", " Documenting"), + ("[FRESH]", " Fresh"), + ("[UPDATING]", " Updating"), + ("[ADDING]", " Adding"), + ("[REMOVING]", " Removing"), + ("[DOCTEST]", " Doc-tests"), + ("[PACKAGING]", " Packaging"), + ("[DOWNLOADING]", " Downloading"), + ("[DOWNLOADED]", " Downloaded"), + ("[UPLOADING]", " Uploading"), + ("[VERIFYING]", " Verifying"), + ("[ARCHIVING]", " Archiving"), + ("[INSTALLING]", " Installing"), + ("[REPLACING]", " Replacing"), + ("[UNPACKING]", " Unpacking"), + ("[SUMMARY]", " Summary"), + ("[FIXED]", " Fixed"), + ("[FIXING]", " Fixing"), + ("[EXE]", env::consts::EXE_SUFFIX), + ("[IGNORED]", " Ignored"), + ("[INSTALLED]", " Installed"), + ("[REPLACED]", " Replaced"), + ("[BUILDING]", " Building"), + ("[LOGIN]", " Login"), + ("[LOGOUT]", " Logout"), + ("[YANK]", " Yank"), + ("[OWNER]", " Owner"), + ("[MIGRATING]", " Migrating"), + ]; + let mut result = input.to_owned(); + for &(pat, subst) in ¯os { + result = result.replace(pat, subst) + } + result +} + +/// Compares one string against another, checking that they both match. +/// +/// See [Patterns](index.html#patterns) for more information on pattern matching. +/// +/// - `description` explains where the output is from (usually "stdout" or "stderr"). +/// - `other_output` is other output to display in the error (usually stdout or stderr). +pub fn match_exact( + expected: &str, + actual: &str, + description: &str, + other_output: &str, + cwd: Option<&Path>, +) -> Result<()> { + let expected = normalize_expected(expected, cwd); + let actual = normalize_actual(actual, cwd); + let e = expected.lines(); + let a = actual.lines(); + + let diffs = diff_lines(a, e, false); + if diffs.is_empty() { + Ok(()) + } else { + bail!( + "{} did not match:\n\ + {}\n\n\ + other output:\n\ + `{}`", + description, + diffs.join("\n"), + other_output, + ) + } +} + +/// 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 fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { + let expected = normalize_expected(expected, cwd); + let actual = normalize_actual(actual, cwd); + let mut a = actual.lines().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.len()); + let mut failures = Vec::new(); + + for e_line in expected.lines() { + match a.iter().position(|a_line| lines_match(e_line, a_line)) { + Some(index) => { + a.remove(index); + } + None => failures.push(e_line), + } + } + if !failures.is_empty() { + bail!( + "Did not find expected line(s):\n{}\n\ + Remaining available output:\n{}\n", + failures.join("\n"), + a.join("\n") + ); + } + if !a.is_empty() { + bail!( + "Output included extra lines:\n\ + {}\n", + a.join("\n") + ) + } else { + Ok(()) + } +} + +/// Checks that the given string contains the given contiguous lines +/// somewhere. +/// +/// See [Patterns](index.html#patterns) for more information on pattern matching. +pub fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { + let expected = normalize_expected(expected, cwd); + let actual = normalize_actual(actual, cwd); + let e = expected.lines(); + let mut a = actual.lines(); + + let mut diffs = diff_lines(a.clone(), e.clone(), true); + while a.next().is_some() { + let a = diff_lines(a.clone(), e.clone(), true); + if a.len() < diffs.len() { + diffs = a; + } + } + if diffs.is_empty() { + Ok(()) + } else { + bail!( + "expected to find:\n\ + {}\n\n\ + did not find in output:\n\ + {}", + expected, + actual + ) + } +} + +/// Checks that the given string does not contain the given contiguous lines +/// anywhere. +/// +/// See [Patterns](index.html#patterns) for more information on pattern matching. +pub fn match_does_not_contain(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { + if match_contains(expected, actual, cwd).is_ok() { + bail!( + "expected not to find:\n\ + {}\n\n\ + but found in output:\n\ + {}", + expected, + actual + ); + } else { + Ok(()) + } +} + +/// Checks that the given string contains the given contiguous lines +/// somewhere, and should be repeated `number` times. +/// +/// See [Patterns](index.html#patterns) for more information on pattern matching. +pub fn match_contains_n( + expected: &str, + number: usize, + actual: &str, + cwd: Option<&Path>, +) -> Result<()> { + let expected = normalize_expected(expected, cwd); + let actual = normalize_actual(actual, cwd); + let e = expected.lines(); + let mut a = actual.lines(); + + let mut matches = 0; + + while let Some(..) = { + if diff_lines(a.clone(), e.clone(), true).is_empty() { + matches += 1; + } + a.next() + } {} + + if matches == number { + Ok(()) + } else { + bail!( + "expected to find {} occurrences:\n\ + {}\n\n\ + did not find in output:\n\ + {}", + number, + expected, + actual + ) + } +} + +/// Checks that the given string has a line that contains the given patterns, +/// and that line also does not contain the `without` patterns. +/// +/// See [Patterns](index.html#patterns) for more information on pattern matching. +/// +/// See [`crate::Execs::with_stderr_line_without`] for an example and cautions +/// against using. +pub fn match_with_without( + actual: &str, + with: &[String], + without: &[String], + cwd: Option<&Path>, +) -> Result<()> { + let actual = normalize_actual(actual, cwd); + let contains = |s, line| { + let mut s = normalize_expected(s, cwd); + s.insert_str(0, "[..]"); + s.push_str("[..]"); + lines_match(&s, line) + }; + let matches: Vec<&str> = actual + .lines() + .filter(|line| with.iter().all(|with| contains(with, line))) + .filter(|line| !without.iter().any(|without| contains(without, line))) + .collect(); + match matches.len() { + 0 => bail!( + "Could not find expected line in output.\n\ + With contents: {:?}\n\ + Without contents: {:?}\n\ + Actual stderr:\n\ + {}\n", + with, + without, + actual + ), + 1 => Ok(()), + _ => bail!( + "Found multiple matching lines, but only expected one.\n\ + With contents: {:?}\n\ + Without contents: {:?}\n\ + Matching lines:\n\ + {}\n", + with, + without, + matches.join("\n") + ), + } +} + +/// Checks that the given string of JSON objects match the given set of +/// expected JSON objects. +/// +/// See [`crate::Execs::with_json`] for more details. +pub fn match_json(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { + let (exp_objs, act_objs) = collect_json_objects(expected, actual)?; + if exp_objs.len() != act_objs.len() { + bail!( + "expected {} json lines, got {}, stdout:\n{}", + exp_objs.len(), + act_objs.len(), + actual + ); + } + for (exp_obj, act_obj) in exp_objs.iter().zip(act_objs) { + find_json_mismatch(exp_obj, &act_obj, cwd)?; + } + Ok(()) +} + +/// Checks that the given string of JSON objects match the given set of +/// expected JSON objects, ignoring their order. +/// +/// See [`crate::Execs::with_json_contains_unordered`] for more details and +/// cautions when using. +pub fn match_json_contains_unordered( + expected: &str, + actual: &str, + cwd: Option<&Path>, +) -> Result<()> { + let (exp_objs, mut act_objs) = collect_json_objects(expected, actual)?; + for exp_obj in exp_objs { + match act_objs + .iter() + .position(|act_obj| find_json_mismatch(&exp_obj, act_obj, cwd).is_ok()) + { + Some(index) => act_objs.remove(index), + None => { + bail!( + "Did not find expected JSON:\n\ + {}\n\ + Remaining available output:\n\ + {}\n", + serde_json::to_string_pretty(&exp_obj).unwrap(), + itertools::join( + act_objs.iter().map(|o| serde_json::to_string(o).unwrap()), + "\n" + ) + ); + } + }; + } + Ok(()) +} + +fn collect_json_objects( + expected: &str, + actual: &str, +) -> Result<(Vec, Vec)> { + let expected_objs: Vec<_> = expected + .split("\n\n") + .map(|expect| { + expect + .parse() + .with_context(|| format!("failed to parse expected JSON object:\n{}", expect)) + }) + .collect::>()?; + let actual_objs: Vec<_> = actual + .lines() + .filter(|line| line.starts_with('{')) + .map(|line| { + line.parse() + .with_context(|| format!("failed to parse JSON object:\n{}", line)) + }) + .collect::>()?; + Ok((expected_objs, actual_objs)) +} + +fn diff_lines<'a>(actual: str::Lines<'a>, expected: str::Lines<'a>, partial: bool) -> Vec { + let actual = actual.take(if partial { + expected.clone().count() + } else { + usize::MAX + }); + zip_all(actual, expected) + .enumerate() + .filter_map(|(i, (a, e))| match (a, e) { + (Some(a), Some(e)) => { + if lines_match(e, a) { + None + } else { + Some(format!("{:3} - |{}|\n + |{}|\n", i, e, a)) + } + } + (Some(a), None) => Some(format!("{:3} -\n + |{}|\n", i, a)), + (None, Some(e)) => Some(format!("{:3} - |{}|\n +\n", i, e)), + (None, None) => unreachable!(), + }) + .collect() +} + +struct ZipAll { + first: I1, + second: I2, +} + +impl, I2: Iterator> Iterator for ZipAll { + type Item = (Option, Option); + fn next(&mut self) -> Option<(Option, Option)> { + let first = self.first.next(); + let second = self.second.next(); + + match (first, second) { + (None, None) => None, + (a, b) => Some((a, b)), + } + } +} + +/// Returns an iterator, similar to `zip`, but exhausts both iterators. +/// +/// Each element is `(Option, Option)` where `None` indicates an +/// iterator ended early. +fn zip_all, I2: Iterator>(a: I1, b: I2) -> ZipAll { + ZipAll { + first: a, + second: b, + } +} + +/// Compares a line with an expected pattern. +/// - Use `[..]` as a wildcard to match 0 or more characters on the same line +/// (similar to `.*` in a regex). It is non-greedy. +/// - Use `[EXE]` to optionally add `.exe` on Windows (empty string on other +/// platforms). +/// - There is a wide range of macros (such as `[COMPILING]` or `[WARNING]`) +/// to match cargo's "status" output and allows you to ignore the alignment. +/// See `substitute_macros` for a complete list of macros. +/// - `[ROOT]` the path to the test directory's root +/// - `[CWD]` is the working directory of the process that was run. +pub fn lines_match(expected: &str, mut actual: &str) -> bool { + for (i, part) in expected.split("[..]").enumerate() { + match actual.find(part) { + Some(j) => { + if i == 0 && j != 0 { + return false; + } + actual = &actual[j + part.len()..]; + } + None => return false, + } + } + actual.is_empty() || expected.ends_with("[..]") +} + +/// Variant of `lines_match` that applies normalization to the strings. +pub fn normalized_lines_match(expected: &str, actual: &str, cwd: Option<&Path>) -> bool { + let expected = normalize_expected(expected, cwd); + let actual = normalize_actual(actual, cwd); + lines_match(&expected, &actual) +} + +/// Compares JSON object for approximate equality. +/// You can use `[..]` wildcard in strings (useful for OS-dependent things such +/// as paths). You can use a `"{...}"` string literal as a wildcard for +/// arbitrary nested JSON (useful for parts of object emitted by other programs +/// (e.g., rustc) rather than Cargo itself). +pub fn find_json_mismatch(expected: &Value, actual: &Value, cwd: Option<&Path>) -> Result<()> { + match find_json_mismatch_r(expected, actual, cwd) { + Some((expected_part, actual_part)) => bail!( + "JSON mismatch\nExpected:\n{}\nWas:\n{}\nExpected part:\n{}\nActual part:\n{}\n", + serde_json::to_string_pretty(expected).unwrap(), + serde_json::to_string_pretty(&actual).unwrap(), + serde_json::to_string_pretty(expected_part).unwrap(), + serde_json::to_string_pretty(actual_part).unwrap(), + ), + None => Ok(()), + } +} + +fn find_json_mismatch_r<'a>( + expected: &'a Value, + actual: &'a Value, + cwd: Option<&Path>, +) -> Option<(&'a Value, &'a Value)> { + use serde_json::Value::*; + match (expected, actual) { + (&Number(ref l), &Number(ref r)) if l == r => None, + (&Bool(l), &Bool(r)) if l == r => None, + (&String(ref l), _) if l == "{...}" => None, + (&String(ref l), &String(ref r)) => { + let l = normalize_expected(l, cwd); + let r = normalize_actual(r, cwd); + if lines_match(&l, &r) { + None + } else { + Some((expected, actual)) + } + } + (&Array(ref l), &Array(ref r)) => { + if l.len() != r.len() { + return Some((expected, actual)); + } + + l.iter() + .zip(r.iter()) + .filter_map(|(l, r)| find_json_mismatch_r(l, r, cwd)) + .next() + } + (&Object(ref l), &Object(ref r)) => { + let same_keys = l.len() == r.len() && l.keys().all(|k| r.contains_key(k)); + if !same_keys { + return Some((expected, actual)); + } + + l.values() + .zip(r.values()) + .filter_map(|(l, r)| find_json_mismatch_r(l, r, cwd)) + .next() + } + (&Null, &Null) => None, + // Magic string literal `"{...}"` acts as wildcard for any sub-JSON. + _ => Some((expected, actual)), + } +} + +#[test] +fn lines_match_works() { + assert!(lines_match("a b", "a b")); + assert!(lines_match("a[..]b", "a b")); + assert!(lines_match("a[..]", "a b")); + assert!(lines_match("[..]", "a b")); + assert!(lines_match("[..]b", "a b")); + + assert!(!lines_match("[..]b", "c")); + assert!(!lines_match("b", "c")); + assert!(!lines_match("b", "cb")); +} diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 487d5910f74..0f82f1aee49 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -1,6 +1,6 @@ //! # Cargo test support. //! -//! See https://rust-lang.github.io/cargo/contrib/ for a guide on writing tests. +//! See for a guide on writing tests. #![allow(clippy::all)] #![warn(clippy::needless_borrow)] @@ -16,9 +16,9 @@ use std::process::{Command, Output}; use std::str; use std::time::{self, Duration}; -use anyhow::{bail, format_err, Result}; +use anyhow::{bail, Result}; use cargo_util::{is_ci, ProcessBuilder, ProcessError}; -use serde_json::{self, Value}; +use serde_json; use url::Url; use self::paths::CargoPathExt; @@ -50,8 +50,10 @@ pub fn panic_error(what: &str, err: impl Into) -> ! { pub use cargo_test_macro::cargo_test; +pub mod compare; pub mod cross_compile; pub mod git; +pub mod install; pub mod paths; pub mod publish; pub mod registry; @@ -449,12 +451,6 @@ pub fn cargo_exe() -> PathBuf { cargo_dir().join(format!("cargo{}", env::consts::EXE_SUFFIX)) } -/* - * - * ===== Matchers ===== - * - */ - /// This is the raw output from the process. /// /// This is similar to `std::process::Output`, however the `status` is @@ -484,10 +480,9 @@ pub struct Execs { expect_stdout_not_contains: Vec, expect_stderr_not_contains: Vec, expect_stderr_unordered: Vec, - expect_neither_contains: Vec, expect_stderr_with_without: Vec<(Vec, Vec)>, - expect_json: Option>, - expect_json_contains_unordered: Vec, + expect_json: Option, + expect_json_contains_unordered: Option, stream_output: bool, } @@ -498,14 +493,14 @@ impl Execs { } /// Verifies that stdout is equal to the given lines. - /// See `lines_match` for supported patterns. + /// See [`compare`] for supported patterns. pub fn with_stdout(&mut self, expected: S) -> &mut Self { self.expect_stdout = Some(expected.to_string()); self } /// Verifies that stderr is equal to the given lines. - /// See `lines_match` for supported patterns. + /// See [`compare`] for supported patterns. pub fn with_stderr(&mut self, expected: S) -> &mut Self { self.expect_stderr = Some(expected.to_string()); self @@ -529,7 +524,8 @@ impl Execs { /// Verifies that stdout contains the given contiguous lines somewhere in /// its output. - /// See `lines_match` for supported patterns. + /// + /// See [`compare`] for supported patterns. pub fn with_stdout_contains(&mut self, expected: S) -> &mut Self { self.expect_stdout_contains.push(expected.to_string()); self @@ -537,7 +533,8 @@ impl Execs { /// Verifies that stderr contains the given contiguous lines somewhere in /// its output. - /// See `lines_match` for supported patterns. + /// + /// See [`compare`] for supported patterns. pub fn with_stderr_contains(&mut self, expected: S) -> &mut Self { self.expect_stderr_contains.push(expected.to_string()); self @@ -545,7 +542,8 @@ impl Execs { /// Verifies that either stdout or stderr contains the given contiguous /// lines somewhere in its output. - /// See `lines_match` for supported patterns. + /// + /// See [`compare`] for supported patterns. pub fn with_either_contains(&mut self, expected: S) -> &mut Self { self.expect_either_contains.push(expected.to_string()); self @@ -553,7 +551,8 @@ impl Execs { /// Verifies that stdout contains the given contiguous lines somewhere in /// its output, and should be repeated `number` times. - /// See `lines_match` for supported patterns. + /// + /// See [`compare`] for supported patterns. pub fn with_stdout_contains_n(&mut self, expected: S, number: usize) -> &mut Self { self.expect_stdout_contains_n .push((expected.to_string(), number)); @@ -561,15 +560,18 @@ impl Execs { } /// Verifies that stdout does not contain the given contiguous lines. - /// See `lines_match` for supported patterns. - /// See note on `with_stderr_does_not_contain`. + /// + /// See [`compare`] for supported patterns. + /// + /// See note on [`Self::with_stderr_does_not_contain`]. pub fn with_stdout_does_not_contain(&mut self, expected: S) -> &mut Self { self.expect_stdout_not_contains.push(expected.to_string()); self } /// Verifies that stderr does not contain the given contiguous lines. - /// See `lines_match` for supported patterns. + /// + /// See [`compare`] for supported patterns. /// /// Care should be taken when using this method because there is a /// limitless number of possible things that *won't* appear. A typo means @@ -583,7 +585,9 @@ impl Execs { /// Verifies that all of the stderr output is equal to the given lines, /// ignoring the order of the lines. - /// See `lines_match` for supported patterns. + /// + /// 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 @@ -593,8 +597,10 @@ impl Execs { /// with multiple lines that might match, and this is not smart enough to /// do anything like longest-match. For example, avoid something like: /// - /// [RUNNING] `rustc [..] - /// [RUNNING] `rustc --crate-name foo [..] + /// ```text + /// [RUNNING] `rustc [..] + /// [RUNNING] `rustc --crate-name foo [..] + /// ``` /// /// This will randomly fail if the other crate name is `bar`, and the /// order changes. @@ -635,28 +641,28 @@ impl Execs { } /// Verifies the JSON output matches the given JSON. - /// Typically used when testing cargo commands that emit JSON. + /// + /// This is typically used when testing cargo commands that emit JSON. /// Each separate JSON object should be separated by a blank line. /// Example: - /// assert_that( - /// p.cargo("metadata"), - /// execs().with_json(r#" - /// {"example": "abc"} /// - /// {"example": "def"} - /// "#) - /// ); - /// Objects should match in the order given. - /// The order of arrays is ignored. - /// Strings support patterns described in `lines_match`. - /// Use `{...}` to match any object. + /// ```rust,ignore + /// assert_that( + /// p.cargo("metadata"), + /// execs().with_json(r#" + /// {"example": "abc"} + /// + /// {"example": "def"} + /// "#) + /// ); + /// ``` + /// + /// - Objects should match in the order given. + /// - The order of arrays is ignored. + /// - Strings support patterns described in [`compare`]. + /// - Use `"{...}"` to match any object. pub fn with_json(&mut self, expected: &str) -> &mut Self { - self.expect_json = Some( - expected - .split("\n\n") - .map(|line| line.to_string()) - .collect(), - ); + self.expect_json = Some(expected.to_string()); self } @@ -670,8 +676,13 @@ impl Execs { /// /// See `with_json` for more detail. pub fn with_json_contains_unordered(&mut self, expected: &str) -> &mut Self { - self.expect_json_contains_unordered - .extend(expected.split("\n\n").map(|line| line.to_string())); + match &mut self.expect_json_contains_unordered { + None => self.expect_json_contains_unordered = Some(expected.to_string()), + Some(e) => { + e.push_str("\n\n"); + e.push_str(expected); + } + } self } @@ -703,6 +714,10 @@ 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); @@ -779,7 +794,7 @@ impl Execs { #[track_caller] pub fn run_output(&mut self, output: &Output) { self.ran = true; - if let Err(e) = self.match_output(output) { + if let Err(e) = self.match_output(output.status.code(), &output.stdout, &output.stderr) { panic_error("process did not return the expected result", e) } } @@ -796,10 +811,9 @@ impl Execs { && self.expect_stdout_not_contains.is_empty() && self.expect_stderr_not_contains.is_empty() && self.expect_stderr_unordered.is_empty() - && self.expect_neither_contains.is_empty() && self.expect_stderr_with_without.is_empty() && self.expect_json.is_none() - && self.expect_json_contains_unordered.is_empty() + && self.expect_json_contains_unordered.is_none() { panic!( "`with_status()` is used, but no output is checked.\n\ @@ -834,7 +848,7 @@ impl Execs { match res { Ok(out) => { - self.match_output(&out)?; + self.match_output(out.status.code(), &out.stdout, &out.stderr)?; return Ok(RawOutput { stdout: out.stdout, stderr: out.stderr, @@ -849,9 +863,7 @@ impl Execs { .. }) = e.downcast_ref::() { - self.match_status(*code, stdout, stderr) - .and(self.match_stdout(stdout, stderr)) - .and(self.match_stderr(stdout, stderr))?; + self.match_output(*code, stdout, stderr)?; return Ok(RawOutput { stdout: stdout.to_vec(), stderr: stderr.to_vec(), @@ -863,376 +875,78 @@ impl Execs { } } - fn match_output(&self, actual: &Output) -> Result<()> { - self.match_status(actual.status.code(), &actual.stdout, &actual.stderr) - .and(self.match_stdout(&actual.stdout, &actual.stderr)) - .and(self.match_stderr(&actual.stdout, &actual.stderr)) - } - - fn match_status(&self, code: Option, stdout: &[u8], stderr: &[u8]) -> Result<()> { + fn match_output(&self, code: Option, stdout: &[u8], stderr: &[u8]) -> Result<()> { self.verify_checks_output(stdout, stderr); + let stdout = str::from_utf8(stdout).expect("stdout is not utf8"); + let stderr = str::from_utf8(stderr).expect("stderr is not utf8"); + let cwd = self.get_cwd(); + match self.expect_exit_code { - None => Ok(()), - Some(expected) if code == Some(expected) => Ok(()), + None => {} + Some(expected) if code == Some(expected) => {} Some(expected) => bail!( "process exited with code {} (expected {})\n--- stdout\n{}\n--- stderr\n{}", code.unwrap_or(-1), expected, - String::from_utf8_lossy(stdout), - String::from_utf8_lossy(stderr) + stdout, + stderr ), } - } - fn match_stdout(&self, stdout: &[u8], stderr: &[u8]) -> Result<()> { - self.match_std( - self.expect_stdout.as_ref(), - stdout, - "stdout", - stderr, - MatchKind::Exact, - )?; + if let Some(expect_stdout) = &self.expect_stdout { + compare::match_exact(expect_stdout, stdout, "stdout", stderr, cwd)?; + } + if let Some(expect_stderr) = &self.expect_stderr { + compare::match_exact(expect_stderr, stderr, "stderr", stdout, cwd)?; + } for expect in self.expect_stdout_contains.iter() { - self.match_std(Some(expect), stdout, "stdout", stderr, MatchKind::Partial)?; + compare::match_contains(expect, stdout, cwd)?; } for expect in self.expect_stderr_contains.iter() { - self.match_std(Some(expect), stderr, "stderr", stdout, MatchKind::Partial)?; + compare::match_contains(expect, stderr, cwd)?; } for &(ref expect, number) in self.expect_stdout_contains_n.iter() { - self.match_std( - Some(expect), - stdout, - "stdout", - stderr, - MatchKind::PartialN(number), - )?; + compare::match_contains_n(expect, number, stdout, cwd)?; } for expect in self.expect_stdout_not_contains.iter() { - self.match_std( - Some(expect), - stdout, - "stdout", - stderr, - MatchKind::NotPresent, - )?; + compare::match_does_not_contain(expect, stdout, cwd)?; } for expect in self.expect_stderr_not_contains.iter() { - self.match_std( - Some(expect), - stderr, - "stderr", - stdout, - MatchKind::NotPresent, - )?; + compare::match_does_not_contain(expect, stderr, cwd)?; } for expect in self.expect_stderr_unordered.iter() { - self.match_std(Some(expect), stderr, "stderr", stdout, MatchKind::Unordered)?; - } - for expect in self.expect_neither_contains.iter() { - self.match_std( - Some(expect), - stdout, - "stdout", - stdout, - MatchKind::NotPresent, - )?; - - self.match_std( - Some(expect), - stderr, - "stderr", - stderr, - MatchKind::NotPresent, - )?; + compare::match_unordered(expect, stderr, cwd)?; } - for expect in self.expect_either_contains.iter() { - let match_std = - self.match_std(Some(expect), stdout, "stdout", stdout, MatchKind::Partial); - let match_err = - self.match_std(Some(expect), stderr, "stderr", stderr, MatchKind::Partial); - + let match_std = compare::match_contains(expect, stdout, cwd); + let match_err = compare::match_contains(expect, stderr, cwd); if let (Err(_), Err(_)) = (match_std, match_err) { bail!( "expected to find:\n\ {}\n\n\ - did not find in either output.", - expect + did not find in either output. + --- stdout\n{}\n + --- stderr\n{}\n", + expect, + stdout, + stderr, ); } } for (with, without) in self.expect_stderr_with_without.iter() { - self.match_with_without(stderr, with, without)?; + compare::match_with_without(stderr, with, without, cwd)?; } - if let Some(ref objects) = self.expect_json { - let stdout = - str::from_utf8(stdout).map_err(|_| format_err!("stdout was not utf8 encoded"))?; - let lines = stdout - .lines() - .filter(|line| line.starts_with('{')) - .collect::>(); - if lines.len() != objects.len() { - bail!( - "expected {} json lines, got {}, stdout:\n{}", - objects.len(), - lines.len(), - stdout - ); - } - for (obj, line) in objects.iter().zip(lines) { - self.match_json(obj, line)?; - } + if let Some(ref expect_json) = self.expect_json { + compare::match_json(expect_json, stdout, cwd)?; } - if !self.expect_json_contains_unordered.is_empty() { - let stdout = - str::from_utf8(stdout).map_err(|_| format_err!("stdout was not utf8 encoded"))?; - let mut lines = stdout - .lines() - .filter(|line| line.starts_with('{')) - .collect::>(); - for obj in &self.expect_json_contains_unordered { - match lines - .iter() - .position(|line| self.match_json(obj, line).is_ok()) - { - Some(index) => lines.remove(index), - None => { - bail!( - "Did not find expected JSON:\n\ - {}\n\ - Remaining available output:\n\ - {}\n", - serde_json::to_string_pretty(obj).unwrap(), - lines.join("\n") - ); - } - }; - } + if let Some(ref expected) = self.expect_json_contains_unordered { + compare::match_json_contains_unordered(expected, stdout, cwd)?; } Ok(()) } - - fn match_stderr(&self, stdout: &[u8], stderr: &[u8]) -> Result<()> { - self.match_std( - self.expect_stderr.as_ref(), - stderr, - "stderr", - stdout, - MatchKind::Exact, - ) - } - - fn normalize_actual(&self, description: &str, actual: &[u8]) -> Result { - let actual = match str::from_utf8(actual) { - Err(..) => bail!("{} was not utf8 encoded", description), - Ok(actual) => actual, - }; - Ok(self.normalize_matcher(actual)) - } - - fn normalize_matcher(&self, matcher: &str) -> String { - normalize_matcher( - matcher, - self.process_builder.as_ref().and_then(|p| p.get_cwd()), - ) - } - - fn match_std( - &self, - expected: Option<&String>, - actual: &[u8], - description: &str, - extra: &[u8], - kind: MatchKind, - ) -> Result<()> { - let out = match expected { - Some(out) => self.normalize_matcher(out), - None => return Ok(()), - }; - - let actual = self.normalize_actual(description, actual)?; - - match kind { - MatchKind::Exact => { - let a = actual.lines(); - let e = out.lines(); - - let diffs = self.diff_lines(a, e, false); - if diffs.is_empty() { - Ok(()) - } else { - bail!( - "{} did not match:\n\ - {}\n\n\ - other output:\n\ - `{}`", - description, - diffs.join("\n"), - String::from_utf8_lossy(extra) - ) - } - } - MatchKind::Partial => { - let mut a = actual.lines(); - let e = out.lines(); - - let mut diffs = self.diff_lines(a.clone(), e.clone(), true); - while a.next().is_some() { - let a = self.diff_lines(a.clone(), e.clone(), true); - if a.len() < diffs.len() { - diffs = a; - } - } - if diffs.is_empty() { - Ok(()) - } else { - bail!( - "expected to find:\n\ - {}\n\n\ - did not find in output:\n\ - {}", - out, - actual - ) - } - } - MatchKind::PartialN(number) => { - let mut a = actual.lines(); - let e = out.lines(); - - let mut matches = 0; - - while let Some(..) = { - if self.diff_lines(a.clone(), e.clone(), true).is_empty() { - matches += 1; - } - a.next() - } {} - - if matches == number { - Ok(()) - } else { - bail!( - "expected to find {} occurrences:\n\ - {}\n\n\ - did not find in output:\n\ - {}", - number, - out, - actual - ) - } - } - MatchKind::NotPresent => { - let mut a = actual.lines(); - let e = out.lines(); - - let mut diffs = self.diff_lines(a.clone(), e.clone(), true); - while a.next().is_some() { - let a = self.diff_lines(a.clone(), e.clone(), true); - if a.len() < diffs.len() { - diffs = a; - } - } - if diffs.is_empty() { - bail!( - "expected not to find:\n\ - {}\n\n\ - but found in output:\n\ - {}", - out, - actual - ) - } else { - Ok(()) - } - } - MatchKind::Unordered => lines_match_unordered(&out, &actual), - } - } - - fn match_with_without(&self, actual: &[u8], with: &[String], without: &[String]) -> Result<()> { - let actual = self.normalize_actual("stderr", actual)?; - let contains = |s, line| { - let mut s = self.normalize_matcher(s); - s.insert_str(0, "[..]"); - s.push_str("[..]"); - lines_match(&s, line) - }; - let matches: Vec<&str> = actual - .lines() - .filter(|line| with.iter().all(|with| contains(with, line))) - .filter(|line| !without.iter().any(|without| contains(without, line))) - .collect(); - match matches.len() { - 0 => bail!( - "Could not find expected line in output.\n\ - With contents: {:?}\n\ - Without contents: {:?}\n\ - Actual stderr:\n\ - {}\n", - with, - without, - actual - ), - 1 => Ok(()), - _ => bail!( - "Found multiple matching lines, but only expected one.\n\ - With contents: {:?}\n\ - Without contents: {:?}\n\ - Matching lines:\n\ - {}\n", - with, - without, - matches.join("\n") - ), - } - } - - fn match_json(&self, expected: &str, line: &str) -> Result<()> { - let actual = match line.parse() { - Err(e) => bail!("invalid json, {}:\n`{}`", e, line), - Ok(actual) => actual, - }; - let expected = match expected.parse() { - Err(e) => bail!("invalid json, {}:\n`{}`", e, line), - Ok(expected) => expected, - }; - - let cwd = self.process_builder.as_ref().and_then(|p| p.get_cwd()); - find_json_mismatch(&expected, &actual, cwd) - } - - fn diff_lines<'a>( - &self, - actual: str::Lines<'a>, - expected: str::Lines<'a>, - partial: bool, - ) -> Vec { - let actual = actual.take(if partial { - expected.clone().count() - } else { - usize::MAX - }); - zip_all(actual, expected) - .enumerate() - .filter_map(|(i, (a, e))| match (a, e) { - (Some(a), Some(e)) => { - if lines_match(e, a) { - None - } else { - Some(format!("{:3} - |{}|\n + |{}|\n", i, e, a)) - } - } - (Some(a), None) => Some(format!("{:3} -\n + |{}|\n", i, a)), - (None, Some(e)) => Some(format!("{:3} - |{}|\n +\n", i, e)), - (None, None) => panic!("Cannot get here"), - }) - .collect() - } } impl Drop for Execs { @@ -1243,227 +957,6 @@ impl Drop for Execs { } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -enum MatchKind { - Exact, - Partial, - PartialN(usize), - NotPresent, - Unordered, -} - -/// Compares a line with an expected pattern. -/// - Use `[..]` as a wildcard to match 0 or more characters on the same line -/// (similar to `.*` in a regex). It is non-greedy. -/// - Use `[EXE]` to optionally add `.exe` on Windows (empty string on other -/// platforms). -/// - There is a wide range of macros (such as `[COMPILING]` or `[WARNING]`) -/// to match cargo's "status" output and allows you to ignore the alignment. -/// See `substitute_macros` for a complete list of macros. -/// - `[ROOT]` the path to the test directory's root -/// - `[CWD]` is the working directory of the process that was run. -pub fn lines_match(expected: &str, mut actual: &str) -> bool { - let expected = substitute_macros(expected); - for (i, part) in expected.split("[..]").enumerate() { - match actual.find(part) { - Some(j) => { - if i == 0 && j != 0 { - return false; - } - actual = &actual[j + part.len()..]; - } - None => return false, - } - } - actual.is_empty() || expected.ends_with("[..]") -} - -pub fn lines_match_unordered(expected: &str, actual: &str) -> Result<()> { - let mut a = actual.lines().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.len()); - let mut failures = Vec::new(); - - for e_line in expected.lines() { - match a.iter().position(|a_line| lines_match(e_line, a_line)) { - Some(index) => { - a.remove(index); - } - None => failures.push(e_line), - } - } - if !failures.is_empty() { - bail!( - "Did not find expected line(s):\n{}\n\ - Remaining available output:\n{}\n", - failures.join("\n"), - a.join("\n") - ); - } - if !a.is_empty() { - bail!( - "Output included extra lines:\n\ - {}\n", - a.join("\n") - ) - } else { - Ok(()) - } -} - -/// Variant of `lines_match` that applies normalization to the strings. -pub fn normalized_lines_match(expected: &str, actual: &str, cwd: Option<&Path>) -> bool { - let expected = normalize_matcher(expected, cwd); - let actual = normalize_matcher(actual, cwd); - lines_match(&expected, &actual) -} - -fn normalize_matcher(matcher: &str, cwd: Option<&Path>) -> String { - // Let's not deal with / vs \ (windows...) - let matcher = matcher.replace("\\\\", "/").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) - }; - - // Do the template replacements on the expected string. - let matcher = match cwd { - None => matcher, - Some(p) => replace_path(&matcher, 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 matcher = replace_path(&matcher, &root, "[ROOT]"); - - // Let's not deal with \r\n vs \n on windows... - let matcher = matcher.replace("\r", ""); - - // It's easier to read tabs in outputs if they don't show up as literal - // hidden characters - matcher.replace("\t", "") -} - -#[test] -fn lines_match_works() { - assert!(lines_match("a b", "a b")); - assert!(lines_match("a[..]b", "a b")); - assert!(lines_match("a[..]", "a b")); - assert!(lines_match("[..]", "a b")); - assert!(lines_match("[..]b", "a b")); - - assert!(!lines_match("[..]b", "c")); - assert!(!lines_match("b", "c")); - assert!(!lines_match("b", "cb")); -} - -/// Compares JSON object for approximate equality. -/// You can use `[..]` wildcard in strings (useful for OS-dependent things such -/// as paths). You can use a `"{...}"` string literal as a wildcard for -/// arbitrary nested JSON (useful for parts of object emitted by other programs -/// (e.g., rustc) rather than Cargo itself). -pub fn find_json_mismatch(expected: &Value, actual: &Value, cwd: Option<&Path>) -> Result<()> { - match find_json_mismatch_r(expected, actual, cwd) { - Some((expected_part, actual_part)) => bail!( - "JSON mismatch\nExpected:\n{}\nWas:\n{}\nExpected part:\n{}\nActual part:\n{}\n", - serde_json::to_string_pretty(expected).unwrap(), - serde_json::to_string_pretty(&actual).unwrap(), - serde_json::to_string_pretty(expected_part).unwrap(), - serde_json::to_string_pretty(actual_part).unwrap(), - ), - None => Ok(()), - } -} - -fn find_json_mismatch_r<'a>( - expected: &'a Value, - actual: &'a Value, - cwd: Option<&Path>, -) -> Option<(&'a Value, &'a Value)> { - use serde_json::Value::*; - match (expected, actual) { - (&Number(ref l), &Number(ref r)) if l == r => None, - (&Bool(l), &Bool(r)) if l == r => None, - (&String(ref l), _) if l == "{...}" => None, - (&String(ref l), &String(ref r)) => { - let normalized = normalize_matcher(r, cwd); - if lines_match(l, &normalized) { - None - } else { - Some((expected, actual)) - } - } - (&Array(ref l), &Array(ref r)) => { - if l.len() != r.len() { - return Some((expected, actual)); - } - - l.iter() - .zip(r.iter()) - .filter_map(|(l, r)| find_json_mismatch_r(l, r, cwd)) - .next() - } - (&Object(ref l), &Object(ref r)) => { - let same_keys = l.len() == r.len() && l.keys().all(|k| r.contains_key(k)); - if !same_keys { - return Some((expected, actual)); - } - - l.values() - .zip(r.values()) - .filter_map(|(l, r)| find_json_mismatch_r(l, r, cwd)) - .next() - } - (&Null, &Null) => None, - // Magic string literal `"{...}"` acts as wildcard for any sub-JSON. - _ => Some((expected, actual)), - } -} - -struct ZipAll { - first: I1, - second: I2, -} - -impl, I2: Iterator> Iterator for ZipAll { - type Item = (Option, Option); - fn next(&mut self) -> Option<(Option, Option)> { - let first = self.first.next(); - let second = self.second.next(); - - match (first, second) { - (None, None) => None, - (a, b) => Some((a, b)), - } - } -} - -fn zip_all, I2: Iterator>(a: I1, b: I2) -> ZipAll { - ZipAll { - first: a, - second: b, - } -} - pub fn execs() -> Execs { Execs { ran: false, @@ -1479,10 +972,9 @@ pub fn execs() -> Execs { expect_stdout_not_contains: Vec::new(), expect_stderr_not_contains: Vec::new(), expect_stderr_unordered: Vec::new(), - expect_neither_contains: Vec::new(), expect_stderr_with_without: Vec::new(), expect_json: None, - expect_json_contains_unordered: Vec::new(), + expect_json_contains_unordered: None, stream_output: false, } } @@ -1537,56 +1029,6 @@ pub fn path2url>(p: P) -> Url { Url::from_file_path(p).ok().unwrap() } -fn substitute_macros(input: &str) -> String { - let macros = [ - ("[RUNNING]", " Running"), - ("[COMPILING]", " Compiling"), - ("[CHECKING]", " Checking"), - ("[COMPLETED]", " Completed"), - ("[CREATED]", " Created"), - ("[FINISHED]", " Finished"), - ("[ERROR]", "error:"), - ("[WARNING]", "warning:"), - ("[NOTE]", "note:"), - ("[HELP]", "help:"), - ("[DOCUMENTING]", " Documenting"), - ("[FRESH]", " Fresh"), - ("[UPDATING]", " Updating"), - ("[ADDING]", " Adding"), - ("[REMOVING]", " Removing"), - ("[DOCTEST]", " Doc-tests"), - ("[PACKAGING]", " Packaging"), - ("[DOWNLOADING]", " Downloading"), - ("[DOWNLOADED]", " Downloaded"), - ("[UPLOADING]", " Uploading"), - ("[VERIFYING]", " Verifying"), - ("[ARCHIVING]", " Archiving"), - ("[INSTALLING]", " Installing"), - ("[REPLACING]", " Replacing"), - ("[UNPACKING]", " Unpacking"), - ("[SUMMARY]", " Summary"), - ("[FIXED]", " Fixed"), - ("[FIXING]", " Fixing"), - ("[EXE]", env::consts::EXE_SUFFIX), - ("[IGNORED]", " Ignored"), - ("[INSTALLED]", " Installed"), - ("[REPLACED]", " Replaced"), - ("[BUILDING]", " Building"), - ("[LOGIN]", " Login"), - ("[LOGOUT]", " Logout"), - ("[YANK]", " Yank"), - ("[OWNER]", " Owner"), - ("[MIGRATING]", " Migrating"), - ]; - let mut result = input.to_owned(); - for &(pat, subst) in ¯os { - result = result.replace(pat, subst) - } - result -} - -pub mod install; - struct RustcInfo { verbose_version: String, host: String, diff --git a/crates/cargo-test-support/src/publish.rs b/crates/cargo-test-support/src/publish.rs index 6a4549f158a..a0b31be2169 100644 --- a/crates/cargo-test-support/src/publish.rs +++ b/crates/cargo-test-support/src/publish.rs @@ -1,5 +1,5 @@ +use crate::compare::{find_json_mismatch, lines_match}; use crate::registry::{self, alt_api_path}; -use crate::{find_json_mismatch, lines_match}; use flate2::read::GzDecoder; use std::collections::{HashMap, HashSet}; use std::fs::File; diff --git a/src/doc/contrib/src/tests/writing.md b/src/doc/contrib/src/tests/writing.md index 3343f1e6598..42d1b5c7910 100644 --- a/src/doc/contrib/src/tests/writing.md +++ b/src/doc/contrib/src/tests/writing.md @@ -58,11 +58,11 @@ p.cargo("run --bin foo") This uses the [`Execs`] struct to build up a command to execute, along with the expected output. -See [`support::lines_match`] for an explanation of the string pattern matching. +See [`support::compare`] for an explanation of the string pattern matching. Patterns are used to make it easier to match against the expected output. -Browse the `pub` functions in the [`support`] crate for a variety of other -helpful utilities. +Browse the `pub` functions and modules in the [`support`] crate for a variety +of other helpful utilities. ### Testing Nightly Features @@ -127,6 +127,6 @@ dependency. [`ProjectBuilder`]: https://github.com/rust-lang/cargo/blob/e4b65bdc80f2a293447f2f6a808fa7c84bf9a357/crates/cargo-test-support/src/lib.rs#L225-L231 [`Execs`]: https://github.com/rust-lang/cargo/blob/e4b65bdc80f2a293447f2f6a808fa7c84bf9a357/crates/cargo-test-support/src/lib.rs#L558-L579 [`support`]: https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/lib.rs -[`support::lines_match`]: https://github.com/rust-lang/cargo/blob/e4b65bdc80f2a293447f2f6a808fa7c84bf9a357/crates/cargo-test-support/src/lib.rs#L1322-L1332 +[`support::compare`]: https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/compare.rs [`support::registry::Package`]: https://github.com/rust-lang/cargo/blob/e4b65bdc80f2a293447f2f6a808fa7c84bf9a357/crates/cargo-test-support/src/registry.rs#L73-L149 [`support::git`]: https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/git.rs diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 5b2f3786bd5..1dfc8d99437 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -6,13 +6,13 @@ use cargo::{ ops::CompileOptions, Config, }; +use cargo_test_support::compare; use cargo_test_support::paths::{root, CargoPathExt}; use cargo_test_support::registry::Package; use cargo_test_support::tools; use cargo_test_support::{ - basic_bin_manifest, basic_lib_manifest, basic_manifest, cargo_exe, git, is_nightly, - lines_match_unordered, main_file, paths, process, project, rustc_host, sleep_ms, - symlink_supported, t, Execs, ProjectBuilder, + basic_bin_manifest, basic_lib_manifest, basic_manifest, cargo_exe, git, is_nightly, main_file, + paths, process, project, rustc_host, sleep_ms, symlink_supported, t, Execs, ProjectBuilder, }; use cargo_util::paths::dylib_path_envvar; use std::env; @@ -5320,7 +5320,7 @@ fn close_output() { }; let stderr = spawn(false); - lines_match_unordered( + compare::match_unordered( "\ [COMPILING] foo [..] hello stderr! @@ -5329,13 +5329,14 @@ hello stderr! [ERROR] [..] ", &stderr, + None, ) .unwrap(); // Try again with stderr. p.build_dir().rm_rf(); let stdout = spawn(true); - lines_match_unordered("hello stdout!\n", &stdout).unwrap(); + assert_eq!(stdout, "hello stdout!\n"); } #[cargo_test] diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 44e9295f60d..46ccb703e9a 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -1,8 +1,9 @@ //! Tests for build.rs scripts. +use cargo_test_support::compare::lines_match; +use cargo_test_support::paths::CargoPathExt; use cargo_test_support::registry::Package; use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project}; -use cargo_test_support::{lines_match, paths::CargoPathExt}; use cargo_test_support::{rustc_host, sleep_ms, slow_cpu_multiplier, symlink_supported}; use cargo_util::paths::remove_dir_all; use std::env; diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index e066a7927da..b416b687e30 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -5,7 +5,8 @@ use cargo::util::config::{self, Config, SslVersionConfig, StringList}; use cargo::util::interning::InternedString; use cargo::util::toml::{self, VecStringOrBool as VSOB}; use cargo::CargoResult; -use cargo_test_support::{normalized_lines_match, paths, project, t}; +use cargo_test_support::compare::normalized_lines_match; +use cargo_test_support::{paths, project, t}; use serde::Deserialize; use std::borrow::Borrow; use std::collections::{BTreeMap, HashMap}; diff --git a/tests/testsuite/lockfile_compat.rs b/tests/testsuite/lockfile_compat.rs index 77d874c5aac..8955beaae04 100644 --- a/tests/testsuite/lockfile_compat.rs +++ b/tests/testsuite/lockfile_compat.rs @@ -1,8 +1,9 @@ //! Tests for supporting older versions of the Cargo.lock file format. +use cargo_test_support::compare::lines_match; use cargo_test_support::git; use cargo_test_support::registry::Package; -use cargo_test_support::{basic_lib_manifest, basic_manifest, lines_match, project}; +use cargo_test_support::{basic_lib_manifest, basic_manifest, project}; #[cargo_test] fn oldest_lockfile_still_works() { From 6dff99781d65a44423daba2de553d41fe4bcefd8 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 15 Jun 2021 17:48:05 -0700 Subject: [PATCH 04/10] Remove with_either_contains. It isn't needed anymore, and I would prefer to not keep around unused code. It can always be added back if ever needed again. --- crates/cargo-test-support/src/lib.rs | 29 ---------------------------- tests/testsuite/bench.rs | 8 ++++---- 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 0f82f1aee49..8082ba61fd8 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -475,7 +475,6 @@ pub struct Execs { expect_exit_code: Option, expect_stdout_contains: Vec, expect_stderr_contains: Vec, - expect_either_contains: Vec, expect_stdout_contains_n: Vec<(String, usize)>, expect_stdout_not_contains: Vec, expect_stderr_not_contains: Vec, @@ -540,15 +539,6 @@ impl Execs { self } - /// Verifies that either stdout or stderr contains the given contiguous - /// lines somewhere in its output. - /// - /// See [`compare`] for supported patterns. - pub fn with_either_contains(&mut self, expected: S) -> &mut Self { - self.expect_either_contains.push(expected.to_string()); - self - } - /// Verifies that stdout contains the given contiguous lines somewhere in /// its output, and should be repeated `number` times. /// @@ -806,7 +796,6 @@ impl Execs { && self.expect_stderr.is_none() && self.expect_stdout_contains.is_empty() && self.expect_stderr_contains.is_empty() - && self.expect_either_contains.is_empty() && self.expect_stdout_contains_n.is_empty() && self.expect_stdout_not_contains.is_empty() && self.expect_stderr_not_contains.is_empty() @@ -917,23 +906,6 @@ impl Execs { for expect in self.expect_stderr_unordered.iter() { compare::match_unordered(expect, stderr, cwd)?; } - for expect in self.expect_either_contains.iter() { - let match_std = compare::match_contains(expect, stdout, cwd); - let match_err = compare::match_contains(expect, stderr, cwd); - if let (Err(_), Err(_)) = (match_std, match_err) { - bail!( - "expected to find:\n\ - {}\n\n\ - did not find in either output. - --- stdout\n{}\n - --- stderr\n{}\n", - expect, - stdout, - stderr, - ); - } - } - for (with, without) in self.expect_stderr_with_without.iter() { compare::match_with_without(stderr, with, without, cwd)?; } @@ -967,7 +939,6 @@ pub fn execs() -> Execs { expect_exit_code: Some(0), expect_stdout_contains: Vec::new(), expect_stderr_contains: Vec::new(), - expect_either_contains: Vec::new(), expect_stdout_contains_n: Vec::new(), expect_stdout_not_contains: Vec::new(), expect_stderr_not_contains: Vec::new(), diff --git a/tests/testsuite/bench.rs b/tests/testsuite/bench.rs index 37884a3f402..a3923dced96 100644 --- a/tests/testsuite/bench.rs +++ b/tests/testsuite/bench.rs @@ -346,12 +346,12 @@ fn cargo_bench_failing_test() { [FINISHED] bench [optimized] target(s) in [..] [RUNNING] [..] (target/release/deps/foo-[..][EXE])", ) - .with_either_contains( + .with_stdout_contains( "[..]thread '[..]' panicked at 'assertion failed: `(left == right)`[..]", ) - .with_either_contains("[..]left: `\"hello\"`[..]") - .with_either_contains("[..]right: `\"nope\"`[..]") - .with_either_contains("[..]src/main.rs:15[..]") + .with_stdout_contains("[..]left: `\"hello\"`[..]") + .with_stdout_contains("[..]right: `\"nope\"`[..]") + .with_stdout_contains("[..]src/main.rs:15[..]") .with_status(101) .run(); } From 24b8936c2036f4f19a77b9f4dd539070a20e79e7 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 15 Jun 2021 18:04:50 -0700 Subject: [PATCH 05/10] Remove normalized_lines_match, it is not needed. --- crates/cargo-test-support/src/compare.rs | 7 ------- tests/testsuite/config.rs | 21 +++++---------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index 4fa21e9c266..ebae661e287 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -512,13 +512,6 @@ pub fn lines_match(expected: &str, mut actual: &str) -> bool { actual.is_empty() || expected.ends_with("[..]") } -/// Variant of `lines_match` that applies normalization to the strings. -pub fn normalized_lines_match(expected: &str, actual: &str, cwd: Option<&Path>) -> bool { - let expected = normalize_expected(expected, cwd); - let actual = normalize_actual(actual, cwd); - lines_match(&expected, &actual) -} - /// Compares JSON object for approximate equality. /// You can use `[..]` wildcard in strings (useful for OS-dependent things such /// as paths). You can use a `"{...}"` string literal as a wildcard for diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index b416b687e30..4b817745d9c 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -5,8 +5,8 @@ use cargo::util::config::{self, Config, SslVersionConfig, StringList}; use cargo::util::interning::InternedString; use cargo::util::toml::{self, VecStringOrBool as VSOB}; use cargo::CargoResult; -use cargo_test_support::compare::normalized_lines_match; -use cargo_test_support::{paths, project, t}; +use cargo_test_support::compare; +use cargo_test_support::{panic_error, paths, project, t}; use serde::Deserialize; use std::borrow::Borrow; use std::collections::{BTreeMap, HashMap}; @@ -210,11 +210,8 @@ pub fn assert_error>(error: E, msgs: &str) { #[track_caller] pub fn assert_match(expected: &str, actual: &str) { - if !normalized_lines_match(expected, actual, None) { - panic!( - "Did not find expected:\n{}\nActual:\n{}\n", - expected, actual - ); + if let Err(e) = compare::match_exact(expected, actual, "output", "", None) { + panic_error("", e); } } @@ -277,15 +274,7 @@ f1 = 1 // It should NOT have warned for the symlink. let output = read_output(config); - let unexpected = "\ -warning: Both `[..]/.cargo/config` and `[..]/.cargo/config.toml` exist. Using `[..]/.cargo/config` -"; - if normalized_lines_match(unexpected, &output, None) { - panic!( - "Found unexpected:\n{}\nActual error:\n{}\n", - unexpected, output - ); - } + assert_eq!(output, ""); } #[cargo_test] From b9f15ab1f052b4a7f8eecfc5fc88e6d7a1c5d35f Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 15 Jun 2021 18:11:02 -0700 Subject: [PATCH 06/10] Remove got_symlink_permission, we already have one of those. --- tests/testsuite/config.rs | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 4b817745d9c..77fac2c5462 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -6,7 +6,7 @@ use cargo::util::interning::InternedString; use cargo::util::toml::{self, VecStringOrBool as VSOB}; use cargo::CargoResult; use cargo_test_support::compare; -use cargo_test_support::{panic_error, paths, project, t}; +use cargo_test_support::{panic_error, paths, project, symlink_supported, t}; use serde::Deserialize; use std::borrow::Borrow; use std::collections::{BTreeMap, HashMap}; @@ -152,28 +152,6 @@ fn write_config_toml(config: &str) { write_config_at(paths::root().join(".cargo/config.toml"), config); } -// Several test fail on windows if the user does not have permission to -// create symlinks (the `SeCreateSymbolicLinkPrivilege`). Instead of -// disabling these test on Windows, use this function to test whether we -// have permission, and return otherwise. This way, we still don't run these -// tests most of the time, but at least we do if the user has the right -// permissions. -// This function is derived from libstd fs tests. -pub fn got_symlink_permission() -> bool { - if cfg!(unix) { - return true; - } - let link = paths::root().join("some_hopefully_unique_link_name"); - let target = paths::root().join("nonexisting_target"); - - match symlink_file(&target, &link) { - Ok(_) => true, - // ERROR_PRIVILEGE_NOT_HELD = 1314 - Err(ref err) if err.raw_os_error() == Some(1314) => false, - Err(_) => true, - } -} - #[cfg(unix)] fn symlink_file(target: &Path, link: &Path) -> io::Result<()> { os::unix::fs::symlink(target, link) @@ -255,7 +233,7 @@ f1 = 1 fn config_ambiguous_filename_symlink_doesnt_warn() { // Windows requires special permissions to create symlinks. // If we don't have permission, just skip this test. - if !got_symlink_permission() { + if !symlink_supported() { return; }; From b73e3d4fa57caceb6ce025581121e36fc3a019b1 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 15 Jun 2021 19:23:17 -0700 Subject: [PATCH 07/10] Don't export lines_match. Use better high-level interfaces to achieve the same thing. --- crates/cargo-test-support/src/compare.rs | 22 ++++++------ crates/cargo-test-support/src/publish.rs | 13 ++----- tests/testsuite/build_script.rs | 46 ++++-------------------- tests/testsuite/features_namespaced.rs | 10 ++++-- tests/testsuite/lockfile_compat.rs | 27 +++++--------- tests/testsuite/publish.rs | 36 +++++++++++++++---- tests/testsuite/weak_dep_features.rs | 5 ++- 7 files changed, 69 insertions(+), 90 deletions(-) diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index ebae661e287..1d923bd6c87 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -165,7 +165,7 @@ pub fn match_exact( "{} did not match:\n\ {}\n\n\ other output:\n\ - `{}`", + {}\n", description, diffs.join("\n"), other_output, @@ -173,6 +173,14 @@ pub fn match_exact( } } +/// Convenience wrapper around [`match_exact`] which will panic on error. +#[track_caller] +pub fn assert_match_exact(expected: &str, actual: &str) { + if let Err(e) = match_exact(expected, actual, "", "", None) { + crate::panic_error("", e); + } +} + /// Checks that the given string contains the given lines, ignoring the order /// of the lines. /// @@ -487,17 +495,7 @@ fn zip_all, I2: Iterator>(a: I1, b: I2) -> Z } } -/// Compares a line with an expected pattern. -/// - Use `[..]` as a wildcard to match 0 or more characters on the same line -/// (similar to `.*` in a regex). It is non-greedy. -/// - Use `[EXE]` to optionally add `.exe` on Windows (empty string on other -/// platforms). -/// - There is a wide range of macros (such as `[COMPILING]` or `[WARNING]`) -/// to match cargo's "status" output and allows you to ignore the alignment. -/// See `substitute_macros` for a complete list of macros. -/// - `[ROOT]` the path to the test directory's root -/// - `[CWD]` is the working directory of the process that was run. -pub fn lines_match(expected: &str, mut actual: &str) -> bool { +fn lines_match(expected: &str, mut actual: &str) -> bool { for (i, part) in expected.split("[..]").enumerate() { match actual.find(part) { Some(j) => { diff --git a/crates/cargo-test-support/src/publish.rs b/crates/cargo-test-support/src/publish.rs index a0b31be2169..94f2559a779 100644 --- a/crates/cargo-test-support/src/publish.rs +++ b/crates/cargo-test-support/src/publish.rs @@ -1,4 +1,4 @@ -use crate::compare::{find_json_mismatch, lines_match}; +use crate::compare::{assert_match_exact, find_json_mismatch}; use crate::registry::{self, alt_api_path}; use flate2::read::GzDecoder; use std::collections::{HashMap, HashSet}; @@ -151,16 +151,7 @@ pub fn validate_crate_contents( let actual_contents = files .get(&full_e_name) .unwrap_or_else(|| panic!("file `{}` missing in archive", e_file_name)); - if !lines_match(e_file_contents, actual_contents) { - panic!( - "Crate contents mismatch for {:?}:\n\ - --- expected\n\ - {}\n\ - --- actual \n\ - {}\n", - e_file_name, e_file_contents, actual_contents - ); - } + assert_match_exact(e_file_contents, actual_contents); } } } diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 46ccb703e9a..da0bb4ba5eb 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -1,6 +1,6 @@ //! Tests for build.rs scripts. -use cargo_test_support::compare::lines_match; +use cargo_test_support::compare::assert_match_exact; use cargo_test_support::paths::CargoPathExt; use cargo_test_support::registry::Package; use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project}; @@ -3038,25 +3038,9 @@ fn generate_good_d_files() { println!("*.d file content*: {}", &dot_d); - #[cfg(windows)] - assert!( - lines_match( - "[..]\\target\\debug\\meow.exe: [..]\\awoo\\barkbarkbark [..]\\awoo\\build.rs[..]", - &dot_d - ) || lines_match( - "[..]\\target\\debug\\meow.exe: [..]\\awoo\\build.rs [..]\\awoo\\barkbarkbark[..]", - &dot_d - ) - ); - #[cfg(not(windows))] - assert!( - lines_match( - "[..]/target/debug/meow: [..]/awoo/barkbarkbark [..]/awoo/build.rs[..]", - &dot_d - ) || lines_match( - "[..]/target/debug/meow: [..]/awoo/build.rs [..]/awoo/barkbarkbark[..]", - &dot_d - ) + assert_match_exact( + "[..]/target/debug/meow[EXE]: [..]/awoo/barkbarkbark [..]/awoo/build.rs[..]", + &dot_d, ); // paths relative to dependency roots should not be allowed @@ -3077,25 +3061,9 @@ fn generate_good_d_files() { println!("*.d file content with dep-info-basedir*: {}", &dot_d); - #[cfg(windows)] - assert!( - lines_match( - "target\\debug\\meow.exe: [..]awoo\\barkbarkbark [..]awoo\\build.rs[..]", - &dot_d - ) || lines_match( - "target\\debug\\meow.exe: [..]awoo\\build.rs [..]awoo\\barkbarkbark[..]", - &dot_d - ) - ); - #[cfg(not(windows))] - assert!( - lines_match( - "target/debug/meow: [..]awoo/barkbarkbark [..]awoo/build.rs[..]", - &dot_d - ) || lines_match( - "target/debug/meow: [..]awoo/build.rs [..]awoo/barkbarkbark[..]", - &dot_d - ) + assert_match_exact( + "target/debug/meow[EXE]: awoo/barkbarkbark awoo/build.rs[..]", + &dot_d, ); // paths relative to dependency roots should not be allowed diff --git a/tests/testsuite/features_namespaced.rs b/tests/testsuite/features_namespaced.rs index 0c3022c8dd3..2e92b2bdb0e 100644 --- a/tests/testsuite/features_namespaced.rs +++ b/tests/testsuite/features_namespaced.rs @@ -1151,7 +1151,8 @@ fn publish_no_implicit() { &["Cargo.toml", "Cargo.toml.orig", "src/lib.rs"], &[( "Cargo.toml", - r#"[..] + &format!( + r#"{} [package] name = "foo" version = "0.1.0" @@ -1169,6 +1170,8 @@ optional = true [features] feat = ["opt-dep1"] "#, + cargo::core::package::MANIFEST_PREAMBLE + ), )], ); } @@ -1255,7 +1258,8 @@ fn publish() { &["Cargo.toml", "Cargo.toml.orig", "src/lib.rs"], &[( "Cargo.toml", - r#"[..] + &format!( + r#"{} [package] name = "foo" version = "0.1.0" @@ -1271,6 +1275,8 @@ feat1 = [] feat2 = ["dep:bar"] feat3 = ["feat2"] "#, + cargo::core::package::MANIFEST_PREAMBLE + ), )], ); } diff --git a/tests/testsuite/lockfile_compat.rs b/tests/testsuite/lockfile_compat.rs index 8955beaae04..087e28ac929 100644 --- a/tests/testsuite/lockfile_compat.rs +++ b/tests/testsuite/lockfile_compat.rs @@ -1,6 +1,6 @@ //! Tests for supporting older versions of the Cargo.lock file format. -use cargo_test_support::compare::lines_match; +use cargo_test_support::compare::assert_match_exact; use cargo_test_support::git; use cargo_test_support::registry::Package; use cargo_test_support::{basic_lib_manifest, basic_manifest, project}; @@ -13,15 +13,6 @@ fn oldest_lockfile_still_works() { } } -#[track_caller] -fn assert_lockfiles_eq(expected: &str, actual: &str) { - for (l, r) in expected.lines().zip(actual.lines()) { - assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r); - } - - assert_eq!(expected.lines().count(), actual.lines().count()); -} - fn oldest_lockfile_still_works_with_command(cargo_command: &str) { Package::new("bar", "0.1.0").publish(); @@ -77,7 +68,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" p.cargo(cargo_command).run(); let lock = p.read_lockfile(); - assert_lockfiles_eq(expected_lockfile, &lock); + assert_match_exact(expected_lockfile, &lock); } #[cargo_test] @@ -123,7 +114,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" p.cargo("build --locked").run(); let lock = p.read_lockfile(); - assert_lockfiles_eq(&old_lockfile, &lock); + assert_match_exact(&old_lockfile, &lock); } #[cargo_test] @@ -170,7 +161,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" p.cargo("build").run(); let lock = p.read_lockfile(); - assert_lockfiles_eq( + assert_match_exact( r#"# This file is automatically @generated by Cargo. # It is not intended for manual editing. version = 3 @@ -428,7 +419,7 @@ dependencies = [ \"bar\", ] "; - assert_lockfiles_eq(expected, &actual); + assert_match_exact(expected, &actual); } #[cargo_test] @@ -472,7 +463,7 @@ dependencies = [ p.cargo("build").run(); let lock = p.read_lockfile(); - assert_lockfiles_eq( + assert_match_exact( r#"# [..] # [..] version = 3 @@ -569,7 +560,7 @@ dependencies = [ p.cargo("fetch").run(); let lock = p.read_lockfile(); - assert_lockfiles_eq(&lockfile, &lock); + assert_match_exact(&lockfile, &lock); } #[cargo_test] @@ -640,7 +631,7 @@ dependencies = [ p.cargo("fetch").run(); let lock = p.read_lockfile(); - assert_lockfiles_eq(&lockfile, &lock); + assert_match_exact(&lockfile, &lock); } #[cargo_test] @@ -696,7 +687,7 @@ dependencies = [ p.cargo("fetch").run(); let lock = p.read_lockfile(); - assert_lockfiles_eq(&lockfile, &lock); + assert_match_exact(&lockfile, &lock); } #[cargo_test] diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 95addc04c35..f05ca6d2847 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -1213,22 +1213,41 @@ fn publish_git_with_version() { ( "Cargo.toml", // Check that only `version` is included in Cargo.toml. - "[..]\n\ - [dependencies.dep1]\n\ - version = \"1.0\"\n\ - ", + &format!( + "{}\n\ + [package]\n\ + edition = \"2018\"\n\ + name = \"foo\"\n\ + version = \"0.1.0\"\n\ + authors = []\n\ + description = \"foo\"\n\ + license = \"MIT\"\n\ + [dependencies.dep1]\n\ + version = \"1.0\"\n\ + ", + cargo::core::package::MANIFEST_PREAMBLE + ), ), ( "Cargo.lock", // The important check here is that it is 1.0.1 in the registry. - "[..]\n\ + "# This file is automatically @generated by Cargo.\n\ + # It is not intended for manual editing.\n\ + version = 3\n\ + \n\ + [[package]]\n\ + name = \"dep1\"\n\ + version = \"1.0.1\"\n\ + source = \"registry+https://github.com/rust-lang/crates.io-index\"\n\ + checksum = \"[..]\"\n\ + \n\ [[package]]\n\ name = \"foo\"\n\ version = \"0.1.0\"\n\ dependencies = [\n\ \x20\"dep1\",\n\ ]\n\ - [..]", + ", ), ], ); @@ -1297,7 +1316,8 @@ fn publish_dev_dep_no_version() { &["Cargo.toml", "Cargo.toml.orig", "src/lib.rs"], &[( "Cargo.toml", - r#"[..] + &format!( + r#"{} [package] name = "foo" version = "0.1.0" @@ -1310,6 +1330,8 @@ repository = "foo" [dev-dependencies] "#, + cargo::core::package::MANIFEST_PREAMBLE + ), )], ); } diff --git a/tests/testsuite/weak_dep_features.rs b/tests/testsuite/weak_dep_features.rs index d51c407727f..4d3bf8b1a4c 100644 --- a/tests/testsuite/weak_dep_features.rs +++ b/tests/testsuite/weak_dep_features.rs @@ -702,7 +702,8 @@ fn publish() { &["Cargo.toml", "Cargo.toml.orig", "src/lib.rs"], &[( "Cargo.toml", - r#"[..] + &format!( + r#"{} [package] name = "foo" version = "0.1.0" @@ -717,6 +718,8 @@ optional = true feat1 = [] feat2 = ["bar?/feat"] "#, + cargo::core::package::MANIFEST_PREAMBLE + ), )], ); } From aea5ca3ca0fb15d8cb96738e2aac47816b394c77 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 16 Jun 2021 09:41:38 -0700 Subject: [PATCH 08/10] Remove the double-backslash escape for matching. Using `with_json` is safer since it knows what JSON escaping is. --- crates/cargo-test-support/src/compare.rs | 4 +--- tests/testsuite/cargo_command.rs | 6 ++--- tests/testsuite/locate_project.rs | 30 +++++++----------------- tests/testsuite/verify_project.rs | 2 +- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index 1d923bd6c87..3032319c07b 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -58,9 +58,7 @@ fn normalize_expected(expected: &str, cwd: Option<&Path>) -> String { /// Normalizes text for both actual and expected strings. fn normalize_common(text: &str, cwd: Option<&Path>) -> String { // Let's not deal with / vs \ (windows...) - // First replace backslash-escaped backslashes with forward slashes - // which can occur in, for example, JSON output - let text = text.replace("\\\\", "/").replace('\\', "/"); + 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 diff --git a/tests/testsuite/cargo_command.rs b/tests/testsuite/cargo_command.rs index 5e26a795c64..492f4c0aae9 100644 --- a/tests/testsuite/cargo_command.rs +++ b/tests/testsuite/cargo_command.rs @@ -313,7 +313,7 @@ fn cargo_subcommand_args() { r#" fn main() { let args: Vec<_> = ::std::env::args().collect(); - println!("{:?}", args); + println!("{}", args.join(" ")); } "#, ) @@ -329,9 +329,7 @@ fn cargo_subcommand_args() { cargo_process("foo bar -v --help") .env("PATH", &path) - .with_stdout( - r#"["[CWD]/cargo-foo/target/debug/cargo-foo[EXE]", "foo", "bar", "-v", "--help"]"#, - ) + .with_stdout("[CWD]/cargo-foo/target/debug/cargo-foo[EXE] foo bar -v --help") .run(); } diff --git a/tests/testsuite/locate_project.rs b/tests/testsuite/locate_project.rs index adff37516cd..7e8ceb4c67a 100644 --- a/tests/testsuite/locate_project.rs +++ b/tests/testsuite/locate_project.rs @@ -5,28 +5,22 @@ use cargo_test_support::project; #[cargo_test] fn simple() { let p = project().build(); - let root_manifest_path = p.root().join("Cargo.toml"); p.cargo("locate-project") - .with_stdout(format!( - r#"{{"root":"{}"}}"#, - root_manifest_path.to_str().unwrap() - )) + .with_json(r#"{"root": "[ROOT]/foo/Cargo.toml"}"#) .run(); } #[cargo_test] fn message_format() { let p = project().build(); - let root_manifest_path = p.root().join("Cargo.toml"); - let root_str = root_manifest_path.to_str().unwrap(); p.cargo("locate-project --message-format plain") - .with_stdout(root_str) + .with_stdout("[ROOT]/foo/Cargo.toml") .run(); p.cargo("locate-project --message-format json") - .with_stdout(format!(r#"{{"root":"{}"}}"#, root_str)) + .with_json(r#"{"root": "[ROOT]/foo/Cargo.toml"}"#) .run(); p.cargo("locate-project --message-format cryptic") @@ -61,28 +55,22 @@ fn workspace() { .file("inner/src/lib.rs", "") .build(); - let outer_manifest = format!( - r#"{{"root":"{}"}}"#, - p.root().join("Cargo.toml").to_str().unwrap(), - ); - let inner_manifest = format!( - r#"{{"root":"{}"}}"#, - p.root().join("inner").join("Cargo.toml").to_str().unwrap(), - ); + let outer_manifest = r#"{"root": "[ROOT]/foo/Cargo.toml"}"#; + let inner_manifest = r#"{"root": "[ROOT]/foo/inner/Cargo.toml"}"#; - p.cargo("locate-project").with_stdout(&outer_manifest).run(); + p.cargo("locate-project").with_json(outer_manifest).run(); p.cargo("locate-project") .cwd("inner") - .with_stdout(&inner_manifest) + .with_json(inner_manifest) .run(); p.cargo("locate-project --workspace") - .with_stdout(&outer_manifest) + .with_json(outer_manifest) .run(); p.cargo("locate-project --workspace") .cwd("inner") - .with_stdout(&outer_manifest) + .with_json(outer_manifest) .run(); } diff --git a/tests/testsuite/verify_project.rs b/tests/testsuite/verify_project.rs index d486101ef71..3769aefa94d 100644 --- a/tests/testsuite/verify_project.rs +++ b/tests/testsuite/verify_project.rs @@ -68,6 +68,6 @@ fn cargo_verify_project_honours_unstable_features() { p.cargo("verify-project") .with_status(1) - .with_stdout(r#"{"invalid":"failed to parse manifest at `[CWD]/Cargo.toml`"}"#) + .with_json(r#"{"invalid":"failed to parse manifest at `[CWD]/Cargo.toml`"}"#) .run(); } From 205148e6452c2acf092ead809043486a7822c9a1 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 16 Jun 2021 10:28:43 -0700 Subject: [PATCH 09/10] Only normalize paths on windows. I don't trust that all these transformations won't have unintended consequences on other platforms. It is nice to verify there aren't any backslash shenanigans on other platforms. --- crates/cargo-test-support/src/compare.rs | 25 ++++++++++++++++++------ tests/testsuite/bad_config.rs | 8 ++++---- tests/testsuite/cargo_config.rs | 8 ++++---- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index 3032319c07b..bd62e79b768 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -44,19 +44,32 @@ 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', ""); - // Let's not deal with \r\n vs \n on windows... - let actual = actual.replace('\r', ""); - normalize_common(&actual, cwd) + 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 + } } /// 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 = substitute_macros(expected); - normalize_common(&expected, cwd) + 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 + } } -/// Normalizes text for both actual and expected strings. -fn normalize_common(text: &str, cwd: Option<&Path>) -> String { +/// 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('\\', "/"); diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index a8aacc737c1..be242bff22b 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -1391,16 +1391,16 @@ fn bad_target_cfg() { .with_stderr( "\ [ERROR] error in [..]/foo/.cargo/config: \ -could not load config key `target.\"cfg(not(target_os = /\"none/\"))\".runner` +could not load config key `target.\"cfg(not(target_os = \\\"none\\\"))\".runner` Caused by: error in [..]/foo/.cargo/config: \ - could not load config key `target.\"cfg(not(target_os = /\"none/\"))\".runner` + could not load config key `target.\"cfg(not(target_os = \\\"none\\\"))\".runner` Caused by: - invalid configuration for key `target.\"cfg(not(target_os = /\"none/\"))\".runner` + invalid configuration for key `target.\"cfg(not(target_os = \\\"none\\\"))\".runner` expected a string or array of strings, but found a boolean for \ - `target.\"cfg(not(target_os = /\"none/\"))\".runner` in [..]/foo/.cargo/config + `target.\"cfg(not(target_os = \\\"none\\\"))\".runner` in [..]/foo/.cargo/config ", ) .run(); diff --git a/tests/testsuite/cargo_config.rs b/tests/testsuite/cargo_config.rs index e130080097d..a11e9afc58f 100644 --- a/tests/testsuite/cargo_config.rs +++ b/tests/testsuite/cargo_config.rs @@ -86,7 +86,7 @@ build.rustflags = [\"--flag-directory\", \"--flag-global\"] extra-table.somekey = \"somevalue\" profile.dev.opt-level = 3 profile.dev.package.foo.opt-level = 1 -target.\"cfg(target_os = /\"linux/\")\".runner = \"runme\" +target.\"cfg(target_os = \\\"linux\\\")\".runner = \"runme\" # The following environment variables may affect the loaded values. # CARGO_ALIAS_BAR=[..]cat dog[..] # CARGO_BUILD_JOBS=100 @@ -263,7 +263,7 @@ build.rustflags = [ extra-table.somekey = \"somevalue\" # [ROOT]/home/.cargo/config.toml profile.dev.opt-level = 3 # [ROOT]/home/.cargo/config.toml profile.dev.package.foo.opt-level = 1 # [ROOT]/home/.cargo/config.toml -target.\"cfg(target_os = /\"linux/\")\".runner = \"runme\" # [ROOT]/home/.cargo/config.toml +target.\"cfg(target_os = \\\"linux\\\")\".runner = \"runme\" # [ROOT]/home/.cargo/config.toml # The following environment variables may affect the loaded values. # CARGO_HOME=[ROOT]/home/.cargo ", @@ -359,7 +359,7 @@ build.rustflags = [\"--flag-global\"] extra-table.somekey = \"somevalue\" profile.dev.opt-level = 3 profile.dev.package.foo.opt-level = 1 -target.\"cfg(target_os = /\"linux/\")\".runner = \"runme\" +target.\"cfg(target_os = \\\"linux\\\")\".runner = \"runme\" ", ) @@ -513,7 +513,7 @@ build.rustflags = [\"--flag-global\"] extra-table.somekey = \"somevalue\" profile.dev.opt-level = 3 profile.dev.package.foo.opt-level = 1 -target.\"cfg(target_os = /\"linux/\")\".runner = \"runme\" +target.\"cfg(target_os = \\\"linux\\\")\".runner = \"runme\" ", ) From 16b5402fd7f22bfaec73ba9063fc63a4fbf3b370 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 16 Jun 2021 15:40:09 -0700 Subject: [PATCH 10/10] testsuite: Switch to colored diffs with Myers diff. --- crates/cargo-test-support/Cargo.toml | 1 + crates/cargo-test-support/src/compare.rs | 308 +++++++++++------------ crates/cargo-test-support/src/diff.rs | 174 +++++++++++++ crates/cargo-test-support/src/lib.rs | 1 + tests/testsuite/doc.rs | 4 +- tests/testsuite/metabuild.rs | 2 +- 6 files changed, 325 insertions(+), 165 deletions(-) create mode 100644 crates/cargo-test-support/src/diff.rs diff --git a/crates/cargo-test-support/Cargo.toml b/crates/cargo-test-support/Cargo.toml index 99f8d271785..7329441aaf5 100644 --- a/crates/cargo-test-support/Cargo.toml +++ b/crates/cargo-test-support/Cargo.toml @@ -21,5 +21,6 @@ lazy_static = "1.0" remove_dir_all = "0.5" serde_json = "1.0" tar = { version = "0.4.18", default-features = false } +termcolor = "1.1.2" toml = "0.5.7" url = "2.2.2" diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index bd62e79b768..b1b5c3fa448 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -31,10 +31,12 @@ //! a problem. //! - Carriage returns are removed, which can help when running on Windows. +use crate::diff; use crate::paths; use anyhow::{bail, Context, Result}; use serde_json::Value; use std::env; +use std::fmt; use std::path::Path; use std::str; use url::Url; @@ -165,23 +167,21 @@ pub fn match_exact( ) -> Result<()> { let expected = normalize_expected(expected, cwd); let actual = normalize_actual(actual, cwd); - let e = expected.lines(); - let a = actual.lines(); - - let diffs = diff_lines(a, e, false); - if diffs.is_empty() { - Ok(()) - } else { - bail!( - "{} did not match:\n\ - {}\n\n\ - other output:\n\ - {}\n", - description, - diffs.join("\n"), - other_output, - ) + let e: Vec<_> = expected.lines().map(WildStr::new).collect(); + let a: Vec<_> = actual.lines().map(WildStr::new).collect(); + if e == a { + return Ok(()); } + let diff = diff::colored_diff(&e, &a); + bail!( + "{} did not match:\n\ + {}\n\n\ + other output:\n\ + {}\n", + description, + diff, + other_output, + ); } /// Convenience wrapper around [`match_exact`] which will panic on error. @@ -199,7 +199,8 @@ pub fn assert_match_exact(expected: &str, actual: &str) { pub fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { let expected = normalize_expected(expected, cwd); let actual = normalize_actual(actual, cwd); - let mut a = actual.lines().collect::>(); + 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, @@ -207,31 +208,35 @@ pub fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Resu // 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.len()); - let mut failures = Vec::new(); - - for e_line in expected.lines() { - match a.iter().position(|a_line| lines_match(e_line, a_line)) { + 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) => { - a.remove(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)); } - None => failures.push(e_line), } } - if !failures.is_empty() { - bail!( - "Did not find expected line(s):\n{}\n\ - Remaining available output:\n{}\n", - failures.join("\n"), - a.join("\n") - ); + for unmatched in a { + failure = true; + changes.push(Change::Add(a_index, unmatched)); + a_index += 1; } - if !a.is_empty() { + if failure { bail!( - "Output included extra lines:\n\ - {}\n", - a.join("\n") - ) + "Expected lines did not match (ignoring order):\n{}\n", + diff::render_colored_changes(&changes) + ); } else { Ok(()) } @@ -244,28 +249,24 @@ pub fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Resu pub fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> { let expected = normalize_expected(expected, cwd); let actual = normalize_actual(actual, cwd); - let e = expected.lines(); - let mut a = actual.lines(); - - let mut diffs = diff_lines(a.clone(), e.clone(), true); - while a.next().is_some() { - let a = diff_lines(a.clone(), e.clone(), true); - if a.len() < diffs.len() { - diffs = a; - } + 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 { + bail!("expected length must not be zero"); } - if diffs.is_empty() { - Ok(()) - } else { - bail!( - "expected to find:\n\ - {}\n\n\ - did not find in output:\n\ - {}", - expected, - actual - ) + for window in a.windows(e.len()) { + if window == e { + return Ok(()); + } } + bail!( + "expected to find:\n\ + {}\n\n\ + did not find in output:\n\ + {}", + expected, + actual + ); } /// Checks that the given string does not contain the given contiguous lines @@ -299,28 +300,23 @@ pub fn match_contains_n( ) -> Result<()> { let expected = normalize_expected(expected, cwd); let actual = normalize_actual(actual, cwd); - let e = expected.lines(); - let mut a = actual.lines(); - - let mut matches = 0; - - while let Some(..) = { - if diff_lines(a.clone(), e.clone(), true).is_empty() { - matches += 1; - } - a.next() - } {} - + 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 { + bail!("expected length must not be zero"); + } + let matches = a.windows(e.len()).filter(|window| *window == e).count(); if matches == number { Ok(()) } else { bail!( - "expected to find {} occurrences:\n\ + "expected to find {} occurrences of:\n\ {}\n\n\ - did not find in output:\n\ + but found {} matches in the output:\n\ {}", number, expected, + matches, actual ) } @@ -340,16 +336,17 @@ pub fn match_with_without( cwd: Option<&Path>, ) -> Result<()> { let actual = normalize_actual(actual, cwd); - let contains = |s, line| { - let mut s = normalize_expected(s, cwd); - s.insert_str(0, "[..]"); - s.push_str("[..]"); - lines_match(&s, line) - }; - let matches: Vec<&str> = actual + let norm = |s: &String| format!("[..]{}[..]", normalize_expected(s, cwd)); + 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(); + let without_wild: Vec<_> = without.iter().map(|w| WildStr::new(w)).collect(); + + let matches: Vec<_> = actual .lines() - .filter(|line| with.iter().all(|with| contains(with, line))) - .filter(|line| !without.iter().any(|without| contains(without, line))) + .map(WildStr::new) + .filter(|line| with_wild.iter().all(|with| with == line)) + .filter(|line| !without_wild.iter().any(|without| without == line)) .collect(); match matches.len() { 0 => bail!( @@ -371,7 +368,7 @@ pub fn match_with_without( {}\n", with, without, - matches.join("\n") + itertools::join(matches, "\n") ), } } @@ -454,73 +451,6 @@ fn collect_json_objects( Ok((expected_objs, actual_objs)) } -fn diff_lines<'a>(actual: str::Lines<'a>, expected: str::Lines<'a>, partial: bool) -> Vec { - let actual = actual.take(if partial { - expected.clone().count() - } else { - usize::MAX - }); - zip_all(actual, expected) - .enumerate() - .filter_map(|(i, (a, e))| match (a, e) { - (Some(a), Some(e)) => { - if lines_match(e, a) { - None - } else { - Some(format!("{:3} - |{}|\n + |{}|\n", i, e, a)) - } - } - (Some(a), None) => Some(format!("{:3} -\n + |{}|\n", i, a)), - (None, Some(e)) => Some(format!("{:3} - |{}|\n +\n", i, e)), - (None, None) => unreachable!(), - }) - .collect() -} - -struct ZipAll { - first: I1, - second: I2, -} - -impl, I2: Iterator> Iterator for ZipAll { - type Item = (Option, Option); - fn next(&mut self) -> Option<(Option, Option)> { - let first = self.first.next(); - let second = self.second.next(); - - match (first, second) { - (None, None) => None, - (a, b) => Some((a, b)), - } - } -} - -/// Returns an iterator, similar to `zip`, but exhausts both iterators. -/// -/// Each element is `(Option, Option)` where `None` indicates an -/// iterator ended early. -fn zip_all, I2: Iterator>(a: I1, b: I2) -> ZipAll { - ZipAll { - first: a, - second: b, - } -} - -fn lines_match(expected: &str, mut actual: &str) -> bool { - for (i, part) in expected.split("[..]").enumerate() { - match actual.find(part) { - Some(j) => { - if i == 0 && j != 0 { - return false; - } - actual = &actual[j + part.len()..]; - } - None => return false, - } - } - actual.is_empty() || expected.ends_with("[..]") -} - /// Compares JSON object for approximate equality. /// You can use `[..]` wildcard in strings (useful for OS-dependent things such /// as paths). You can use a `"{...}"` string literal as a wildcard for @@ -550,12 +480,10 @@ fn find_json_mismatch_r<'a>( (&Bool(l), &Bool(r)) if l == r => None, (&String(ref l), _) if l == "{...}" => None, (&String(ref l), &String(ref r)) => { - let l = normalize_expected(l, cwd); - let r = normalize_actual(r, cwd); - if lines_match(&l, &r) { - None - } else { + if match_exact(l, r, "", "", cwd).is_err() { Some((expected, actual)) + } else { + None } } (&Array(ref l), &Array(ref r)) => { @@ -585,15 +513,71 @@ fn find_json_mismatch_r<'a>( } } +/// A single line string that supports `[..]` wildcard matching. +pub struct WildStr<'a> { + has_meta: bool, + line: &'a str, +} + +impl<'a> WildStr<'a> { + pub fn new(line: &'a str) -> WildStr<'a> { + WildStr { + has_meta: line.contains("[..]"), + line, + } + } +} + +impl<'a> PartialEq for WildStr<'a> { + fn eq(&self, other: &Self) -> bool { + match (self.has_meta, other.has_meta) { + (false, false) => self.line == other.line, + (true, false) => meta_cmp(self.line, other.line), + (false, true) => meta_cmp(other.line, self.line), + (true, true) => panic!("both lines cannot have [..]"), + } + } +} + +fn meta_cmp(a: &str, mut b: &str) -> bool { + for (i, part) in a.split("[..]").enumerate() { + match b.find(part) { + Some(j) => { + if i == 0 && j != 0 { + return false; + } + b = &b[j + part.len()..]; + } + None => return false, + } + } + b.is_empty() || a.ends_with("[..]") +} + +impl fmt::Display for WildStr<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.line) + } +} + +impl fmt::Debug for WildStr<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self.line) + } +} + #[test] -fn lines_match_works() { - assert!(lines_match("a b", "a b")); - assert!(lines_match("a[..]b", "a b")); - assert!(lines_match("a[..]", "a b")); - assert!(lines_match("[..]", "a b")); - assert!(lines_match("[..]b", "a b")); - - assert!(!lines_match("[..]b", "c")); - assert!(!lines_match("b", "c")); - assert!(!lines_match("b", "cb")); +fn wild_str_cmp() { + for (a, b) in &[ + ("a b", "a b"), + ("a[..]b", "a b"), + ("a[..]", "a b"), + ("[..]", "a b"), + ("[..]b", "a b"), + ] { + assert_eq!(WildStr::new(a), WildStr::new(b)); + } + for (a, b) in &[("[..]b", "c"), ("b", "c"), ("b", "cb")] { + assert_ne!(WildStr::new(a), WildStr::new(b)); + } } diff --git a/crates/cargo-test-support/src/diff.rs b/crates/cargo-test-support/src/diff.rs new file mode 100644 index 00000000000..f3b283b109f --- /dev/null +++ b/crates/cargo-test-support/src/diff.rs @@ -0,0 +1,174 @@ +//! 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; +use termcolor::{Ansi, Color, ColorSpec, NoColor, WriteColor}; + +/// 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 diff<'a, T>(a: &'a [T], b: &'a [T]) -> Vec> +where + T: PartialEq, +{ + if a.is_empty() && b.is_empty() { + return vec![]; + } + let mut diff = vec![]; + for (prev_x, prev_y, x, y) in backtrack(&a, &b) { + if x == prev_x { + diff.push(Change::Add(prev_y + 1, &b[prev_y])); + } else if y == prev_y { + diff.push(Change::Remove(prev_x + 1, &a[prev_x])); + } else { + diff.push(Change::Keep(prev_x + 1, prev_y + 1, &a[prev_x])); + } + } + diff.reverse(); + diff +} + +fn shortest_edit(a: &[T], b: &[T]) -> Vec> +where + T: PartialEq, +{ + let max = a.len() + b.len(); + let mut v = vec![0; 2 * max + 1]; + let mut trace = vec![]; + for d in 0..=max { + trace.push(v.clone()); + for k in (0..=(2 * d)).step_by(2) { + let mut x = if k == 0 || (k != 2 * d && v[max - d + k - 1] < v[max - d + k + 1]) { + // Move down + v[max - d + k + 1] + } else { + // Move right + v[max - d + k - 1] + 1 + }; + let mut y = x + d - k; + // Step diagonally as far as possible. + while x < a.len() && y < b.len() && a[x] == b[y] { + x += 1; + y += 1; + } + v[max - d + k] = x; + // Return if reached the bottom-right position. + if x >= a.len() && y >= b.len() { + return trace; + } + } + } + panic!("finished without hitting end?"); +} + +fn backtrack(a: &[T], b: &[T]) -> Vec<(usize, usize, usize, usize)> +where + T: PartialEq, +{ + let mut result = vec![]; + let mut x = a.len(); + let mut y = b.len(); + let max = x + y; + for (d, v) in shortest_edit(a, b).iter().enumerate().rev() { + let k = x + d - y; + let prev_k = if k == 0 || (k != 2 * d && v[max - d + k - 1] < v[max - d + k + 1]) { + k + 1 + } else { + k - 1 + }; + let prev_x = v[max - d + prev_k]; + let prev_y = (prev_x + d).saturating_sub(prev_k); + while x > prev_x && y > prev_y { + result.push((x - 1, y - 1, x, y)); + x -= 1; + y -= 1; + } + if d > 0 { + result.push((prev_x, prev_y, x, y)); + } + x = prev_x; + y = prev_y; + } + return result; +} + +pub fn colored_diff<'a, T>(a: &'a [T], b: &'a [T]) -> String +where + T: PartialEq + fmt::Display, +{ + let changes = diff(a, b); + render_colored_changes(&changes) +} + +pub fn render_colored_changes(changes: &[Change]) -> String { + // termcolor is not very ergonomic, but I don't want to bring in another dependency. + let mut red = ColorSpec::new(); + red.set_fg(Some(Color::Red)); + let mut green = ColorSpec::new(); + green.set_fg(Some(Color::Green)); + let mut dim = ColorSpec::new(); + dim.set_dimmed(true); + let mut v = Vec::new(); + let mut result: Box = if crate::is_ci() { + // Don't use color on CI. Even though GitHub can display colors, it + // makes reading the raw logs more difficult. + Box::new(NoColor::new(&mut v)) + } else { + Box::new(Ansi::new(&mut v)) + }; + + 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), + }; + result.set_color(&dim).unwrap(); + write!(result, "{}", nums).unwrap(); + let mut bold = color.clone(); + bold.set_bold(true); + result.set_color(&bold).unwrap(); + write!(result, "{}", sign).unwrap(); + result.reset().unwrap(); + result.set_color(&color).unwrap(); + write!(result, "{}", text).unwrap(); + result.reset().unwrap(); + writeln!(result).unwrap(); + } + drop(result); + String::from_utf8(v).unwrap() +} + +#[cfg(test)] +pub fn compare(a: &str, b: &str) { + let a: Vec<_> = a.chars().collect(); + let b: Vec<_> = b.chars().collect(); + let changes = diff(&a, &b); + let mut result = vec![]; + for change in changes { + match change { + Change::Add(_, s) => result.push(*s), + Change::Remove(_, _s) => {} + Change::Keep(_, _, s) => result.push(*s), + } + } + assert_eq!(b, result); +} + +#[test] +fn basic_tests() { + compare("", ""); + compare("A", ""); + compare("", "B"); + compare("ABCABBA", "CBABAC"); +} diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 8082ba61fd8..992446f7fc8 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -52,6 +52,7 @@ pub use cargo_test_macro::cargo_test; pub mod compare; pub mod cross_compile; +mod diff; pub mod git; pub mod install; pub mod paths; diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 1dcf7735b6d..f376f62d6b5 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1470,8 +1470,8 @@ fn doc_message_format() { "children": "{...}", "code": "{...}", "level": "error", - "message": "[..]", - "rendered": "[..]", + "message": "{...}", + "rendered": "{...}", "spans": "{...}" }, "package_id": "foo [..]", diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs index d23c171daff..96fcbd75dd6 100644 --- a/tests/testsuite/metabuild.rs +++ b/tests/testsuite/metabuild.rs @@ -737,7 +737,7 @@ fn metabuild_failed_build_json() { "code": "{...}", "level": "error", "message": "cannot find function `metabuild` in [..] `mb`", - "rendered": "[..]", + "rendered": "{...}", "spans": "{...}" }, "package_id": "foo [..]",