Skip to content

Commit

Permalink
Auto merge of #13980 - epage:compare, r=<try>
Browse files Browse the repository at this point in the history 
refactor: Transition direct assertions from cargo-test-support to snapbox

### What does this PR try to resolve?

Cargo has a bespoke testing framework for functional tests
- Extra stuff for us to maintain
- Don't leverage benefits from contributions related to other projects
- Less incentive to be thoroughly documented

UI tests are written using snapbox.  The latest release of snapbox (#13963) was geared at supporting cargo's needs in the hope that we can consolidate on testing frameworks.

Besides having a single set of semantics, benefits we'd gain include
- Updating of test snapshots
- Fancier redacting of test output (e.g. #13973)

This is the first incremental step in this direction.  This replaces direct assertions with snapbox assertions.  This still leaves all of the CLI output assertions. These will be done incrementally.

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

### Additional information
  • Loading branch information
@bors
bors committed Jun 2, 2024
2 parents 721cd55 + 995746b commit 2883e4f
Show file tree
Hide file tree
Showing 19 changed files with 392 additions and 267 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock
View file

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

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ sha1 = "0.10.6"
sha2 = "0.10.8"
shell-escape = "0.1.5"
supports-hyperlinks = "3.0.0"
snapbox = { version = "0.6.5", features = ["diff", "dir", "term-svg", "regex"] }
snapbox = { version = "0.6.7", features = ["diff", "dir", "term-svg", "regex"] }
tar = { version = "0.4.40", default-features = false }
tempfile = "3.10.1"
thiserror = "1.0.59"
Expand Down
213 changes: 142 additions & 71 deletions crates/cargo-test-support/src/compare.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,14 @@ 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;

/// Default `snapbox` Assertions
/// Assertion policy for UI tests
///
/// This emphasizes showing as much content as possible at the cost of more brittleness
///
/// # Snapshots
///
Expand Down Expand Up @@ -82,7 +83,7 @@ pub fn assert_ui() -> snapbox::Assert {
let root_url = url::Url::from_file_path(&root).unwrap().to_string();

let mut subs = snapbox::Redactions::new();
subs.extend([("[EXE]", std::env::consts::EXE_SUFFIX)])
subs.extend(MIN_LITERAL_REDACTIONS.into_iter().cloned())
.unwrap();
subs.insert("[ROOT]", root).unwrap();
subs.insert("[ROOTURL]", root_url).unwrap();
Expand All @@ -96,6 +97,121 @@ pub fn assert_ui() -> snapbox::Assert {
.redact_with(subs)
}

/// Assertion policy for functional end-to-end tests
///
/// This emphasizes showing as much content as possible at the cost of more brittleness
///
/// # Snapshots
///
/// Updating of snapshots is controlled with the `SNAPSHOTS` environment variable:
///
/// - `skip`: do not run the tests
/// - `ignore`: run the tests but ignore their failure
/// - `verify`: run the tests
/// - `overwrite`: update the snapshots based on the output of the tests
///
/// # Patterns
///
/// - `[..]` is a character wildcard, stopping at line breaks
/// - `\n...\n` is a multi-line wildcard
/// - `[EXE]` matches the exe suffix for the current platform
/// - `[ROOT]` matches [`paths::root()`][crate::paths::root]
/// - `[ROOTURL]` matches [`paths::root()`][crate::paths::root] as a URL
///
/// # Normalization
///
/// In addition to the patterns described above, text is normalized
/// in such a way to avoid unwanted differences. The normalizations are:
///
/// - 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.
pub fn assert_e2e() -> snapbox::Assert {
let root = paths::root();
// Use `from_file_path` instead of `from_dir_path` so the trailing slash is
// put in the users output, rather than hidden in the variable
let root_url = url::Url::from_file_path(&root).unwrap().to_string();

let mut subs = snapbox::Redactions::new();
subs.extend(MIN_LITERAL_REDACTIONS.into_iter().cloned())
.unwrap();
subs.extend(E2E_LITERAL_REDACTIONS.into_iter().cloned())
.unwrap();
subs.insert("[ROOT]", root).unwrap();
subs.insert("[ROOTURL]", root_url).unwrap();
subs.insert(
"[ELAPSED]",
regex::Regex::new("[FINISHED].*in (?<redacted>[0-9]+(\\.[0-9]+))s").unwrap(),
)
.unwrap();
snapbox::Assert::new()
.action_env(snapbox::assert::DEFAULT_ACTION_ENV)
.redact_with(subs)
}

static MIN_LITERAL_REDACTIONS: &[(&str, &str)] = &[
("[EXE]", std::env::consts::EXE_SUFFIX),
("[BROKEN_PIPE]", "Broken pipe (os error 32)"),
("[BROKEN_PIPE]", "The pipe is being closed. (os error 232)"),
];
static E2E_LITERAL_REDACTIONS: &[(&str, &str)] = &[
("[RUNNING]", " Running"),
("[COMPILING]", " Compiling"),
("[CHECKING]", " Checking"),
("[COMPLETED]", " Completed"),
("[CREATED]", " Created"),
("[CREATING]", " Creating"),
("[CREDENTIAL]", " Credential"),
("[DOWNGRADING]", " Downgrading"),
("[FINISHED]", " Finished"),
("[ERROR]", "error:"),
("[WARNING]", "warning:"),
("[NOTE]", "note:"),
("[HELP]", "help:"),
("[DOCUMENTING]", " Documenting"),
("[SCRAPING]", " Scraping"),
("[FRESH]", " Fresh"),
("[DIRTY]", " Dirty"),
("[LOCKING]", " Locking"),
("[UPDATING]", " Updating"),
("[ADDING]", " Adding"),
("[REMOVING]", " Removing"),
("[REMOVED]", " Removed"),
("[UNCHANGED]", " Unchanged"),
("[DOCTEST]", " Doc-tests"),
("[PACKAGING]", " Packaging"),
("[PACKAGED]", " Packaged"),
("[DOWNLOADING]", " Downloading"),
("[DOWNLOADED]", " Downloaded"),
("[UPLOADING]", " Uploading"),
("[UPLOADED]", " Uploaded"),
("[VERIFYING]", " Verifying"),
("[ARCHIVING]", " Archiving"),
("[INSTALLING]", " Installing"),
("[REPLACING]", " Replacing"),
("[UNPACKING]", " Unpacking"),
("[SUMMARY]", " Summary"),
("[FIXED]", " Fixed"),
("[FIXING]", " Fixing"),
("[IGNORED]", " Ignored"),
("[INSTALLED]", " Installed"),
("[REPLACED]", " Replaced"),
("[BUILDING]", " Building"),
("[LOGIN]", " Login"),
("[LOGOUT]", " Logout"),
("[YANK]", " Yank"),
("[OWNER]", " Owner"),
("[MIGRATING]", " Migrating"),
("[EXECUTABLE]", " Executable"),
("[SKIPPING]", " Skipping"),
("[WAITING]", " Waiting"),
("[PUBLISHED]", " Published"),
("[BLOCKING]", " Blocking"),
("[GENERATED]", " Generated"),
];

/// 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
Expand Down Expand Up @@ -185,64 +301,11 @@ fn normalize_windows(text: &str, cwd: Option<&Path>) -> String {
}

fn substitute_macros(input: &str) -> String {
let macros = [
("[RUNNING]", " Running"),
("[COMPILING]", " Compiling"),
("[CHECKING]", " Checking"),
("[COMPLETED]", " Completed"),
("[CREATED]", " Created"),
("[CREATING]", " Creating"),
("[CREDENTIAL]", " Credential"),
("[DOWNGRADING]", " Downgrading"),
("[FINISHED]", " Finished"),
("[ERROR]", "error:"),
("[WARNING]", "warning:"),
("[NOTE]", "note:"),
("[HELP]", "help:"),
("[DOCUMENTING]", " Documenting"),
("[SCRAPING]", " Scraping"),
("[FRESH]", " Fresh"),
("[DIRTY]", " Dirty"),
("[LOCKING]", " Locking"),
("[UPDATING]", " Updating"),
("[ADDING]", " Adding"),
("[REMOVING]", " Removing"),
("[REMOVED]", " Removed"),
("[UNCHANGED]", " Unchanged"),
("[DOCTEST]", " Doc-tests"),
("[PACKAGING]", " Packaging"),
("[PACKAGED]", " Packaged"),
("[DOWNLOADING]", " Downloading"),
("[DOWNLOADED]", " Downloaded"),
("[UPLOADING]", " Uploading"),
("[UPLOADED]", " Uploaded"),
("[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"),
("[EXECUTABLE]", " Executable"),
("[SKIPPING]", " Skipping"),
("[WAITING]", " Waiting"),
("[PUBLISHED]", " Published"),
("[BLOCKING]", " Blocking"),
("[GENERATED]", " Generated"),
];
let mut result = input.to_owned();
for &(pat, subst) in &macros {
for &(pat, subst) in MIN_LITERAL_REDACTIONS {
result = result.replace(pat, subst)
}
for &(pat, subst) in E2E_LITERAL_REDACTIONS {
result = result.replace(pat, subst)
}
result
Expand All @@ -254,7 +317,7 @@ fn substitute_macros(input: &str) -> String {
///
/// - `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(
pub(crate) fn match_exact(
expected: &str,
actual: &str,
description: &str,
Expand Down Expand Up @@ -282,7 +345,7 @@ 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) {
pub(crate) fn assert_match_exact(expected: &str, actual: &str) {
if let Err(e) = match_exact(expected, actual, "", "", None) {
crate::panic_error("", e);
}
Expand All @@ -292,7 +355,7 @@ pub fn assert_match_exact(expected: &str, actual: &str) {
/// 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<()> {
pub(crate) fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
let expected = normalize_expected(expected, cwd);
let actual = normalize_actual(actual, cwd);
let e: Vec<_> = expected.lines().map(|line| WildStr::new(line)).collect();
Expand Down Expand Up @@ -342,7 +405,7 @@ pub fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Resu
/// somewhere.
///
/// See [Patterns](index.html#patterns) for more information on pattern matching.
pub fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
pub(crate) fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
let expected = normalize_expected(expected, cwd);
let actual = normalize_actual(actual, cwd);
let e: Vec<_> = expected.lines().map(|line| WildStr::new(line)).collect();
Expand All @@ -369,7 +432,11 @@ pub fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Resul
/// 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<()> {
pub(crate) 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\
Expand All @@ -388,7 +455,7 @@ pub fn match_does_not_contain(expected: &str, actual: &str, cwd: Option<&Path>)
/// somewhere, and should be repeated `number` times.
///
/// See [Patterns](index.html#patterns) for more information on pattern matching.
pub fn match_contains_n(
pub(crate) fn match_contains_n(
expected: &str,
number: usize,
actual: &str,
Expand Down Expand Up @@ -425,7 +492,7 @@ pub fn match_contains_n(
///
/// See [`crate::Execs::with_stderr_line_without`] for an example and cautions
/// against using.
pub fn match_with_without(
pub(crate) fn match_with_without(
actual: &str,
with: &[String],
without: &[String],
Expand Down Expand Up @@ -473,7 +540,7 @@ pub fn match_with_without(
/// expected JSON objects.
///
/// See [`crate::Execs::with_json`] for more details.
pub fn match_json(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
pub(crate) 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!(
Expand All @@ -494,7 +561,7 @@ pub fn match_json(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()
///
/// See [`crate::Execs::with_json_contains_unordered`] for more details and
/// cautions when using.
pub fn match_json_contains_unordered(
pub(crate) fn match_json_contains_unordered(
expected: &str,
actual: &str,
cwd: Option<&Path>,
Expand Down Expand Up @@ -552,7 +619,11 @@ fn collect_json_objects(
/// 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<()> {
pub(crate) 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",
Expand Down Expand Up @@ -619,7 +690,7 @@ fn find_json_mismatch_r<'a>(
}

/// A single line string that supports `[..]` wildcard matching.
pub struct WildStr<'a> {
pub(crate) struct WildStr<'a> {
has_meta: bool,
line: &'a str,
}
Expand Down
1 change: 1 addition & 0 deletions crates/cargo-test-support/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ pub mod prelude {
pub use crate::CargoCommand;
pub use crate::ChannelChanger;
pub use crate::TestEnv;
pub use snapbox::IntoData;
}

/*
Expand Down
14 changes: 9 additions & 5 deletions tests/testsuite/alt_registry.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! Tests for alternative registries.
use cargo_test_support::compare::assert_match_exact;
use cargo_test_support::compare::assert_e2e;
use cargo_test_support::publish::validate_alt_upload;
use cargo_test_support::registry::{self, Package, RegistryBuilder};
use cargo_test_support::str;
use cargo_test_support::{basic_manifest, paths, project};
use std::fs;

Expand Down Expand Up @@ -1476,9 +1477,10 @@ fn sparse_lockfile() {
.build();

p.cargo("generate-lockfile").run();
assert_match_exact(
assert_e2e().eq(
&p.read_lockfile(),
r#"# This file is automatically @generated by Cargo.
str![[r##"
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3
Expand All @@ -1492,8 +1494,10 @@ dependencies = [
[[package]]
name = "foo"
version = "0.1.0"
source = "sparse+http://[..]/"
checksum = "458c1addb23fde7dfbca0410afdbcc0086f96197281ec304d9e0e10def3cb899""#,
source = "sparse+http://127.0.0.1:[..]/index/"
checksum = "458c1addb23fde7dfbca0410afdbcc0086f96197281ec304d9e0e10def3cb899"
"##]],
);
}

Expand Down
Loading

0 comments on commit 2883e4f

Please sign in to comment.