From f80fc018a60a28ec00d7cf50b1279a5e07d68ceb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 18 Nov 2024 20:52:53 -0600 Subject: [PATCH 1/3] test: Ensure we don't rely on unit return values This will allow changing `Execs::run` return type --- tests/testsuite/build.rs | 2 +- tests/testsuite/build_script.rs | 2 +- tests/testsuite/config.rs | 2 +- tests/testsuite/future_incompat_report.rs | 2 +- tests/testsuite/metadata.rs | 2 +- tests/testsuite/open_namespaces.rs | 14 ++++++------ tests/testsuite/package.rs | 2 +- tests/testsuite/pkgid.rs | 2 +- tests/testsuite/precise_pre_release.rs | 2 +- tests/testsuite/pub_priv.rs | 26 +++++++++++------------ tests/testsuite/registry.rs | 4 ++-- tests/testsuite/test.rs | 2 +- 12 files changed, 31 insertions(+), 31 deletions(-) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 9f8649f827a..37a6d7cbdb7 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -82,7 +82,7 @@ fn build_with_symlink_to_path_dependency_with_build_script_in_git() { // It is necessary to have a sub-repository and to add files so there is an index. let repo = git::init(&root.join("original")); git::add(&repo); - cargo_process("build").run() + cargo_process("build").run(); } #[cargo_test] diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 897a0b493ef..5725c728ea7 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -5234,7 +5234,7 @@ fn dev_dep_with_links() { .file("bar/build.rs", "fn main() {}") .file("bar/src/lib.rs", "") .build(); - p.cargo("check --tests").run() + p.cargo("check --tests").run(); } #[cargo_test] diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 8f9092c2fb2..ddaa168eb97 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -1640,7 +1640,7 @@ fn cargo_target_empty_env() { "#]]) .with_status(101) - .run() + .run(); } #[cargo_test] diff --git a/tests/testsuite/future_incompat_report.rs b/tests/testsuite/future_incompat_report.rs index b6e15f1d757..8b3c6700ef6 100644 --- a/tests/testsuite/future_incompat_report.rs +++ b/tests/testsuite/future_incompat_report.rs @@ -453,5 +453,5 @@ big_update v1.0.0 has the following newer versions available: 2.0.0 with_updates v1.0.0 has the following newer versions available: 1.0.1, 1.0.2, 3.0.1 ... "#]]) - .run() + .run(); } diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index a923c30a84c..5c9015a87a7 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -3401,7 +3401,7 @@ fn metadata_links() { "#]] .is_json(), ) - .run() + .run(); } #[cargo_test] diff --git a/tests/testsuite/open_namespaces.rs b/tests/testsuite/open_namespaces.rs index 4365af26604..6fc018f111e 100644 --- a/tests/testsuite/open_namespaces.rs +++ b/tests/testsuite/open_namespaces.rs @@ -31,7 +31,7 @@ Caused by: See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#open-namespaces for more information about the status of this feature. "#]]) - .run() + .run(); } #[cargo_test] @@ -100,7 +100,7 @@ fn implicit_lib_within_namespace() { .is_json(), ) .with_stderr_data("") - .run() + .run(); } #[cargo_test] @@ -169,7 +169,7 @@ fn implicit_bin_within_namespace() { .is_json(), ) .with_stderr_data("") - .run() + .run(); } #[cargo_test] @@ -256,7 +256,7 @@ fn explicit_bin_within_namespace() { .is_json(), ) .with_stderr_data("") - .run() + .run(); } #[cargo_test] @@ -354,7 +354,7 @@ path+[ROOTURL]/foo#foo::bar@0.0.1 "#]]) .with_stderr_data("") - .run() + .run(); } #[cargo_test] @@ -384,7 +384,7 @@ fn update_spec_accepts_namespaced_name() { [LOCKING] 0 packages to latest compatible versions "#]]) - .run() + .run(); } #[cargo_test] @@ -414,7 +414,7 @@ fn update_spec_accepts_namespaced_pkgid() { [LOCKING] 0 packages to latest compatible versions "#]]) - .run() + .run(); } #[cargo_test] diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index b834493aeaf..0bfed0f53c0 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3923,7 +3923,7 @@ See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for [PACKAGED] 6 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) "#]]) - .run() + .run(); } #[cargo_test] diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 02656031658..9b74420c0ba 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -403,5 +403,5 @@ fn pkgid_json_message_metadata_consistency() { "#]] .is_json(), ) - .run() + .run(); } diff --git a/tests/testsuite/precise_pre_release.rs b/tests/testsuite/precise_pre_release.rs index 6c6b6cc55ff..71c9a3a787e 100644 --- a/tests/testsuite/precise_pre_release.rs +++ b/tests/testsuite/precise_pre_release.rs @@ -39,7 +39,7 @@ if you are looking for the prerelease package it needs to be specified explicitl perhaps a crate was updated and forgotten to be re-vendored? "#]]) - .run() + .run(); } #[cargo_test] diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs index 6d81faced09..b9769809749 100644 --- a/tests/testsuite/pub_priv.rs +++ b/tests/testsuite/pub_priv.rs @@ -42,7 +42,7 @@ fn exported_priv_warning() { src/lib.rs:3:13: [WARNING] type `FromPriv` from private dependency 'priv_dep' in public interface ... "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -87,7 +87,7 @@ fn exported_pub_dep() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test] @@ -113,7 +113,7 @@ Caused by: See https://doc.rust-lang.org/[..]cargo/reference/unstable.html#public-dependency for more information about using this feature. "#]]) - .run() + .run(); } #[cargo_test] @@ -151,7 +151,7 @@ fn requires_feature() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test] @@ -194,7 +194,7 @@ Caused by: 'public' specifier can only be used on regular dependencies, not dev-dependencies "#]]) - .run() + .run(); } #[cargo_test] @@ -233,7 +233,7 @@ fn pub_dev_dependency_without_feature() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -291,7 +291,7 @@ Caused by: foo2 is public, but workspace dependencies cannot be public "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -336,7 +336,7 @@ fn allow_priv_in_tests() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -381,7 +381,7 @@ fn allow_priv_in_benchs() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -427,7 +427,7 @@ fn allow_priv_in_bins() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -473,7 +473,7 @@ fn allow_priv_in_examples() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -520,7 +520,7 @@ fn allow_priv_in_custom_build() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] @@ -576,7 +576,7 @@ fn publish_package_with_public_dependency() { [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .run() + .run(); } #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 193e093bbeb..896aab44bb8 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -3242,7 +3242,7 @@ fn protocol() { [ERROR] unsupported registry protocol `invalid` (defined in environment variable `CARGO_REGISTRIES_CRATES_IO_PROTOCOL`) "#]]) - .run() + .run(); } #[cargo_test] @@ -3253,7 +3253,7 @@ fn http_requires_trailing_slash() { [ERROR] sparse registry url must end in a slash `/`: sparse+https://invalid.crates.io/test "#]]) - .run() + .run(); } // Limit the test to debug builds so that `__CARGO_TEST_MAX_UNPACK_SIZE` will take affect. diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index d96309a6828..48d69526ad7 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -4307,7 +4307,7 @@ fn test_hint_workspace_virtual() { "#]]) .with_status(101) - .run() + .run(); } #[cargo_test] From 8b41a8ecbe17b7631d87ae753a035c5a5ea851c3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 18 Nov 2024 20:53:50 -0600 Subject: [PATCH 2/3] feat(test): Provide access to 'RawOutput' from 'Execs::run' This will allow more custom assertions. --- crates/cargo-test-support/src/lib.rs | 38 +++++++++++++--------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 7812835f694..a36e61c3b80 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -623,12 +623,10 @@ pub fn cargo_exe() -> PathBuf { /// 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. -struct RawOutput { - #[allow(dead_code)] - code: Option, - stdout: Vec, - #[allow(dead_code)] - stderr: Vec, +pub struct RawOutput { + pub code: Option, + pub stdout: Vec, + pub stderr: Vec, } /// Run and verify a [`ProcessBuilder`] @@ -1042,14 +1040,16 @@ impl Execs { } #[track_caller] - pub fn run(&mut self) { + pub fn run(&mut self) -> RawOutput { self.ran = true; let mut p = (&self.process_builder).clone().unwrap(); if let Some(stdin) = self.expect_stdin.take() { p.stdin(stdin); } - if let Err(e) = self.match_process(&p) { - panic_error(&format!("test failed running {}", p), e); + + match self.match_process(&p) { + Err(e) => panic_error(&format!("test failed running {}", p), e), + Ok(output) => output, } } @@ -1057,19 +1057,15 @@ impl Execs { /// 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_error(&format!("test failed running {}", p), e), - Ok(output) => serde_json::from_slice(&output.stdout).unwrap_or_else(|e| { - panic!( - "\nfailed to parse JSON: {}\n\ + let output = self.run(); + 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) - ); - }), - } + e, + String::from_utf8_lossy(&output.stdout) + ); + }) } #[track_caller] From 0b10d6f3688c7493d73ccddb077d785a21ec6d61 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 20 Nov 2024 15:29:50 -0600 Subject: [PATCH 3/3] test(rustflags): Verify -Cmetadata directly, not through -Cextra-filename This unblocks experimenting with having these diverge for #8716 --- tests/testsuite/rustflags.rs | 53 ++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/tests/testsuite/rustflags.rs b/tests/testsuite/rustflags.rs index 582439d04d4..0d73f4b99aa 100644 --- a/tests/testsuite/rustflags.rs +++ b/tests/testsuite/rustflags.rs @@ -4,7 +4,10 @@ use std::fs; use cargo_test_support::prelude::*; use cargo_test_support::registry::Package; -use cargo_test_support::{basic_manifest, paths, project, project_in_home, rustc_host, str}; +use cargo_test_support::{ + basic_manifest, paths, project, project_in_home, rustc_host, str, RawOutput, +}; +use snapbox::assert_data_eq; #[cargo_test] fn env_rustflags_normal_source() { @@ -1478,36 +1481,44 @@ fn env_rustflags_misspelled_build_script() { #[cargo_test] fn remap_path_prefix_ignored() { - // Ensure that --remap-path-prefix does not affect metadata hash. - let p = project().file("src/lib.rs", "").build(); - p.cargo("build").run(); - let rlibs = p - .glob("target/debug/deps/*.rlib") - .collect::, _>>() - .unwrap(); - assert_eq!(rlibs.len(), 1); - p.cargo("clean").run(); - - let check_metadata_same = || { - let rlibs2 = p - .glob("target/debug/deps/*.rlib") - .collect::, _>>() - .unwrap(); - assert_eq!(rlibs, rlibs2); + let get_c_metadata_re = + regex::Regex::new(r".* (--crate-name [^ ]+).* (-C ?metadata=[^ ]+).*").unwrap(); + let get_c_metadata = |output: RawOutput| { + let stderr = String::from_utf8(output.stderr).unwrap(); + let mut c_metadata = get_c_metadata_re + .captures_iter(&stderr) + .map(|c| { + let (_, [name, c_metadata]) = c.extract(); + format!("{name} {c_metadata}") + }) + .collect::>(); + assert!( + !c_metadata.is_empty(), + "`{get_c_metadata_re:?}` did not match:\n```\n{stderr}\n```" + ); + c_metadata.sort(); + c_metadata.join("\n") }; - p.cargo("build") + let p = project().file("src/lib.rs", "").build(); + + let build_output = p + .cargo("build -v") .env( "RUSTFLAGS", "--remap-path-prefix=/abc=/zoo --remap-path-prefix /spaced=/zoo", ) .run(); - check_metadata_same(); + let build_c_metadata = dbg!(get_c_metadata(build_output)); p.cargo("clean").run(); - p.cargo("rustc -- --remap-path-prefix=/abc=/zoo --remap-path-prefix /spaced=/zoo") + + let rustc_output = p + .cargo("rustc -v -- --remap-path-prefix=/abc=/zoo --remap-path-prefix /spaced=/zoo") .run(); - check_metadata_same(); + let rustc_c_metadata = dbg!(get_c_metadata(rustc_output)); + + assert_data_eq!(rustc_c_metadata, build_c_metadata); } #[cargo_test]