Skip to content

Commit

Permalink
test: Switch from 'exec_with_output' to 'run' (#14848)
Browse files Browse the repository at this point in the history
### What does this PR try to resolve?

This is a follow up to #14846 which changed `run` to return the
`RawOutput`.

Reasons I didn't "update" some code to the new `run` return value
- We were actually using `ProcessBuilder::exec_with_output` and I didn't
want to disentangle what it would take to switch to `Execs`
- We did processing on the `Result` and I didn't want to check how that
could be updated

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

### Additional information
  • Loading branch information
weihanglo authored Nov 21, 2024
2 parents dfcfa44 + 55350fc commit 81848c3
Show file tree
Hide file tree
Showing 14 changed files with 42 additions and 116 deletions.
4 changes: 1 addition & 3 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5452,9 +5452,7 @@ fn same_metadata_different_directory() {
.file("Cargo.toml", &basic_bin_manifest("foo"))
.file("src/foo.rs", &main_file(r#""i am foo""#, &[]))
.build();
let output = t!(String::from_utf8(
t!(p.cargo("build -v").exec_with_output()).stderr,
));
let output = t!(String::from_utf8(p.cargo("build -v").run().stderr,));
let metadata = output
.split_whitespace()
.find(|arg| arg.starts_with("metadata="))
Expand Down
60 changes: 12 additions & 48 deletions tests/testsuite/cache_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,11 @@ fn simple() {
let rustc_output = raw_rustc_output(&p, "src/lib.rs", &[]);

// -q so the output is the same as rustc (no "Compiling" or "Finished").
let cargo_output1 = p
.cargo("check -q --color=never")
.exec_with_output()
.expect("cargo to run");
let cargo_output1 = p.cargo("check -q --color=never").run();
assert_eq!(rustc_output, as_str(&cargo_output1.stderr));
assert!(cargo_output1.stdout.is_empty());
// Check that the cached version is exactly the same.
let cargo_output2 = p
.cargo("check -q")
.exec_with_output()
.expect("cargo to run");
let cargo_output2 = p.cargo("check -q").run();
assert_eq!(rustc_output, as_str(&cargo_output2.stderr));
assert!(cargo_output2.stdout.is_empty());
}
Expand All @@ -61,14 +55,10 @@ fn simple_short() {

let cargo_output1 = p
.cargo("check -q --color=never --message-format=short")
.exec_with_output()
.expect("cargo to run");
.run();
assert_eq!(rustc_output, as_str(&cargo_output1.stderr));
// assert!(cargo_output1.stdout.is_empty());
let cargo_output2 = p
.cargo("check -q --message-format=short")
.exec_with_output()
.expect("cargo to run");
let cargo_output2 = p.cargo("check -q --message-format=short").run();
println!("{}", String::from_utf8_lossy(&cargo_output2.stdout));
assert_eq!(rustc_output, as_str(&cargo_output2.stderr));
assert!(cargo_output2.stdout.is_empty());
Expand Down Expand Up @@ -102,24 +92,15 @@ fn color() {
assert!(!rustc_nocolor.contains("\x1b["));

// First pass, non-cached, with color, should be the same.
let cargo_output1 = p
.cargo("check -q --color=always")
.exec_with_output()
.expect("cargo to run");
let cargo_output1 = p.cargo("check -q --color=always").run();
compare(&rustc_color, as_str(&cargo_output1.stderr));

// Replay cached, with color.
let cargo_output2 = p
.cargo("check -q --color=always")
.exec_with_output()
.expect("cargo to run");
let cargo_output2 = p.cargo("check -q --color=always").run();
compare(&rustc_color, as_str(&cargo_output2.stderr));

// Replay cached, no color.
let cargo_output_nocolor = p
.cargo("check -q --color=never")
.exec_with_output()
.expect("cargo to run");
let cargo_output_nocolor = p.cargo("check -q --color=never").run();
compare(&rustc_nocolor, as_str(&cargo_output_nocolor.stderr));
}

Expand All @@ -130,27 +111,17 @@ fn cached_as_json() {

// Grab the non-cached output, feature disabled.
// NOTE: When stabilizing, this will need to be redone.
let cargo_output = p
.cargo("check --message-format=json")
.exec_with_output()
.expect("cargo to run");
assert!(cargo_output.status.success());
let cargo_output = p.cargo("check --message-format=json").run();
let orig_cargo_out = as_str(&cargo_output.stdout);
assert!(orig_cargo_out.contains("compiler-message"));
p.cargo("clean").run();

// Check JSON output, not fresh.
let cargo_output1 = p
.cargo("check --message-format=json")
.exec_with_output()
.expect("cargo to run");
let cargo_output1 = p.cargo("check --message-format=json").run();
assert_eq!(as_str(&cargo_output1.stdout), orig_cargo_out);

// Check JSON output, fresh.
let cargo_output2 = p
.cargo("check --message-format=json")
.exec_with_output()
.expect("cargo to run");
let cargo_output2 = p.cargo("check --message-format=json").run();
// The only difference should be this field.
let fix_fresh = as_str(&cargo_output2.stdout).replace("\"fresh\":true", "\"fresh\":false");
assert_eq!(fix_fresh, orig_cargo_out);
Expand Down Expand Up @@ -220,11 +191,7 @@ fn rustdoc() {
)
.build();

let rustdoc_output = p
.cargo("doc -q --color=always")
.exec_with_output()
.expect("rustdoc to run");
assert!(rustdoc_output.status.success());
let rustdoc_output = p.cargo("doc -q --color=always").run();
let rustdoc_stderr = as_str(&rustdoc_output.stderr);
assert!(rustdoc_stderr.contains("missing"));
assert!(rustdoc_stderr.contains("\x1b["));
Expand All @@ -234,10 +201,7 @@ fn rustdoc() {
);

// Check the cached output.
let rustdoc_output = p
.cargo("doc -q --color=always")
.exec_with_output()
.expect("rustdoc to run");
let rustdoc_output = p.cargo("doc -q --color=always").run();
assert_eq!(as_str(&rustdoc_output.stderr), rustdoc_stderr);
}

Expand Down
13 changes: 3 additions & 10 deletions tests/testsuite/cargo_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,7 @@ fn list_command_looks_at_path() {
let mut path = path();
path.push(proj.root().join("path-test"));
let path = env::join_paths(path.iter()).unwrap();
let output = cargo_process("-v --list")
.env("PATH", &path)
.exec_with_output()
.unwrap();
let output = cargo_process("-v --list").env("PATH", &path).run();
let output = str::from_utf8(&output.stdout).unwrap();
assert!(
output.contains("\n 1 "),
Expand All @@ -127,8 +124,7 @@ fn list_command_looks_at_path_case_mismatch() {
let output = cargo_process("-v --list")
.env("Path", &path)
.env_remove("PATH")
.exec_with_output()
.unwrap();
.run();
let output = str::from_utf8(&output.stdout).unwrap();
assert!(
output.contains("\n 1 "),
Expand Down Expand Up @@ -174,10 +170,7 @@ fn list_command_resolves_symlinks() {
let mut path = path();
path.push(proj.root().join("path-test"));
let path = env::join_paths(path.iter()).unwrap();
let output = cargo_process("-v --list")
.env("PATH", &path)
.exec_with_output()
.unwrap();
let output = cargo_process("-v --list").env("PATH", &path).run();
let output = str::from_utf8(&output.stdout).unwrap();
assert!(
output.contains("\n 2 "),
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,7 @@ fn pkgid_querystring_works() {

p.cargo("generate-lockfile").run();

let output = p.cargo("pkgid").arg("gitdep").exec_with_output().unwrap();
let output = p.cargo("pkgid").arg("gitdep").run();
let gitdep_pkgid = String::from_utf8(output.stdout).unwrap();
let gitdep_pkgid = gitdep_pkgid.trim();
assert_e2e().eq(
Expand Down
3 changes: 1 addition & 2 deletions tests/testsuite/freshness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2527,8 +2527,7 @@ LLVM version: 9.0
let output = p
.cargo("check --message-format=json")
.env("RUSTC", compiler.bin(version))
.exec_with_output()
.unwrap();
.run();
// Collect the filenames generated.
let mut artifacts: Vec<_> = std::str::from_utf8(&output.stdout)
.unwrap()
Expand Down
3 changes: 1 addition & 2 deletions tests/testsuite/freshness_checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2435,8 +2435,7 @@ LLVM version: 9.0
.cargo("check -Zchecksum-freshness --message-format=json")
.masquerade_as_nightly_cargo(&["checksum-freshness"])
.env("RUSTC", compiler.bin(version))
.exec_with_output()
.unwrap();
.run();
// Collect the filenames generated.
let mut artifacts: Vec<_> = std::str::from_utf8(&output.stdout)
.unwrap()
Expand Down
8 changes: 2 additions & 6 deletions tests/testsuite/future_incompat_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,7 @@ The package `second-dep v0.0.2` currently triggers the following future incompat
let output = p
.cargo("check")
.env("RUSTFLAGS", "-Zfuture-incompat-test")
.exec_with_output()
.unwrap();
.run();

// Extract the 'id' from the stdout. We are looking
// for the id in a line of the form "run `cargo report future-incompatibilities --id yZ7S`"
Expand Down Expand Up @@ -282,10 +281,7 @@ The package `second-dep v0.0.2` currently triggers the following future incompat
.run();

// Test without --id, and also the full output of the report.
let output = p
.cargo("report future-incompat")
.exec_with_output()
.unwrap();
let output = p.cargo("report future-incompat").run();
let output = std::str::from_utf8(&output.stdout).unwrap();
assert!(output.starts_with("The following warnings were discovered"));
let mut lines = output
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3396,7 +3396,7 @@ fn historical_lockfile_works_with_vendor() {
.file("src/lib.rs", "")
.build();

let output = project.cargo("vendor").exec_with_output().unwrap();
let output = project.cargo("vendor").run();
project.change_file(
".cargo/config.toml",
str::from_utf8(&output.stdout).unwrap(),
Expand Down
18 changes: 5 additions & 13 deletions tests/testsuite/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::str::from_utf8;
use cargo_test_support::prelude::*;
use cargo_test_support::registry::Package;
use cargo_test_support::str;
use cargo_test_support::{basic_manifest, cargo_exe, cargo_process, paths, process, project};
use cargo_test_support::{basic_manifest, cargo_process, paths, project};

#[cargo_test]
fn help() {
Expand Down Expand Up @@ -83,13 +83,9 @@ fn help_with_man_and_path(
.unwrap()
};

let output = process(&cargo_exe())
.arg("help")
.arg(subcommand)
let output = cargo_process(&format!("help {subcommand}"))
.env("PATH", path)
.exec_with_output()
.unwrap();
assert!(output.status.success());
.run();
let stderr = from_utf8(&output.stderr).unwrap();
if display_command.is_empty() {
assert_eq!(stderr, "");
Expand All @@ -101,13 +97,9 @@ fn help_with_man_and_path(
}

fn help_with_stdout_and_path(subcommand: &str, path: &Path) -> String {
let output = process(&cargo_exe())
.arg("help")
.arg(subcommand)
let output = cargo_process(&format!("help {subcommand}"))
.env("PATH", path)
.exec_with_output()
.unwrap();
assert!(output.status.success());
.run();
let stderr = from_utf8(&output.stderr).unwrap();
assert_eq!(stderr, "");
let stdout = from_utf8(&output.stdout).unwrap();
Expand Down
11 changes: 5 additions & 6 deletions tests/testsuite/lto.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::process::Output;

use cargo::core::compiler::Lto;
use cargo_test_support::prelude::*;
use cargo_test_support::registry::Package;
use cargo_test_support::RawOutput;
use cargo_test_support::{basic_manifest, project, str, Project};

#[cargo_test]
Expand Down Expand Up @@ -509,7 +508,7 @@ fn project_with_dep(crate_types: &str) -> Project {
///
/// `krate_info` is extra compiler flags used to distinguish this if the same
/// crate name is being built multiple times.
fn verify_lto(output: &Output, krate: &str, krate_info: &str, expected_lto: Lto) {
fn verify_lto(output: &RawOutput, krate: &str, krate_info: &str, expected_lto: Lto) {
let stderr = std::str::from_utf8(&output.stderr).unwrap();
let mut matches = stderr.lines().filter(|line| {
line.contains("Running")
Expand Down Expand Up @@ -554,7 +553,7 @@ fn verify_lto(output: &Output, krate: &str, krate_info: &str, expected_lto: Lto)
#[cargo_test]
fn cdylib_and_rlib() {
let p = project_with_dep("'cdylib', 'rlib'");
let output = p.cargo("build --release -v").exec_with_output().unwrap();
let output = p.cargo("build --release -v").run();
// `registry` is ObjectAndBitcode because it needs Object for the
// rlib, and Bitcode for the cdylib (which doesn't support LTO).
verify_lto(
Expand Down Expand Up @@ -627,7 +626,7 @@ fn cdylib_and_rlib() {
#[cargo_test]
fn dylib() {
let p = project_with_dep("'dylib'");
let output = p.cargo("build --release -v").exec_with_output().unwrap();
let output = p.cargo("build --release -v").run();
// `registry` is OnlyObject because rustc doesn't support LTO with dylibs.
verify_lto(&output, "registry", "--crate-type lib", Lto::OnlyObject);
// `registry_shared` is both because it is needed by both bar (Object) and
Expand Down Expand Up @@ -860,7 +859,7 @@ fn dylib_rlib_bin() {
.file("src/bin/ferret.rs", "fn main() { foo::foo(); }")
.build();

let output = p.cargo("build --release -v").exec_with_output().unwrap();
let output = p.cargo("build --release -v").run();
verify_lto(
&output,
"foo",
Expand Down
8 changes: 4 additions & 4 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3143,7 +3143,7 @@ path = "src/lib.rs"
}

fn verify_packaged_status_line(
output: std::process::Output,
output: cargo_test_support::RawOutput,
num_files: usize,
uncompressed_size: u64,
compressed_size: u64,
Expand Down Expand Up @@ -3228,7 +3228,7 @@ version = "0.0.1"
+ main_rs_contents.len()
+ cargo_toml_contents.len()
+ cargo_lock_contents.len()) as u64;
let output = p.cargo("package").exec_with_output().unwrap();
let output = p.cargo("package").run();

assert!(p.root().join("target/package/foo-0.0.1.crate").is_file());
p.cargo("package -l")
Expand Down Expand Up @@ -3333,7 +3333,7 @@ version = "0.0.1"
+ cargo_lock_contents.len()
+ bar_txt_contents.len()) as u64;

let output = p.cargo("package").exec_with_output().unwrap();
let output = p.cargo("package").run();
assert!(p.root().join("target/package/foo-0.0.1.crate").is_file());
p.cargo("package -l")
.with_stdout_data(str![[r#"
Expand Down Expand Up @@ -3452,7 +3452,7 @@ version = "0.0.1"
+ cargo_lock_contents.len()
+ bar_txt_contents.len() * 2) as u64;

let output = p.cargo("package").exec_with_output().unwrap();
let output = p.cargo("package").run();
assert!(p.root().join("target/package/foo-0.0.1.crate").is_file());
p.cargo("package -l")
.with_stdout_data(str![[r#"
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/pkgid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ fn pkgid_json_message_metadata_consistency() {

p.cargo("generate-lockfile").run();

let output = p.cargo("pkgid").arg("foo").exec_with_output().unwrap();
let output = p.cargo("pkgid").arg("foo").run();
let pkgid = String::from_utf8(output.stdout).unwrap();
let pkgid = pkgid.trim();
assert_e2e().eq(pkgid, str!["path+[ROOTURL]/foo#0.5.0"]);
Expand Down
3 changes: 1 addition & 2 deletions tests/testsuite/registry_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,7 @@ fn token_not_logged() {
.replace_crates_io(crates_io.index_url())
.env("CARGO_HTTP_DEBUG", "true")
.env("CARGO_LOG", "trace")
.exec_with_output()
.unwrap();
.run();
let log = String::from_utf8(output.stderr).unwrap();
assert_e2e().eq(
&log,
Expand Down
Loading

0 comments on commit 81848c3

Please sign in to comment.