From da535b9155fdab852dfd18006a5585612e50a70c Mon Sep 17 00:00:00 2001 From: jyn Date: Tue, 10 Dec 2024 03:03:28 -0500 Subject: [PATCH 1/2] Fix `--nocapture` for run-make tests This was confusing because there are three layers of output hiding. 1. libtest shoves all output into a buffer and does not print it unless the test fails or `--nocapture` is passed. 2. compiletest chooses whether to print the output from any given process. 3. run-make-support chooses what output to print. This modifies 2 and 3. - compiletest: Don't require both `--verbose` and `--nocapture` to show the output of run-make tests. - compiletest: Distinguish rustc and rmake stderr by printing the command name (e.g. "--stderr--" to "--rustc stderr--"). - run-make-support: Unconditionally print the needle/haystack being searched. Previously this was only printed on failure. Before: ``` $ x t tests/run-make/linker-warning --force-rerun -- --nocapture running 1 tests . test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 281.64ms $ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture 2>&1 | wc -l 1004 $ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture | tail -n40 running 1 tests ------stdout------------------------------ ------stderr------------------------------ warning: unused import: `std::path::Path` --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:1:5 | 1 | use std::path::Path; | ^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default warning: unused import: `run_make_support::rfs::remove_file` --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:3:5 | 3 | use run_make_support::rfs::remove_file; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: 2 warnings emitted ------------------------------------------ test [run-make] tests/run-make/linker-warning ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 285.89ms ``` After: ``` Testing stage1 compiletest suite=run-make mode=run-make (x86_64-unknown-linux-gnu) running 1 tests ------rmake stdout------------------------------ ------rmake stderr------------------------------ assert_contains_regex: === HAYSTACK === error: linking with `./fake-linker` failed: exit status: 1 | = note: LC_ALL="C" PATH="/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin:...:/bin" VSLANG="1033" "./fake-linker" "-m64" "/tmp/rustcYqdAZT/symbols.o" "main.main.d17f5fbe6225cf88-cgu.0.rcgu.o" "main.2uoctswmurc6ir5rvoay0p9ke.rcgu.o" "-Wl,--as-needed" "-Wl,-Bstatic" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-B/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld" "-fuse-ld=lld" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/test/run-make/linker-warning/rmake_out" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "main" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs" "run_make_error" = note: error: baz error: aborting due to 1 previous error === NEEDLE === fake-linker.*run_make_error assert_not_contains_regex: === HAYSTACK === === NEEDLE === fake-linker.*run_make_error ------------------------------------------ . test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 314.81ms ``` --- src/tools/compiletest/src/runtest.rs | 38 +++++++----- src/tools/compiletest/src/runtest/run_make.rs | 13 ++--- .../run-make-support/src/assertion_helpers.rs | 58 ++++++++----------- 3 files changed, 54 insertions(+), 55 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 7b11bf3b1219c..4a4494a1e23d1 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -410,7 +410,12 @@ impl<'test> TestCx<'test> { truncated: Truncated::No, cmdline: format!("{cmd:?}"), }; - self.dump_output(&proc_res.stdout, &proc_res.stderr); + self.dump_output( + self.config.verbose, + &cmd.get_program().to_string_lossy(), + &proc_res.stdout, + &proc_res.stderr, + ); proc_res } @@ -1401,7 +1406,12 @@ impl<'test> TestCx<'test> { cmdline, }; - self.dump_output(&result.stdout, &result.stderr); + self.dump_output( + self.config.verbose, + &command.get_program().to_string_lossy(), + &result.stdout, + &result.stderr, + ); result } @@ -1816,12 +1826,22 @@ impl<'test> TestCx<'test> { } } - fn dump_output(&self, out: &str, err: &str) { + fn dump_output(&self, print_output: bool, proc_name: &str, out: &str, err: &str) { let revision = if let Some(r) = self.revision { format!("{}.", r) } else { String::new() }; self.dump_output_file(out, &format!("{}out", revision)); self.dump_output_file(err, &format!("{}err", revision)); - self.maybe_dump_to_stdout(out, err); + + if !print_output { + return; + } + + let proc_name = Path::new(proc_name).file_name().unwrap().to_string_lossy(); + println!("------{proc_name} stdout------------------------------"); + println!("{}", out); + println!("------{proc_name} stderr------------------------------"); + println!("{}", err); + println!("------------------------------------------"); } fn dump_output_file(&self, out: &str, extension: &str) { @@ -1874,16 +1894,6 @@ impl<'test> TestCx<'test> { output_base_name(self.config, self.testpaths, self.safe_revision()) } - fn maybe_dump_to_stdout(&self, out: &str, err: &str) { - if self.config.verbose { - println!("------stdout------------------------------"); - println!("{}", out); - println!("------stderr------------------------------"); - println!("{}", err); - println!("------------------------------------------"); - } - } - fn error(&self, err: &str) { match self.revision { Some(rev) => println!("\nerror in revision `{}`: {}", rev, err), diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs index 04bc2d7787da7..85ade5b727a3b 100644 --- a/src/tools/compiletest/src/runtest/run_make.rs +++ b/src/tools/compiletest/src/runtest/run_make.rs @@ -517,14 +517,13 @@ impl TestCx<'_> { let proc = disable_error_reporting(|| cmd.spawn().expect("failed to spawn `rmake`")); let (Output { stdout, stderr, status }, truncated) = self.read2_abbreviated(proc); + let stdout = String::from_utf8_lossy(&stdout).into_owned(); + let stderr = String::from_utf8_lossy(&stderr).into_owned(); + // This conditions on `status.success()` so we don't print output twice on error. + // NOTE: this code is called from a libtest thread, so it's hidden by default unless --nocapture is passed. + self.dump_output(status.success(), &cmd.get_program().to_string_lossy(), &stdout, &stderr); if !status.success() { - let res = ProcRes { - status, - stdout: String::from_utf8_lossy(&stdout).into_owned(), - stderr: String::from_utf8_lossy(&stderr).into_owned(), - truncated, - cmdline: format!("{:?}", cmd), - }; + let res = ProcRes { status, stdout, stderr, truncated, cmdline: format!("{:?}", cmd) }; self.fatal_proc_rec("rmake recipe failed to complete", &res); } } diff --git a/src/tools/run-make-support/src/assertion_helpers.rs b/src/tools/run-make-support/src/assertion_helpers.rs index b4da65aff4ab1..e84a3cf633f97 100644 --- a/src/tools/run-make-support/src/assertion_helpers.rs +++ b/src/tools/run-make-support/src/assertion_helpers.rs @@ -5,16 +5,31 @@ use std::path::Path; use crate::{fs, regex}; +fn print<'a, 'e, A: AsRef, E: AsRef>( + assertion_kind: &str, + haystack: &'a A, + needle: &'e E, +) -> (&'a str, &'e str) { + let haystack = haystack.as_ref(); + let needle = needle.as_ref(); + eprintln!("{assertion_kind}:"); + eprintln!("=== HAYSTACK ==="); + eprintln!("{}", haystack); + eprintln!("=== NEEDLE ==="); + eprintln!("{}", needle); + (haystack, needle) +} + /// Assert that `actual` is equal to `expected`. #[track_caller] pub fn assert_equals, E: AsRef>(actual: A, expected: E) { let actual = actual.as_ref(); let expected = expected.as_ref(); + eprintln!("=== ACTUAL TEXT ==="); + eprintln!("{}", actual); + eprintln!("=== EXPECTED ==="); + eprintln!("{}", expected); if actual != expected { - eprintln!("=== ACTUAL TEXT ==="); - eprintln!("{}", actual); - eprintln!("=== EXPECTED ==="); - eprintln!("{}", expected); panic!("expected text was not found in actual text"); } } @@ -22,13 +37,8 @@ pub fn assert_equals, E: AsRef>(actual: A, expected: E) { /// Assert that `haystack` contains `needle`. #[track_caller] pub fn assert_contains, N: AsRef>(haystack: H, needle: N) { - let haystack = haystack.as_ref(); - let needle = needle.as_ref(); + let (haystack, needle) = print("assert_contains", &haystack, &needle); if !haystack.contains(needle) { - eprintln!("=== HAYSTACK ==="); - eprintln!("{}", haystack); - eprintln!("=== NEEDLE ==="); - eprintln!("{}", needle); panic!("needle was not found in haystack"); } } @@ -36,13 +46,8 @@ pub fn assert_contains, N: AsRef>(haystack: H, needle: N) { /// Assert that `haystack` does not contain `needle`. #[track_caller] pub fn assert_not_contains, N: AsRef>(haystack: H, needle: N) { - let haystack = haystack.as_ref(); - let needle = needle.as_ref(); + let (haystack, needle) = print("assert_not_contains", &haystack, &needle); if haystack.contains(needle) { - eprintln!("=== HAYSTACK ==="); - eprintln!("{}", haystack); - eprintln!("=== NEEDLE ==="); - eprintln!("{}", needle); panic!("needle was unexpectedly found in haystack"); } } @@ -50,14 +55,9 @@ pub fn assert_not_contains, N: AsRef>(haystack: H, needle: N) /// Assert that `haystack` contains the regex pattern `needle`. #[track_caller] pub fn assert_contains_regex, N: AsRef>(haystack: H, needle: N) { - let haystack = haystack.as_ref(); - let needle = needle.as_ref(); + let (haystack, needle) = print("assert_contains_regex", &haystack, &needle); let re = regex::Regex::new(needle).unwrap(); if !re.is_match(haystack) { - eprintln!("=== HAYSTACK ==="); - eprintln!("{}", haystack); - eprintln!("=== NEEDLE ==="); - eprintln!("{}", needle); panic!("needle was not found in haystack"); } } @@ -65,14 +65,9 @@ pub fn assert_contains_regex, N: AsRef>(haystack: H, needle: /// Assert that `haystack` does not contain the regex pattern `needle`. #[track_caller] pub fn assert_not_contains_regex, N: AsRef>(haystack: H, needle: N) { - let haystack = haystack.as_ref(); - let needle = needle.as_ref(); + let (haystack, needle) = print("assert_not_contains_regex", &haystack, &needle); let re = regex::Regex::new(needle).unwrap(); if re.is_match(haystack) { - eprintln!("=== HAYSTACK ==="); - eprintln!("{}", haystack); - eprintln!("=== NEEDLE ==="); - eprintln!("{}", needle); panic!("needle was unexpectedly found in haystack"); } } @@ -80,13 +75,8 @@ pub fn assert_not_contains_regex, N: AsRef>(haystack: H, need /// Assert that `haystack` contains `needle` a `count` number of times. #[track_caller] pub fn assert_count_is, N: AsRef>(count: usize, haystack: H, needle: N) { - let haystack = haystack.as_ref(); - let needle = needle.as_ref(); + let (haystack, needle) = print("assert_count_is", &haystack, &needle); if count != haystack.matches(needle).count() { - eprintln!("=== HAYSTACK ==="); - eprintln!("{}", haystack); - eprintln!("=== NEEDLE ==="); - eprintln!("{}", needle); panic!("needle did not appear {count} times in haystack"); } } From 056eb758e7e68df9c1cdcc3a91a07372df4de49c Mon Sep 17 00:00:00 2001 From: jyn Date: Thu, 12 Dec 2024 08:07:59 -0500 Subject: [PATCH 2/2] show which test the `rmake` process belongs to --- src/tools/compiletest/src/runtest.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 4a4494a1e23d1..8af4325e7b106 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::fs::{self, File, create_dir_all}; use std::hash::{DefaultHasher, Hash, Hasher}; use std::io::prelude::*; @@ -1836,7 +1836,20 @@ impl<'test> TestCx<'test> { return; } - let proc_name = Path::new(proc_name).file_name().unwrap().to_string_lossy(); + let path = Path::new(proc_name); + let proc_name = if path.file_stem().is_some_and(|p| p == "rmake") { + OsString::from_iter( + path.parent() + .unwrap() + .file_name() + .into_iter() + .chain(Some(OsStr::new("/"))) + .chain(path.file_name()), + ) + } else { + path.file_name().unwrap().into() + }; + let proc_name = proc_name.to_string_lossy(); println!("------{proc_name} stdout------------------------------"); println!("{}", out); println!("------{proc_name} stderr------------------------------");