Skip to content

Commit

Permalink
Auto merge of #14642 - epage:test-publish, r=weihanglo
Browse files Browse the repository at this point in the history
test: Migrate publish snapshotting to snapbox

### What does this PR try to resolve?

This is part of #14039 and allows snapshotting in more places.

### How should we test and review this PR?

### Additional information

`InMemoryDir` is taken from some experiments I've been doing in snapshot for filesystem assertions.  That got held up because of design complexity but publish validation's needs are simpler than I was designing for.
  • Loading branch information
bors committed Oct 16, 2024
2 parents ca14836 + 5b84fc9 commit 2835ddc
Show file tree
Hide file tree
Showing 15 changed files with 665 additions and 450 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ cargo-credential-macos-keychain = { version = "0.4.7", path = "credential/cargo-
cargo-credential-wincred = { version = "0.4.7", path = "credential/cargo-credential-wincred" }
cargo-platform = { path = "crates/cargo-platform", version = "0.1.5" }
cargo-test-macro = { version = "0.3.0", path = "crates/cargo-test-macro" }
cargo-test-support = { version = "0.5.0", path = "crates/cargo-test-support" }
cargo-test-support = { version = "0.6.0", path = "crates/cargo-test-support" }
cargo-util = { version = "0.2.14", path = "crates/cargo-util" }
cargo-util-schemas = { version = "0.7.0", path = "crates/cargo-util-schemas" }
cargo_metadata = "0.18.1"
Expand Down
2 changes: 1 addition & 1 deletion crates/cargo-test-support/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-test-support"
version = "0.5.0"
version = "0.6.0"
edition.workspace = true
rust-version = "1.81" # MSRV:1
license.workspace = true
Expand Down
182 changes: 142 additions & 40 deletions crates/cargo-test-support/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ use crate::cross_compile::try_alternate;
use crate::paths;
use crate::{diff, rustc_host};
use anyhow::{bail, Result};
use snapbox::Data;
use snapbox::IntoData;
use std::fmt;
use std::path::Path;
use std::path::PathBuf;
use std::str;
use url::Url;

Expand Down Expand Up @@ -428,46 +431,6 @@ fn substitute_macros(input: &str) -> String {
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(crate) 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: 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.
#[track_caller]
pub(crate) 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.
///
Expand Down Expand Up @@ -706,6 +669,145 @@ impl fmt::Debug for WildStr<'_> {
}
}

pub struct InMemoryDir {
files: Vec<(PathBuf, Data)>,
}

impl InMemoryDir {
pub fn paths(&self) -> impl Iterator<Item = &Path> {
self.files.iter().map(|(p, _)| p.as_path())
}

#[track_caller]
pub fn assert_contains(&self, expected: &Self) {
use std::fmt::Write as _;
let assert = assert_e2e();
let mut errs = String::new();
for (path, expected_data) in &expected.files {
let actual_data = self
.files
.iter()
.find_map(|(p, d)| (path == p).then(|| d.clone()))
.unwrap_or_else(|| Data::new());
if let Err(err) =
assert.try_eq(Some(&path.display()), actual_data, expected_data.clone())
{
let _ = write!(&mut errs, "{err}");
}
}
if !errs.is_empty() {
panic!("{errs}")
}
}
}

impl<P, D> FromIterator<(P, D)> for InMemoryDir
where
P: Into<std::path::PathBuf>,
D: IntoData,
{
fn from_iter<I: IntoIterator<Item = (P, D)>>(files: I) -> Self {
let files = files
.into_iter()
.map(|(p, d)| (p.into(), d.into_data()))
.collect();
Self { files }
}
}

impl<const N: usize, P, D> From<[(P, D); N]> for InMemoryDir
where
P: Into<PathBuf>,
D: IntoData,
{
fn from(files: [(P, D); N]) -> Self {
let files = files
.into_iter()
.map(|(p, d)| (p.into(), d.into_data()))
.collect();
Self { files }
}
}

impl<P, D> From<std::collections::HashMap<P, D>> for InMemoryDir
where
P: Into<PathBuf>,
D: IntoData,
{
fn from(files: std::collections::HashMap<P, D>) -> Self {
let files = files
.into_iter()
.map(|(p, d)| (p.into(), d.into_data()))
.collect();
Self { files }
}
}

impl<P, D> From<std::collections::BTreeMap<P, D>> for InMemoryDir
where
P: Into<PathBuf>,
D: IntoData,
{
fn from(files: std::collections::BTreeMap<P, D>) -> Self {
let files = files
.into_iter()
.map(|(p, d)| (p.into(), d.into_data()))
.collect();
Self { files }
}
}

impl From<()> for InMemoryDir {
fn from(_files: ()) -> Self {
let files = Vec::new();
Self { files }
}
}

/// Create an `impl _ for InMemoryDir` for a generic tuple
///
/// Must pass in names for each tuple parameter for
/// - internal variable name
/// - `Path` type
/// - `Data` type
macro_rules! impl_from_tuple_for_inmemorydir {
($($var:ident $path:ident $data:ident),+) => {
impl<$($path: Into<PathBuf>, $data: IntoData),+> From<($(($path, $data)),+ ,)> for InMemoryDir {
fn from(files: ($(($path, $data)),+,)) -> Self {
let ($($var),+ ,) = files;
let files = [$(($var.0.into(), $var.1.into_data())),+];
files.into()
}
}
};
}

/// Extend `impl_from_tuple_for_inmemorydir`` to generate for the specified tuple and all smaller
/// tuples
macro_rules! impl_from_tuples_for_inmemorydir {
($var1:ident $path1:ident $data1:ident, $($var:ident $path:ident $data:ident),+) => {
impl_from_tuples_for_inmemorydir!(__impl $var1 $path1 $data1; $($var $path $data),+);
};
(__impl $($var:ident $path:ident $data:ident),+; $var1:ident $path1:ident $data1:ident $(,$var2:ident $path2:ident $data2:ident)*) => {
impl_from_tuple_for_inmemorydir!($($var $path $data),+);
impl_from_tuples_for_inmemorydir!(__impl $($var $path $data),+, $var1 $path1 $data1; $($var2 $path2 $data2),*);
};
(__impl $($var:ident $path:ident $data:ident),+;) => {
impl_from_tuple_for_inmemorydir!($($var $path $data),+);
}
}

// Generate for tuples of size `1..=7`
impl_from_tuples_for_inmemorydir!(
s1 P1 D1,
s2 P2 D2,
s3 P3 D3,
s4 P4 D4,
s5 P5 D5,
s6 P6 D6,
s7 P7 D7
);

#[cfg(test)]
mod test {
use snapbox::assert_data_eq;
Expand Down
117 changes: 0 additions & 117 deletions crates/cargo-test-support/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,99 +16,6 @@ pub enum Change<T> {
Keep(usize, usize, T),
}

pub fn diff<'a, T>(a: &'a [T], b: &'a [T]) -> Vec<Change<&'a T>>
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<T>(a: &[T], b: &[T]) -> Vec<Vec<usize>>
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<T>(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<T: fmt::Display>(changes: &[Change<T>]) -> String {
// anstyle is not very ergonomic, but I don't want to bring in another dependency.
let red = anstyle::AnsiColor::Red.on_default().render();
Expand Down Expand Up @@ -140,27 +47,3 @@ pub fn render_colored_changes<T: fmt::Display>(changes: &[Change<T>]) -> String
}
String::from_utf8(buffer.into_inner()).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");
}
Loading

0 comments on commit 2835ddc

Please sign in to comment.