Auto merge of #14576 - epage:tests, r=weihanglo

test: Remove the last of our custom json assertions

### What does this PR try to resolve?

This is part of #14039 and consolidates us down to only one way of doing json assertions, using snapbox.

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

### Additional information

Author: bors

Cargo.lock

@@ -93,7 +93,7 @@ sha2 = "0.10.8"
 shell-escape = "0.1.5"
 similar = "2.6.0"
 supports-hyperlinks = "3.0.0"
-snapbox = { version = "0.6.17", features = ["diff", "dir", "term-svg", "regex", "json"] }
+snapbox = { version = "0.6.18", features = ["diff", "dir", "term-svg", "regex", "json"] }
 tar = { version = "0.4.42", default-features = false }
 tempfile = "3.10.1"
 thiserror = "1.0.63"

Cargo.toml

@@ -44,8 +44,7 @@
 use crate::cross_compile::try_alternate;
 use crate::paths;
 use crate::{diff, rustc_host};
-use anyhow::{bail, Context, Result};
-use serde_json::Value;
+use anyhow::{bail, Result};
 use std::fmt;
 use std::path::Path;
 use std::str;
@@ -654,159 +653,6 @@ pub(crate) fn match_with_without(
     }
 }
 
-/// 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(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!(
-            "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(crate) 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<serde_json::Value>, Vec<serde_json::Value>)> {
-    let expected_objs: Vec<_> = expected
-        .split("\n\n")
-        .map(|expect| {
-            expect
-                .parse()
-                .with_context(|| format!("failed to parse expected JSON object:\n{}", expect))
-        })
-        .collect::<Result<_>>()?;
-    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::<Result<_>>()?;
-    Ok((expected_objs, actual_objs))
-}
-
-/// 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(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",
-            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)) => {
-            if match_exact(l, r, "", "", cwd).is_err() {
-                Some((expected, actual))
-            } else {
-                None
-            }
-        }
-        (&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 mut expected_entries = l.iter();
-            let mut actual_entries = r.iter();
-
-            loop {
-                match (expected_entries.next(), actual_entries.next()) {
-                    (None, None) => return None,
-                    (Some((expected_key, expected_value)), Some((actual_key, actual_value)))
-                        if expected_key == actual_key =>
-                    {
-                        if let mismatch @ Some(_) =
-                            find_json_mismatch_r(expected_value, actual_value, cwd)
-                        {
-                            return mismatch;
-                        }
-                    }
-                    _ => return Some((expected, actual)),
-                }
-            }
-        }
-        (&Null, &Null) => None,
-        // Magic string literal `"{...}"` acts as wildcard for any sub-JSON.
-        _ => Some((expected, actual)),
-    }
-}
-
 /// A single line string that supports `[..]` wildcard matching.
 pub(crate) struct WildStr<'a> {
     has_meta: bool,

crates/cargo-test-support/src/compare.rs

@@ -655,8 +655,6 @@ pub struct Execs {
     expect_stdout_unordered: Vec<String>,
     expect_stderr_unordered: Vec<String>,
     expect_stderr_with_without: Vec<(Vec<String>, Vec<String>)>,
-    expect_json: Option<String>,
-    expect_json_contains_unordered: Option<String>,
     stream_output: bool,
     assert: snapbox::Assert,
 }
@@ -824,56 +822,6 @@ impl Execs {
         self.expect_stderr_with_without.push((with, without));
         self
     }
-
-    /// Verifies the JSON output matches the given JSON.
-    ///
-    /// This is typically used when testing cargo commands that emit JSON.
-    /// Each separate JSON object should be separated by a blank line.
-    /// Example:
-    ///
-    /// ```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.
-    #[deprecated(
-        note = "replaced with `Execs::with_stdout_data(expected.is_json().against_jsonlines())`"
-    )]
-    pub fn with_json(&mut self, expected: &str) -> &mut Self {
-        self.expect_json = Some(expected.to_string());
-        self
-    }
-
-    /// Verifies JSON output contains the given objects (in any order) somewhere
-    /// in its output.
-    ///
-    /// CAUTION: Be very careful when using this. Make sure every object is
-    /// unique (not a subset of one another). Also avoid using objects that
-    /// could possibly match multiple output lines unless you're very sure of
-    /// what you are doing.
-    ///
-    /// See `with_json` for more detail.
-    #[deprecated]
-    pub fn with_json_contains_unordered(&mut self, expected: &str) -> &mut Self {
-        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
-    }
 }
 
 /// # Configure the process
@@ -1050,8 +998,6 @@ impl Execs {
             && self.expect_stdout_unordered.is_empty()
             && self.expect_stderr_unordered.is_empty()
             && self.expect_stderr_with_without.is_empty()
-            && self.expect_json.is_none()
-            && self.expect_json_contains_unordered.is_none()
         {
             panic!(
                 "`with_status()` is used, but no output is checked.\n\
@@ -1175,14 +1121,6 @@ impl Execs {
         for (with, without) in self.expect_stderr_with_without.iter() {
             compare::match_with_without(stderr, with, without, cwd)?;
         }
-
-        if let Some(ref expect_json) = self.expect_json {
-            compare::match_json(expect_json, stdout, cwd)?;
-        }
-
-        if let Some(ref expected) = self.expect_json_contains_unordered {
-            compare::match_json_contains_unordered(expected, stdout, cwd)?;
-        }
         Ok(())
     }
 }
@@ -1212,8 +1150,6 @@ pub fn execs() -> Execs {
         expect_stdout_unordered: Vec::new(),
         expect_stderr_unordered: Vec::new(),
         expect_stderr_with_without: Vec::new(),
-        expect_json: None,
-        expect_json_contains_unordered: None,
         stream_output: false,
         assert: compare::assert_e2e(),
     }

crates/cargo-test-support/src/lib.rs

@@ -57,9 +57,10 @@
 //! );
 //! ```
 
-use crate::compare::{assert_match_exact, find_json_mismatch};
+use crate::compare::assert_match_exact;
 use crate::registry::{self, alt_api_path, FeatureMap};
 use flate2::read::GzDecoder;
+use snapbox::prelude::*;
 use std::collections::{HashMap, HashSet};
 use std::fs;
 use std::fs::File;
@@ -133,12 +134,8 @@ fn _validate_upload(
     let json_sz = read_le_u32(&mut f).expect("read json length");
     let mut json_bytes = vec![0; json_sz as usize];
     f.read_exact(&mut json_bytes).expect("read JSON data");
-    let actual_json = serde_json::from_slice(&json_bytes).expect("uploaded JSON should be valid");
-    let expected_json = serde_json::from_str(expected_json).expect("expected JSON does not parse");
 
-    if let Err(e) = find_json_mismatch(&expected_json, &actual_json, None) {
-        panic!("{}", e);
-    }
+    snapbox::assert_data_eq!(json_bytes, expected_json.is_json());
 
     // 32-bit little-endian integer of length of crate file.
     let crate_sz = read_le_u32(&mut f).expect("read crate length");