Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

docs(test): Document Execs assertions based on port effort #14793

Merged
merged 5 commits into from
Nov 8, 2024
Merged

docs(test): Document Execs assertions based on port effort #14793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2dd062f
refactor(test): Move normalize functions after use
epage Nov 7, 2024
1b76a90
refactor(test): Limit the scope of WildStr
epage Nov 7, 2024
84ac190
refactor(test): Turn WildStr panic into compiler error
epage Nov 7, 2024
d20216c
docs(test): Document Execs assertions based on port effort
epage Nov 7, 2024
af3cfd5
fix(test): Un-deprecate contains assertions
epage Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 33 additions & 35 deletions crates/cargo-test-support/src/compare.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,27 +331,6 @@ static E2E_LITERAL_REDACTIONS: &[(&str, &str)] = &[
("[OPENING]", " Opening"),
];

/// Normalizes the output so that it can be compared against the expected value.
fn normalize_actual(content: &str, redactions: &snapbox::Redactions) -> String {
use snapbox::filter::Filter as _;
let content = snapbox::filter::FilterPaths.filter(content.into_data());
let content = snapbox::filter::FilterNewlines.filter(content);
let content = content.render().expect("came in as a String");
let content = redactions.redact(&content);
content
}

/// Normalizes the expected string so that it can be compared against the actual output.
fn normalize_expected(content: &str, redactions: &snapbox::Redactions) -> String {
use snapbox::filter::Filter as _;
let content = snapbox::filter::FilterPaths.filter(content.into_data());
let content = snapbox::filter::FilterNewlines.filter(content);
// Remove any conditionally absent redactions like `[EXE]`
let content = content.render().expect("came in as a String");
let content = redactions.clear_unused(&content);
content.into_owned()
}

