diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 2efe4e57cc1..b99055e1e1b 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -8,7 +8,6 @@ use std::env; use std::ffi::OsStr; -use std::fmt; use std::fs; use std::os; use std::path::{Path, PathBuf}; @@ -16,6 +15,7 @@ use std::process::{Command, Output}; use std::str; use std::time::{self, Duration}; +use anyhow::{bail, format_err, Result}; use cargo_util::{is_ci, ProcessBuilder, ProcessError}; use serde_json::{self, Value}; use url::Url; @@ -413,19 +413,6 @@ pub fn main_file(println: &str, deps: &[&str]) -> String { buf } -trait ErrMsg { - fn with_err_msg(self, val: String) -> Result; -} - -impl ErrMsg for Result { - fn with_err_msg(self, val: String) -> Result { - match self { - Ok(val) => Ok(val), - Err(err) => Err(format!("{}; original={}", val, err)), - } - } -} - // Path to cargo executables pub fn cargo_dir() -> PathBuf { env::var_os("CARGO_BIN_PATH") @@ -452,7 +439,18 @@ pub fn cargo_exe() -> PathBuf { * */ -pub type MatchResult = Result<(), String>; +/// This is the raw output from the process. +/// +/// This is similar to `std::process::Output`, however the `status` is +/// translated to the raw `code`. This is necessary because `ProcessError` +/// does not have access to the raw `ExitStatus` because `ProcessError` needs +/// to be serializable (for the Rustc cache), and `ExitStatus` does not +/// provide a constructor. +pub struct RawOutput { + pub code: Option, + pub stdout: Vec, + pub stderr: Vec, +} #[must_use] #[derive(Clone)] @@ -703,7 +701,7 @@ impl Execs { self } - pub fn exec_with_output(&mut self) -> anyhow::Result { + pub fn exec_with_output(&mut self) -> Result { self.ran = true; // TODO avoid unwrap let p = (&self.process_builder).clone().unwrap(); @@ -739,7 +737,26 @@ impl Execs { self.ran = true; let p = (&self.process_builder).clone().unwrap(); if let Err(e) = self.match_process(&p) { - panic!("\nExpected: {:?}\n but: {}", self, e) + panic!("\n{}", e) + } + } + + /// Runs the process, checks the expected output, and returns the first + /// JSON object on stdout. + #[track_caller] + pub fn run_json(&mut self) -> serde_json::Value { + self.ran = true; + let p = (&self.process_builder).clone().unwrap(); + match self.match_process(&p) { + Err(e) => panic!("\n{}", e), + Ok(output) => serde_json::from_slice(&output.stdout).unwrap_or_else(|e| { + panic!( + "\nfailed to parse JSON: {}\n\ + output was:\n{}\n", + e, + String::from_utf8_lossy(&output.stdout) + ); + }), } } @@ -747,11 +764,11 @@ impl Execs { pub fn run_output(&mut self, output: &Output) { self.ran = true; if let Err(e) = self.match_output(output) { - panic!("\nExpected: {:?}\n but: {}", self, e) + panic!("\n{}", e) } } - fn verify_checks_output(&self, output: &Output) { + fn verify_checks_output(&self, stdout: &[u8], stderr: &[u8]) { if self.expect_exit_code.unwrap_or(0) != 0 && self.expect_stdout.is_none() && self.expect_stdin.is_none() @@ -772,13 +789,13 @@ impl Execs { "`with_status()` is used, but no output is checked.\n\ The test must check the output to ensure the correct error is triggered.\n\ --- stdout\n{}\n--- stderr\n{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(stdout), + String::from_utf8_lossy(stderr), ); } } - fn match_process(&self, process: &ProcessBuilder) -> MatchResult { + fn match_process(&self, process: &ProcessBuilder) -> Result { println!("running {}", process); let res = if self.stream_output { if is_ci() { @@ -800,7 +817,14 @@ impl Execs { }; match res { - Ok(out) => self.match_output(&out), + Ok(out) => { + self.match_output(&out)?; + return Ok(RawOutput { + stdout: out.stdout, + stderr: out.stderr, + code: out.status.code(), + }); + } Err(e) => { if let Some(ProcessError { stdout: Some(stdout), @@ -809,37 +833,41 @@ impl Execs { .. }) = e.downcast_ref::() { - return self - .match_status(*code, stdout, stderr) + self.match_status(*code, stdout, stderr) .and(self.match_stdout(stdout, stderr)) - .and(self.match_stderr(stdout, stderr)); + .and(self.match_stderr(stdout, stderr))?; + return Ok(RawOutput { + stdout: stdout.to_vec(), + stderr: stderr.to_vec(), + code: *code, + }); } - Err(format!("could not exec process {}: {:?}", process, e)) + bail!("could not exec process {}: {:?}", process, e) } } } - fn match_output(&self, actual: &Output) -> MatchResult { - self.verify_checks_output(actual); + fn match_output(&self, actual: &Output) -> Result<()> { self.match_status(actual.status.code(), &actual.stdout, &actual.stderr) .and(self.match_stdout(&actual.stdout, &actual.stderr)) .and(self.match_stderr(&actual.stdout, &actual.stderr)) } - fn match_status(&self, code: Option, stdout: &[u8], stderr: &[u8]) -> MatchResult { + fn match_status(&self, code: Option, stdout: &[u8], stderr: &[u8]) -> Result<()> { + self.verify_checks_output(stdout, stderr); match self.expect_exit_code { None => Ok(()), Some(expected) if code == Some(expected) => Ok(()), - Some(_) => Err(format!( + Some(_) => bail!( "exited with {:?}\n--- stdout\n{}\n--- stderr\n{}", code, String::from_utf8_lossy(stdout), String::from_utf8_lossy(stderr) - )), + ), } } - fn match_stdout(&self, stdout: &[u8], stderr: &[u8]) -> MatchResult { + fn match_stdout(&self, stdout: &[u8], stderr: &[u8]) -> Result<()> { self.match_std( self.expect_stdout.as_ref(), stdout, @@ -908,12 +936,12 @@ impl Execs { self.match_std(Some(expect), stderr, "stderr", stderr, MatchKind::Partial); if let (Err(_), Err(_)) = (match_std, match_err) { - return Err(format!( + bail!( "expected to find:\n\ {}\n\n\ did not find in either output.", expect - )); + ); } } @@ -923,18 +951,18 @@ impl Execs { if let Some(ref objects) = self.expect_json { let stdout = - str::from_utf8(stdout).map_err(|_| "stdout was not utf8 encoded".to_owned())?; + str::from_utf8(stdout).map_err(|_| format_err!("stdout was not utf8 encoded"))?; let lines = stdout .lines() .filter(|line| line.starts_with('{')) .collect::>(); if lines.len() != objects.len() { - return Err(format!( + bail!( "expected {} json lines, got {}, stdout:\n{}", objects.len(), lines.len(), stdout - )); + ); } for (obj, line) in objects.iter().zip(lines) { self.match_json(obj, line)?; @@ -943,7 +971,7 @@ impl Execs { if !self.expect_json_contains_unordered.is_empty() { let stdout = - str::from_utf8(stdout).map_err(|_| "stdout was not utf8 encoded".to_owned())?; + str::from_utf8(stdout).map_err(|_| format_err!("stdout was not utf8 encoded"))?; let mut lines = stdout .lines() .filter(|line| line.starts_with('{')) @@ -955,14 +983,14 @@ impl Execs { { Some(index) => lines.remove(index), None => { - return Err(format!( + bail!( "Did not find expected JSON:\n\ {}\n\ Remaining available output:\n\ {}\n", serde_json::to_string_pretty(obj).unwrap(), lines.join("\n") - )); + ); } }; } @@ -970,7 +998,7 @@ impl Execs { Ok(()) } - fn match_stderr(&self, stdout: &[u8], stderr: &[u8]) -> MatchResult { + fn match_stderr(&self, stdout: &[u8], stderr: &[u8]) -> Result<()> { self.match_std( self.expect_stderr.as_ref(), stderr, @@ -980,9 +1008,9 @@ impl Execs { ) } - fn normalize_actual(&self, description: &str, actual: &[u8]) -> Result { + fn normalize_actual(&self, description: &str, actual: &[u8]) -> Result { let actual = match str::from_utf8(actual) { - Err(..) => return Err(format!("{} was not utf8 encoded", description)), + Err(..) => bail!("{} was not utf8 encoded", description), Ok(actual) => actual, }; Ok(self.normalize_matcher(actual)) @@ -1002,7 +1030,7 @@ impl Execs { description: &str, extra: &[u8], kind: MatchKind, - ) -> MatchResult { + ) -> Result<()> { let out = match expected { Some(out) => self.normalize_matcher(out), None => return Ok(()), @@ -1019,14 +1047,15 @@ impl Execs { if diffs.is_empty() { Ok(()) } else { - Err(format!( - "differences:\n\ + bail!( + "{} did not match:\n\ {}\n\n\ other output:\n\ `{}`", + description, diffs.join("\n"), String::from_utf8_lossy(extra) - )) + ) } } MatchKind::Partial => { @@ -1043,13 +1072,14 @@ impl Execs { if diffs.is_empty() { Ok(()) } else { - Err(format!( + bail!( "expected to find:\n\ {}\n\n\ did not find in output:\n\ {}", - out, actual - )) + out, + actual + ) } } MatchKind::PartialN(number) => { @@ -1068,13 +1098,15 @@ impl Execs { if matches == number { Ok(()) } else { - Err(format!( + bail!( "expected to find {} occurrences:\n\ {}\n\n\ did not find in output:\n\ {}", - number, out, actual - )) + number, + out, + actual + ) } } MatchKind::NotPresent => { @@ -1089,13 +1121,14 @@ impl Execs { } } if diffs.is_empty() { - Err(format!( + bail!( "expected not to find:\n\ {}\n\n\ but found in output:\n\ {}", - out, actual - )) + out, + actual + ) } else { Ok(()) } @@ -1104,12 +1137,7 @@ impl Execs { } } - fn match_with_without( - &self, - actual: &[u8], - with: &[String], - without: &[String], - ) -> MatchResult { + fn match_with_without(&self, actual: &[u8], with: &[String], without: &[String]) -> Result<()> { let actual = self.normalize_actual("stderr", actual)?; let contains = |s, line| { let mut s = self.normalize_matcher(s); @@ -1123,16 +1151,18 @@ impl Execs { .filter(|line| !without.iter().any(|without| contains(without, line))) .collect(); match matches.len() { - 0 => Err(format!( + 0 => bail!( "Could not find expected line in output.\n\ With contents: {:?}\n\ Without contents: {:?}\n\ Actual stderr:\n\ {}\n", - with, without, actual - )), + with, + without, + actual + ), 1 => Ok(()), - _ => Err(format!( + _ => bail!( "Found multiple matching lines, but only expected one.\n\ With contents: {:?}\n\ Without contents: {:?}\n\ @@ -1141,17 +1171,17 @@ impl Execs { with, without, matches.join("\n") - )), + ), } } - fn match_json(&self, expected: &str, line: &str) -> MatchResult { + fn match_json(&self, expected: &str, line: &str) -> Result<()> { let actual = match line.parse() { - Err(e) => return Err(format!("invalid json, {}:\n`{}`", e, line)), + Err(e) => bail!("invalid json, {}:\n`{}`", e, line), Ok(actual) => actual, }; let expected = match expected.parse() { - Err(e) => return Err(format!("invalid json, {}:\n`{}`", e, line)), + Err(e) => bail!("invalid json, {}:\n`{}`", e, line), Ok(expected) => expected, }; @@ -1231,7 +1261,7 @@ pub fn lines_match(expected: &str, mut actual: &str) -> bool { actual.is_empty() || expected.ends_with("[..]") } -pub fn lines_match_unordered(expected: &str, actual: &str) -> Result<(), String> { +pub fn lines_match_unordered(expected: &str, actual: &str) -> Result<()> { let mut a = actual.lines().collect::>(); // match more-constrained lines first, although in theory we'll // need some sort of recursive match here. This handles the case @@ -1252,19 +1282,19 @@ pub fn lines_match_unordered(expected: &str, actual: &str) -> Result<(), String> } } if !failures.is_empty() { - return Err(format!( + bail!( "Did not find expected line(s):\n{}\n\ Remaining available output:\n{}\n", failures.join("\n"), a.join("\n") - )); + ); } if !a.is_empty() { - Err(format!( + bail!( "Output included extra lines:\n\ {}\n", a.join("\n") - )) + ) } else { Ok(()) } @@ -1334,19 +1364,15 @@ fn lines_match_works() { /// 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<(), String> { +pub 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)) => Err(format!( + 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(()), } } @@ -1421,12 +1447,6 @@ fn zip_all, I2: Iterator>(a: I1, b: I2) -> Z } } -impl fmt::Debug for Execs { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "execs") - } -} - pub fn execs() -> Execs { Execs { ran: false, diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs index 149b6dd298a..d23c171daff 100644 --- a/tests/testsuite/metabuild.rs +++ b/tests/testsuite/metabuild.rs @@ -427,13 +427,10 @@ fn metabuild_metadata() { // The metabuild Target is filtered out of the `metadata` results. let p = basic_project(); - let output = p + let meta = p .cargo("metadata --format-version=1") .masquerade_as_nightly_cargo() - .exec_with_output() - .expect("cargo metadata failed"); - let stdout = str::from_utf8(&output.stdout).unwrap(); - let meta: serde_json::Value = serde_json::from_str(stdout).expect("failed to parse json"); + .run_json(); let mb_info: Vec<&str> = meta["packages"] .as_array() .unwrap() diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index a6545582e58..312603d984f 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -4,6 +4,7 @@ use cargo_test_support::install::cargo_home; use cargo_test_support::paths::CargoPathExt; use cargo_test_support::registry::Package; use cargo_test_support::{basic_bin_manifest, basic_lib_manifest, main_file, project, rustc_host}; +use serde_json::json; #[cargo_test] fn cargo_metadata_simple() { @@ -1550,106 +1551,8 @@ fn package_default_run() { "#, ) .build(); - p.cargo("metadata") - .with_json( - r#" - { - "packages": [ - { - "authors": [ - "wycats@example.com" - ], - "categories": [], - "default_run": "a", - "dependencies": [], - "description": null, - "edition": "2018", - "features": {}, - "id": "foo 0.1.0 (path+file:[..])", - "keywords": [], - "license": null, - "license_file": null, - "links": null, - "manifest_path": "[..]Cargo.toml", - "metadata": null, - "publish": null, - "name": "foo", - "readme": null, - "repository": null, - "homepage": null, - "documentation": null, - "source": null, - "targets": [ - { - "crate_types": [ - "lib" - ], - "doc": true, - "doctest": true, - "test": true, - "edition": "2018", - "kind": [ - "lib" - ], - "name": "foo", - "src_path": "[..]src/lib.rs" - }, - { - "crate_types": [ - "bin" - ], - "doc": true, - "doctest": false, - "test": true, - "edition": "2018", - "kind": [ - "bin" - ], - "name": "a", - "src_path": "[..]src/bin/a.rs", - "test": true - }, - { - "crate_types": [ - "bin" - ], - "doc": true, - "doctest": false, - "test": true, - "edition": "2018", - "kind": [ - "bin" - ], - "name": "b", - "src_path": "[..]src/bin/b.rs", - "test": true - } - ], - "version": "0.1.0" - } - ], - "resolve": { - "nodes": [ - { - "dependencies": [], - "deps": [], - "features": [], - "id": "foo 0.1.0 (path+file:[..])" - } - ], - "root": "foo 0.1.0 (path+file:[..])" - }, - "target_directory": "[..]", - "version": 1, - "workspace_members": [ - "foo 0.1.0 (path+file:[..])" - ], - "workspace_root": "[..]", - "metadata": null - } - "#, - ) - .run(); + let json = p.cargo("metadata").run_json(); + assert_eq!(json["packages"][0]["default_run"], json!("a")); } #[cargo_test]