Auto merge of #9577 - ehuss:json-test, r=alexcrichton

Fix package_default_run test.

The `package_default_run` test was checking the output of `cargo metadata`, which included the "targets" field. Unfortunately the order of the targets in this test depend on the filesystem order, so the test may randomly fail.

The fix here is to just check for the one field that this test was interested in.

An alternate solution would be to sort the targets.  Another alternate solution would be to use `"{...}"` for the targets to ignore them.  I kinda liked simplifying it to check just one field.

This includes a series of commits that are just general changes to the test infrastructure:

* Change cargo-test-support to use anyhow.
* Remove unused `ErrMsg`.
* Fix a bug with `verify_checks_output`.
* Remove the weird Debug impl for Execs.
* Added a helper function for getting the JSON output from cargo.  (I can imagine a lot of possible enhancements here.)
This commit is contained in:
bors 2021-06-12 18:00:01 +00:00
commit 44456677b5
3 changed files with 114 additions and 194 deletions

View file

@ -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<T> {
fn with_err_msg(self, val: String) -> Result<T, String>;
}
impl<T, E: fmt::Display> ErrMsg<T> for Result<T, E> {
fn with_err_msg(self, val: String) -> Result<T, String> {
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<i32>,
pub stdout: Vec<u8>,
pub stderr: Vec<u8>,
}
#[must_use]
#[derive(Clone)]
@ -703,7 +701,7 @@ impl Execs {
self
}
pub fn exec_with_output(&mut self) -> anyhow::Result<Output> {
pub fn exec_with_output(&mut self) -> Result<Output> {
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<RawOutput> {
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::<ProcessError>()
{
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<i32>, stdout: &[u8], stderr: &[u8]) -> MatchResult {
fn match_status(&self, code: Option<i32>, 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::<Vec<_>>();
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<String, String> {
fn normalize_actual(&self, description: &str, actual: &[u8]) -> Result<String> {
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::<Vec<_>>();
// 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<T, I1: Iterator<Item = T>, I2: Iterator<Item = T>>(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,

View file

@ -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()

View file

@ -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]