/// Checks that the given string contains the given contiguous lines
/// somewhere.
///
Expand All @@ -364,12 +343,12 @@ pub(crate) fn match_contains(
let expected = normalize_expected(expected, redactions);
let actual = normalize_actual(actual, redactions);
let e: Vec<_> = expected.lines().map(|line| WildStr::new(line)).collect();
let a: Vec<_> = actual.lines().map(|line| WildStr::new(line)).collect();
let a: Vec<_> = actual.lines().collect();
if e.len() == 0 {
bail!("expected length must not be zero");
}
for window in a.windows(e.len()) {
if window == e {
if e == window {
return Ok(());
}
}
Expand Down Expand Up @@ -428,7 +407,6 @@ pub(crate) fn match_with_without(

let matches: Vec<_> = actual
.lines()
.map(WildStr::new)
.filter(|line| with_wild.iter().all(|with| with == line))
.filter(|line| !without_wild.iter().any(|without| without == line))
.collect();
Expand Down Expand Up @@ -457,28 +435,48 @@ pub(crate) fn match_with_without(
}
}

/// Normalizes the output so that it can be compared against the expected value.
fn normalize_actual(content: &str, redactions: &snapbox::Redactions) -> String {
use snapbox::filter::Filter as _;
let content = snapbox::filter::FilterPaths.filter(content.into_data());
let content = snapbox::filter::FilterNewlines.filter(content);
let content = content.render().expect("came in as a String");
let content = redactions.redact(&content);
content
}

/// Normalizes the expected string so that it can be compared against the actual output.
fn normalize_expected(content: &str, redactions: &snapbox::Redactions) -> String {
use snapbox::filter::Filter as _;
let content = snapbox::filter::FilterPaths.filter(content.into_data());
let content = snapbox::filter::FilterNewlines.filter(content);
// Remove any conditionally absent redactions like `[EXE]`
let content = content.render().expect("came in as a String");
let content = redactions.clear_unused(&content);
content.into_owned()
}

/// A single line string that supports `[..]` wildcard matching.
pub(crate) struct WildStr<'a> {
struct WildStr<'a> {
has_meta: bool,
line: &'a str,
}

impl<'a> WildStr<'a> {
pub fn new(line: &'a str) -> WildStr<'a> {
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 [..]"),
impl PartialEq<&str> for WildStr<'_> {
fn eq(&self, other: &&str) -> bool {
if self.has_meta {
meta_cmp(self.line, other)
} else {
self.line == *other
}
}
}
Expand Down Expand Up @@ -666,10 +664,10 @@ mod test {
("[..]", "a b"),
("[..]b", "a b"),
] {
assert_eq!(WildStr::new(a), WildStr::new(b));
assert_eq!(WildStr::new(a), b);
}
for (a, b) in &[("[..]b", "c"), ("b", "c"), ("b", "cb")] {
assert_ne!(WildStr::new(a), WildStr::new(b));
assert_ne!(WildStr::new(a), b);
}
}

Expand Down
167 changes: 154 additions & 13 deletions crates/cargo-test-support/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,57 @@ impl Execs {
/// Verifies that stdout is equal to the given lines.
///
/// See [`compare::assert_e2e`] for assertion details.
///
/// <div class="warning">
///
/// Prefer passing in [`str!`] for `expected` to get snapshot updating.
///
/// If `format!` is needed for content that changes from run to run that you don't care about,
/// consider whether you could have [`compare::assert_e2e`] redact the content.
/// If nothing else, a wildcard (`[..]`, `...`) may be useful.
///
/// However, `""` may be preferred for intentionally empty output so people don't accidentally
/// bless a change.
///
/// </div>
///
/// # Examples
///
/// ```no_run
/// use cargo_test_support::prelude::*;
/// use cargo_test_support::str;
/// use cargo_test_support::execs;
///
/// execs().with_stdout_data(str![r#"
/// Hello world!
/// "#]);
/// ```
///
/// Non-deterministic compiler output
/// ```no_run
/// use cargo_test_support::prelude::*;
/// use cargo_test_support::str;
/// use cargo_test_support::execs;
///
/// execs().with_stdout_data(str![r#"
/// [COMPILING] foo
/// [COMPILING] bar
/// "#].unordered());
/// ```
///
/// jsonlines
/// ```no_run
/// use cargo_test_support::prelude::*;
/// use cargo_test_support::str;
/// use cargo_test_support::execs;
///
/// execs().with_stdout_data(str![r#"
/// [
/// {},
/// {}
/// ]
/// "#].is_json().against_jsonlines());
/// ```
pub fn with_stdout_data(&mut self, expected: impl snapbox::IntoData) -> &mut Self {
self.expect_stdout_data = Some(expected.into_data());
self
Expand All @@ -675,6 +726,57 @@ impl Execs {
/// Verifies that stderr is equal to the given lines.
///
/// See [`compare::assert_e2e`] for assertion details.
///
/// <div class="warning">
///
/// Prefer passing in [`str!`] for `expected` to get snapshot updating.
///
/// If `format!` is needed for content that changes from run to run that you don't care about,
/// consider whether you could have [`compare::assert_e2e`] redact the content.
/// If nothing else, a wildcard (`[..]`, `...`) may be useful.
///
/// However, `""` may be preferred for intentionally empty output so people don't accidentally
/// bless a change.
///
/// </div>
///
/// # Examples
///
/// ```no_run
/// use cargo_test_support::prelude::*;
/// use cargo_test_support::str;
/// use cargo_test_support::execs;
///
/// execs().with_stderr_data(str![r#"
/// Hello world!
/// "#]);
/// ```
///
/// Non-deterministic compiler output
/// ```no_run
/// use cargo_test_support::prelude::*;
/// use cargo_test_support::str;
/// use cargo_test_support::execs;
///
/// execs().with_stderr_data(str![r#"
/// [COMPILING] foo
/// [COMPILING] bar
/// "#].unordered());
/// ```
///
/// jsonlines
/// ```no_run
/// use cargo_test_support::prelude::*;
/// use cargo_test_support::str;
/// use cargo_test_support::execs;
///
/// execs().with_stderr_data(str![r#"
/// [
/// {},
/// {}
/// ]
/// "#].is_json().against_jsonlines());
/// ```
pub fn with_stderr_data(&mut self, expected: impl snapbox::IntoData) -> &mut Self {
self.expect_stderr_data = Some(expected.into_data());
self
Expand Down Expand Up @@ -706,7 +808,14 @@ impl Execs {
/// its output.
///
/// See [`compare`] for supported patterns.
#[deprecated(note = "replaced with `Execs::with_stdout_data(expected)`")]
///
/// <div class="warning">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! I didn't know rustdoc has special CSS classes until today.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they actually have something newer but its not documented. I'll ping them about it.

///
/// Prefer [`Execs::with_stdout_data`] where possible.
/// - `expected` cannot be snapshotted
/// - `expected` can end up being ambiguous, causing the assertion to succeed when it should fail
///
/// </div>
pub fn with_stdout_contains<S: ToString>(&mut self, expected: S) -> &mut Self {
self.expect_stdout_contains.push(expected.to_string());
self
Expand All @@ -716,7 +825,14 @@ impl Execs {
/// its output.
///
/// See [`compare`] for supported patterns.
#[deprecated(note = "replaced with `Execs::with_stderr_data(expected)`")]
///
/// <div class="warning">
///
/// Prefer [`Execs::with_stderr_data`] where possible.
/// - `expected` cannot be snapshotted
/// - `expected` can end up being ambiguous, causing the assertion to succeed when it should fail
///
/// </div>
pub fn with_stderr_contains<S: ToString>(&mut self, expected: S) -> &mut Self {
self.expect_stderr_contains.push(expected.to_string());
self
Expand All @@ -727,7 +843,18 @@ impl Execs {
/// See [`compare`] for supported patterns.
///
/// See note on [`Self::with_stderr_does_not_contain`].
#[deprecated]
///
/// <div class="warning">
///
/// Prefer [`Execs::with_stdout_data`] where possible.
/// - `expected` cannot be snapshotted
/// - The absence of `expected` can either mean success or that the string being looked for
/// changed.
///
/// To mitigate this, consider matching this up with
/// [`Execs::with_stdout_contains`].
///
/// </div>
pub fn with_stdout_does_not_contain<S: ToString>(&mut self, expected: S) -> &mut Self {
self.expect_stdout_not_contains.push(expected.to_string());
self
Expand All @@ -737,12 +864,18 @@ impl Execs {
///
/// 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
/// your test will pass without verifying the correct behavior. If
/// possible, write the test first so that it fails, and then implement
/// your fix/feature to make it pass.
#[deprecated]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to block this, but still prefer them being deprecated, as people have less change to misuse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have other ideas, I'm open

/// <div class="warning">
///
/// Prefer [`Execs::with_stdout_data`] where possible.
/// - `expected` cannot be snapshotted
/// - The absence of `expected` can either mean success or that the string being looked for
/// changed.
///
/// To mitigate this, consider either matching this up with
/// [`Execs::with_stdout_contains`] or replace it
/// with [`Execs::with_stderr_line_without`].
///
/// </div>
pub fn with_stderr_does_not_contain<S: ToString>(&mut self, expected: S) -> &mut Self {
self.expect_stderr_not_contains.push(expected.to_string());
self
Expand All @@ -751,7 +884,18 @@ impl Execs {
/// Verify that a particular line appears in stderr with and without the
/// given substrings. Exactly one line must match.
///
/// The substrings are matched as `contains`. Example:
/// The substrings are matched as `contains`.
///
/// <div class="warning">
///
/// Prefer [`Execs::with_stdout_data`] where possible.
/// - `with` cannot be snapshotted
/// - The absence of `without`` can either mean success or that the string being looked for
/// changed.
///
/// </div>
///
/// # Example
///
/// ```no_run
/// use cargo_test_support::execs;
Expand All @@ -768,9 +912,6 @@ impl Execs {
/// This will check that a build line includes `-C opt-level=3` but does
/// not contain `-C debuginfo` or `-C incremental`.
///
/// Be careful writing the `without` fragments, see note in
/// `with_stderr_does_not_contain`.
#[deprecated]
pub fn with_stderr_line_without<S: ToString>(
&mut self,
with: &[S],
Expand Down
5 changes: 0 additions & 5 deletions tests/testsuite/artifact_dep.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,6 @@ fn build_script_with_bin_artifact_and_lib_false() {
)
.build();

#[expect(deprecated)]
p.cargo("build -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_status(101)
Expand Down Expand Up @@ -731,7 +730,6 @@ fn lib_with_bin_artifact_and_lib_false() {
)
.build();

#[expect(deprecated)]
p.cargo("build -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_status(101)
Expand Down Expand Up @@ -1117,7 +1115,6 @@ fn build_script_deps_adopt_specified_target_unconditionally() {
.file("bar/src/lib.rs", "pub fn doit() {}")
.build();

#[expect(deprecated)]
p.cargo("check -v -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_does_not_contain(
Expand Down Expand Up @@ -1233,7 +1230,6 @@ fn non_build_script_deps_adopt_specified_target_unconditionally() {
.file("bar/src/lib.rs", "pub fn doit() {}")
.build();

#[expect(deprecated)]
p.cargo("check -v -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_contains(
Expand Down Expand Up @@ -1378,7 +1374,6 @@ fn build_script_deps_adopts_target_platform_if_target_equals_target() {
.build();

let alternate_target = cross_compile::alternate();
#[expect(deprecated)]
p.cargo("check -v -Z bindeps --target")
.arg(alternate_target)
.masquerade_as_nightly_cargo(&["bindeps"])
Expand Down
Loading