Skip to content

Commit 2883e4f

Browse files
committed
Auto merge of #13980 - epage:compare, r=<try>
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
2 parents 721cd55 + 995746b commit 2883e4f

19 files changed

+392
-267
lines changed

Diff for: Cargo.lock

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ sha1 = "0.10.6"
9191
sha2 = "0.10.8"
9292
shell-escape = "0.1.5"
9393
supports-hyperlinks = "3.0.0"
94-
snapbox = { version = "0.6.5", features = ["diff", "dir", "term-svg", "regex"] }
94+
snapbox = { version = "0.6.7", features = ["diff", "dir", "term-svg", "regex"] }
9595
tar = { version = "0.4.40", default-features = false }
9696
tempfile = "3.10.1"
9797
thiserror = "1.0.59"

Diff for: crates/cargo-test-support/src/compare.rs

+142-71
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,14 @@ use crate::diff;
4040
use crate::paths;
4141
use anyhow::{bail, Context, Result};
4242
use serde_json::Value;
43-
use std::env;
4443
use std::fmt;
4544
use std::path::Path;
4645
use std::str;
4746
use url::Url;
4847

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

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

100+
/// Assertion policy for functional end-to-end tests
101+
///
102+
/// This emphasizes showing as much content as possible at the cost of more brittleness
103+
///
104+
/// # Snapshots
105+
///
106+
/// Updating of snapshots is controlled with the `SNAPSHOTS` environment variable:
107+
///
108+
/// - `skip`: do not run the tests
109+
/// - `ignore`: run the tests but ignore their failure
110+
/// - `verify`: run the tests
111+
/// - `overwrite`: update the snapshots based on the output of the tests
112+
///
113+
/// # Patterns
114+
///
115+
/// - `[..]` is a character wildcard, stopping at line breaks
116+
/// - `\n...\n` is a multi-line wildcard
117+
/// - `[EXE]` matches the exe suffix for the current platform
118+
/// - `[ROOT]` matches [`paths::root()`][crate::paths::root]
119+
/// - `[ROOTURL]` matches [`paths::root()`][crate::paths::root] as a URL
120+
///
121+
/// # Normalization
122+
///
123+
/// In addition to the patterns described above, text is normalized
124+
/// in such a way to avoid unwanted differences. The normalizations are:
125+
///
126+
/// - Backslashes are converted to forward slashes to deal with Windows paths.
127+
/// This helps so that all tests can be written assuming forward slashes.
128+
/// Other heuristics are applied to try to ensure Windows-style paths aren't
129+
/// a problem.
130+
/// - Carriage returns are removed, which can help when running on Windows.
131+
pub fn assert_e2e() -> snapbox::Assert {
132+
let root = paths::root();
133+
// Use `from_file_path` instead of `from_dir_path` so the trailing slash is
134+
// put in the users output, rather than hidden in the variable
135+
let root_url = url::Url::from_file_path(&root).unwrap().to_string();
136+
137+
let mut subs = snapbox::Redactions::new();
138+
subs.extend(MIN_LITERAL_REDACTIONS.into_iter().cloned())
139+
.unwrap();
140+
subs.extend(E2E_LITERAL_REDACTIONS.into_iter().cloned())
141+
.unwrap();
142+
subs.insert("[ROOT]", root).unwrap();
143+
subs.insert("[ROOTURL]", root_url).unwrap();
144+
subs.insert(
145+
"[ELAPSED]",
146+
regex::Regex::new("[FINISHED].*in (?<redacted>[0-9]+(\\.[0-9]+))s").unwrap(),
147+
)
148+
.unwrap();
149+
snapbox::Assert::new()
150+
.action_env(snapbox::assert::DEFAULT_ACTION_ENV)
151+
.redact_with(subs)
152+
}
153+
154+
static MIN_LITERAL_REDACTIONS: &[(&str, &str)] = &[
155+
("[EXE]", std::env::consts::EXE_SUFFIX),
156+
("[BROKEN_PIPE]", "Broken pipe (os error 32)"),
157+
("[BROKEN_PIPE]", "The pipe is being closed. (os error 232)"),
158+
];
159+
static E2E_LITERAL_REDACTIONS: &[(&str, &str)] = &[
160+
("[RUNNING]", " Running"),
161+
("[COMPILING]", " Compiling"),
162+
("[CHECKING]", " Checking"),
163+
("[COMPLETED]", " Completed"),
164+
("[CREATED]", " Created"),
165+
("[CREATING]", " Creating"),
166+
("[CREDENTIAL]", " Credential"),
167+
("[DOWNGRADING]", " Downgrading"),
168+
("[FINISHED]", " Finished"),
169+
("[ERROR]", "error:"),
170+
("[WARNING]", "warning:"),
171+
("[NOTE]", "note:"),
172+
("[HELP]", "help:"),
173+
("[DOCUMENTING]", " Documenting"),
174+
("[SCRAPING]", " Scraping"),
175+
("[FRESH]", " Fresh"),
176+
("[DIRTY]", " Dirty"),
177+
("[LOCKING]", " Locking"),
178+
("[UPDATING]", " Updating"),
179+
("[ADDING]", " Adding"),
180+
("[REMOVING]", " Removing"),
181+
("[REMOVED]", " Removed"),
182+
("[UNCHANGED]", " Unchanged"),
183+
("[DOCTEST]", " Doc-tests"),
184+
("[PACKAGING]", " Packaging"),
185+
("[PACKAGED]", " Packaged"),
186+
("[DOWNLOADING]", " Downloading"),
187+
("[DOWNLOADED]", " Downloaded"),
188+
("[UPLOADING]", " Uploading"),
189+
("[UPLOADED]", " Uploaded"),
190+
("[VERIFYING]", " Verifying"),
191+
("[ARCHIVING]", " Archiving"),
192+
("[INSTALLING]", " Installing"),
193+
("[REPLACING]", " Replacing"),
194+
("[UNPACKING]", " Unpacking"),
195+
("[SUMMARY]", " Summary"),
196+
("[FIXED]", " Fixed"),
197+
("[FIXING]", " Fixing"),
198+
("[IGNORED]", " Ignored"),
199+
("[INSTALLED]", " Installed"),
200+
("[REPLACED]", " Replaced"),
201+
("[BUILDING]", " Building"),
202+
("[LOGIN]", " Login"),
203+
("[LOGOUT]", " Logout"),
204+
("[YANK]", " Yank"),
205+
("[OWNER]", " Owner"),
206+
("[MIGRATING]", " Migrating"),
207+
("[EXECUTABLE]", " Executable"),
208+
("[SKIPPING]", " Skipping"),
209+
("[WAITING]", " Waiting"),
210+
("[PUBLISHED]", " Published"),
211+
("[BLOCKING]", " Blocking"),
212+
("[GENERATED]", " Generated"),
213+
];
214+
99215
/// Normalizes the output so that it can be compared against the expected value.
100216
fn normalize_actual(actual: &str, cwd: Option<&Path>) -> String {
101217
// It's easier to read tabs in outputs if they don't show up as literal
@@ -185,64 +301,11 @@ fn normalize_windows(text: &str, cwd: Option<&Path>) -> String {
185301
}
186302

187303
fn substitute_macros(input: &str) -> String {
188-
let macros = [
189-
("[RUNNING]", " Running"),
190-
("[COMPILING]", " Compiling"),
191-
("[CHECKING]", " Checking"),
192-
("[COMPLETED]", " Completed"),
193-
("[CREATED]", " Created"),
194-
("[CREATING]", " Creating"),
195-
("[CREDENTIAL]", " Credential"),
196-
("[DOWNGRADING]", " Downgrading"),
197-
("[FINISHED]", " Finished"),
198-
("[ERROR]", "error:"),
199-
("[WARNING]", "warning:"),
200-
("[NOTE]", "note:"),
201-
("[HELP]", "help:"),
202-
("[DOCUMENTING]", " Documenting"),
203-
("[SCRAPING]", " Scraping"),
204-
("[FRESH]", " Fresh"),
205-
("[DIRTY]", " Dirty"),
206-
("[LOCKING]", " Locking"),
207-
("[UPDATING]", " Updating"),
208-
("[ADDING]", " Adding"),
209-
("[REMOVING]", " Removing"),
210-
("[REMOVED]", " Removed"),
211-
("[UNCHANGED]", " Unchanged"),
212-
("[DOCTEST]", " Doc-tests"),
213-
("[PACKAGING]", " Packaging"),
214-
("[PACKAGED]", " Packaged"),
215-
("[DOWNLOADING]", " Downloading"),
216-
("[DOWNLOADED]", " Downloaded"),
217-
("[UPLOADING]", " Uploading"),
218-
("[UPLOADED]", " Uploaded"),
219-
("[VERIFYING]", " Verifying"),
220-
("[ARCHIVING]", " Archiving"),
221-
("[INSTALLING]", " Installing"),
222-
("[REPLACING]", " Replacing"),
223-
("[UNPACKING]", " Unpacking"),
224-
("[SUMMARY]", " Summary"),
225-
("[FIXED]", " Fixed"),
226-
("[FIXING]", " Fixing"),
227-
("[EXE]", env::consts::EXE_SUFFIX),
228-
("[IGNORED]", " Ignored"),
229-
("[INSTALLED]", " Installed"),
230-
("[REPLACED]", " Replaced"),
231-
("[BUILDING]", " Building"),
232-
("[LOGIN]", " Login"),
233-
("[LOGOUT]", " Logout"),
234-
("[YANK]", " Yank"),
235-
("[OWNER]", " Owner"),
236-
("[MIGRATING]", " Migrating"),
237-
("[EXECUTABLE]", " Executable"),
238-
("[SKIPPING]", " Skipping"),
239-
("[WAITING]", " Waiting"),
240-
("[PUBLISHED]", " Published"),
241-
("[BLOCKING]", " Blocking"),
242-
("[GENERATED]", " Generated"),
243-
];
244304
let mut result = input.to_owned();
245-
for &(pat, subst) in &macros {
305+
for &(pat, subst) in MIN_LITERAL_REDACTIONS {
306+
result = result.replace(pat, subst)
307+
}
308+
for &(pat, subst) in E2E_LITERAL_REDACTIONS {
246309
result = result.replace(pat, subst)
247310
}
248311
result
@@ -254,7 +317,7 @@ fn substitute_macros(input: &str) -> String {
254317
///
255318
/// - `description` explains where the output is from (usually "stdout" or "stderr").
256319
/// - `other_output` is other output to display in the error (usually stdout or stderr).
257-
pub fn match_exact(
320+
pub(crate) fn match_exact(
258321
expected: &str,
259322
actual: &str,
260323
description: &str,
@@ -282,7 +345,7 @@ pub fn match_exact(
282345

283346
/// Convenience wrapper around [`match_exact`] which will panic on error.
284347
#[track_caller]
285-
pub fn assert_match_exact(expected: &str, actual: &str) {
348+
pub(crate) fn assert_match_exact(expected: &str, actual: &str) {
286349
if let Err(e) = match_exact(expected, actual, "", "", None) {
287350
crate::panic_error("", e);
288351
}
@@ -292,7 +355,7 @@ pub fn assert_match_exact(expected: &str, actual: &str) {
292355
/// of the lines.
293356
///
294357
/// See [Patterns](index.html#patterns) for more information on pattern matching.
295-
pub fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
358+
pub(crate) fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
296359
let expected = normalize_expected(expected, cwd);
297360
let actual = normalize_actual(actual, cwd);
298361
let e: Vec<_> = expected.lines().map(|line| WildStr::new(line)).collect();
@@ -342,7 +405,7 @@ pub fn match_unordered(expected: &str, actual: &str, cwd: Option<&Path>) -> Resu
342405
/// somewhere.
343406
///
344407
/// See [Patterns](index.html#patterns) for more information on pattern matching.
345-
pub fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
408+
pub(crate) fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
346409
let expected = normalize_expected(expected, cwd);
347410
let actual = normalize_actual(actual, cwd);
348411
let e: Vec<_> = expected.lines().map(|line| WildStr::new(line)).collect();
@@ -369,7 +432,11 @@ pub fn match_contains(expected: &str, actual: &str, cwd: Option<&Path>) -> Resul
369432
/// anywhere.
370433
///
371434
/// See [Patterns](index.html#patterns) for more information on pattern matching.
372-
pub fn match_does_not_contain(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
435+
pub(crate) fn match_does_not_contain(
436+
expected: &str,
437+
actual: &str,
438+
cwd: Option<&Path>,
439+
) -> Result<()> {
373440
if match_contains(expected, actual, cwd).is_ok() {
374441
bail!(
375442
"expected not to find:\n\
@@ -388,7 +455,7 @@ pub fn match_does_not_contain(expected: &str, actual: &str, cwd: Option<&Path>)
388455
/// somewhere, and should be repeated `number` times.
389456
///
390457
/// See [Patterns](index.html#patterns) for more information on pattern matching.
391-
pub fn match_contains_n(
458+
pub(crate) fn match_contains_n(
392459
expected: &str,
393460
number: usize,
394461
actual: &str,
@@ -425,7 +492,7 @@ pub fn match_contains_n(
425492
///
426493
/// See [`crate::Execs::with_stderr_line_without`] for an example and cautions
427494
/// against using.
428-
pub fn match_with_without(
495+
pub(crate) fn match_with_without(
429496
actual: &str,
430497
with: &[String],
431498
without: &[String],
@@ -473,7 +540,7 @@ pub fn match_with_without(
473540
/// expected JSON objects.
474541
///
475542
/// See [`crate::Execs::with_json`] for more details.
476-
pub fn match_json(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
543+
pub(crate) fn match_json(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()> {
477544
let (exp_objs, act_objs) = collect_json_objects(expected, actual)?;
478545
if exp_objs.len() != act_objs.len() {
479546
bail!(
@@ -494,7 +561,7 @@ pub fn match_json(expected: &str, actual: &str, cwd: Option<&Path>) -> Result<()
494561
///
495562
/// See [`crate::Execs::with_json_contains_unordered`] for more details and
496563
/// cautions when using.
497-
pub fn match_json_contains_unordered(
564+
pub(crate) fn match_json_contains_unordered(
498565
expected: &str,
499566
actual: &str,
500567
cwd: Option<&Path>,
@@ -552,7 +619,11 @@ fn collect_json_objects(
552619
/// as paths). You can use a `"{...}"` string literal as a wildcard for
553620
/// arbitrary nested JSON (useful for parts of object emitted by other programs
554621
/// (e.g., rustc) rather than Cargo itself).
555-
pub fn find_json_mismatch(expected: &Value, actual: &Value, cwd: Option<&Path>) -> Result<()> {
622+
pub(crate) fn find_json_mismatch(
623+
expected: &Value,
624+
actual: &Value,
625+
cwd: Option<&Path>,
626+
) -> Result<()> {
556627
match find_json_mismatch_r(expected, actual, cwd) {
557628
Some((expected_part, actual_part)) => bail!(
558629
"JSON mismatch\nExpected:\n{}\nWas:\n{}\nExpected part:\n{}\nActual part:\n{}\n",
@@ -619,7 +690,7 @@ fn find_json_mismatch_r<'a>(
619690
}
620691

621692
/// A single line string that supports `[..]` wildcard matching.
622-
pub struct WildStr<'a> {
693+
pub(crate) struct WildStr<'a> {
623694
has_meta: bool,
624695
line: &'a str,
625696
}

Diff for: crates/cargo-test-support/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ pub mod prelude {
7676
pub use crate::CargoCommand;
7777
pub use crate::ChannelChanger;
7878
pub use crate::TestEnv;
79+
pub use snapbox::IntoData;
7980
}
8081

8182
/*

Diff for: tests/testsuite/alt_registry.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
//! Tests for alternative registries.
22
3-
use cargo_test_support::compare::assert_match_exact;
3+
use cargo_test_support::compare::assert_e2e;
44
use cargo_test_support::publish::validate_alt_upload;
55
use cargo_test_support::registry::{self, Package, RegistryBuilder};
6+
use cargo_test_support::str;
67
use cargo_test_support::{basic_manifest, paths, project};
78
use std::fs;
89

@@ -1476,9 +1477,10 @@ fn sparse_lockfile() {
14761477
.build();
14771478

14781479
p.cargo("generate-lockfile").run();
1479-
assert_match_exact(
1480+
assert_e2e().eq(
14801481
&p.read_lockfile(),
1481-
r#"# This file is automatically @generated by Cargo.
1482+
str![[r##"
1483+
# This file is automatically @generated by Cargo.
14821484
# It is not intended for manual editing.
14831485
version = 3
14841486
@@ -1492,8 +1494,10 @@ dependencies = [
14921494
[[package]]
14931495
name = "foo"
14941496
version = "0.1.0"
1495-
source = "sparse+http://[..]/"
1496-
checksum = "458c1addb23fde7dfbca0410afdbcc0086f96197281ec304d9e0e10def3cb899""#,
1497+
source = "sparse+http://127.0.0.1:[..]/index/"
1498+
checksum = "458c1addb23fde7dfbca0410afdbcc0086f96197281ec304d9e0e10def3cb899"
1499+
1500+
"##]],
14971501
);
14981502
}
14991503

0 commit comments

Comments
 (0)