From 3d54f19421d07781e1ce7283ed99b4eb39a08bb5 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 28 Aug 2025 13:53:10 +1000 Subject: [PATCH 1/4] Don't bother logging an arbitrary subset of the compiletest config Running `./x --verbose` will still print out the command-line arguments, and setting `RUST_LOG=compiletest` will now log the full config instead of a subset. --- src/tools/compiletest/src/bin/main.rs | 3 +- src/tools/compiletest/src/lib.rs | 48 ++------------------------- 2 files changed, 3 insertions(+), 48 deletions(-) diff --git a/src/tools/compiletest/src/bin/main.rs b/src/tools/compiletest/src/bin/main.rs index 1f777e71cf97f..8fac6ccdc582b 100644 --- a/src/tools/compiletest/src/bin/main.rs +++ b/src/tools/compiletest/src/bin/main.rs @@ -2,7 +2,7 @@ use std::env; use std::io::IsTerminal; use std::sync::Arc; -use compiletest::{early_config_check, log_config, parse_config, run_tests}; +use compiletest::{early_config_check, parse_config, run_tests}; fn main() { tracing_subscriber::fmt::init(); @@ -19,6 +19,5 @@ fn main() { early_config_check(&config); - log_config(&config); run_tests(config); } diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 58f7c6b50718a..91ab40edc6028 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -44,7 +44,6 @@ use crate::common::{ }; use crate::directives::DirectivesCache; use crate::executor::{CollectedTest, ColorConfig}; -use crate::util::logv; /// Creates the `Config` instance for this invocation of compiletest. /// @@ -477,51 +476,6 @@ pub fn parse_config(args: Vec) -> Config { } } -pub fn log_config(config: &Config) { - let c = config; - logv(c, "configuration:".to_string()); - logv(c, format!("compile_lib_path: {}", config.compile_lib_path)); - logv(c, format!("run_lib_path: {}", config.run_lib_path)); - logv(c, format!("rustc_path: {}", config.rustc_path)); - logv(c, format!("cargo_path: {:?}", config.cargo_path)); - logv(c, format!("rustdoc_path: {:?}", config.rustdoc_path)); - - logv(c, format!("src_root: {}", config.src_root)); - logv(c, format!("src_test_suite_root: {}", config.src_test_suite_root)); - - logv(c, format!("build_root: {}", config.build_root)); - logv(c, format!("build_test_suite_root: {}", config.build_test_suite_root)); - - logv(c, format!("sysroot_base: {}", config.sysroot_base)); - - logv(c, format!("stage: {}", config.stage)); - logv(c, format!("stage_id: {}", config.stage_id)); - logv(c, format!("mode: {}", config.mode)); - logv(c, format!("run_ignored: {}", config.run_ignored)); - logv(c, format!("filters: {:?}", config.filters)); - logv(c, format!("skip: {:?}", config.skip)); - logv(c, format!("filter_exact: {}", config.filter_exact)); - logv( - c, - format!("force_pass_mode: {}", opt_str(&config.force_pass_mode.map(|m| format!("{}", m))),), - ); - logv(c, format!("runner: {}", opt_str(&config.runner))); - logv(c, format!("host-rustcflags: {:?}", config.host_rustcflags)); - logv(c, format!("target-rustcflags: {:?}", config.target_rustcflags)); - logv(c, format!("target: {}", config.target)); - logv(c, format!("host: {}", config.host)); - logv(c, format!("android-cross-path: {}", config.android_cross_path)); - logv(c, format!("adb_path: {}", config.adb_path)); - logv(c, format!("adb_test_dir: {}", config.adb_test_dir)); - logv(c, format!("adb_device_status: {}", config.adb_device_status)); - logv(c, format!("ar: {}", config.ar)); - logv(c, format!("target-linker: {:?}", config.target_linker)); - logv(c, format!("host-linker: {:?}", config.host_linker)); - logv(c, format!("verbose: {}", config.verbose)); - logv(c, format!("minicore_path: {}", config.minicore_path)); - logv(c, "\n".to_string()); -} - pub fn opt_str(maybestr: &Option) -> &str { match *maybestr { None => "(none)", @@ -538,6 +492,8 @@ pub fn opt_str2(maybestr: Option) -> String { /// Called by `main` after the config has been parsed. pub fn run_tests(config: Arc) { + debug!(?config, "run_tests"); + // If we want to collect rustfix coverage information, // we first make sure that the coverage file does not exist. // It will be created later on. From 269db62491b43be6292af0fdabb84a1eb6c77c5d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 28 Aug 2025 14:15:33 +1000 Subject: [PATCH 2/4] Change the `logv` function into a `TestCx` method. When working on a new output-capture system, this will make it easier to obtain a capturing stream from the test context. --- src/tools/compiletest/src/runtest.rs | 18 +++++++++++++++--- src/tools/compiletest/src/runtest/debuginfo.rs | 3 +-- src/tools/compiletest/src/runtest/pretty.rs | 10 +++------- src/tools/compiletest/src/util.rs | 11 ----------- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 3a05b242519ef..aa463d7b25561 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -7,7 +7,7 @@ use std::io::prelude::*; use std::io::{self, BufReader}; use std::process::{Child, Command, ExitStatus, Output, Stdio}; use std::sync::Arc; -use std::{env, iter, str}; +use std::{env, fmt, iter, str}; use build_helper::fs::remove_and_create_dir_all; use camino::{Utf8Path, Utf8PathBuf}; @@ -25,7 +25,7 @@ use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff}; use crate::directives::TestProps; use crate::errors::{Error, ErrorKind, load_errors}; use crate::read2::{Truncated, read2_abbreviated}; -use crate::util::{Utf8PathBufExt, add_dylib_path, logv, static_regex}; +use crate::util::{Utf8PathBufExt, add_dylib_path, static_regex}; use crate::{ColorConfig, help, json, stamp_file_path, warning}; mod debugger; @@ -1459,7 +1459,7 @@ impl<'test> TestCx<'test> { ) -> ProcRes { let cmdline = { let cmdline = self.make_cmdline(&command, lib_path); - logv(self.config, format!("executing {}", cmdline)); + self.logv(format_args!("executing {cmdline}")); cmdline }; @@ -2006,6 +2006,18 @@ impl<'test> TestCx<'test> { output_base_name(self.config, self.testpaths, self.safe_revision()) } + /// Prints a message to (captured) stdout if `config.verbose` is true. + /// The message is also logged to `tracing::debug!` regardles of verbosity. + /// + /// Use `format_args!` as the argument to perform formatting if required. + fn logv(&self, message: impl fmt::Display) { + debug!("{message}"); + if self.config.verbose { + // Note: `./x test ... --verbose --no-capture` is needed to see this print. + println!("{message}"); + } + } + /// Prefix to print before error messages. Normally just `error`, but also /// includes the revision name for tests that use revisions. #[must_use] diff --git a/src/tools/compiletest/src/runtest/debuginfo.rs b/src/tools/compiletest/src/runtest/debuginfo.rs index 24fdbab3aec4f..88d022b8bbaaf 100644 --- a/src/tools/compiletest/src/runtest/debuginfo.rs +++ b/src/tools/compiletest/src/runtest/debuginfo.rs @@ -10,7 +10,6 @@ use super::debugger::DebuggerCommands; use super::{Debugger, Emit, ProcRes, TestCx, Truncated, WillExecute}; use crate::common::Config; use crate::debuggers::{extract_gdb_version, is_android_gdb_target}; -use crate::util::logv; impl TestCx<'_> { pub(super) fn run_debuginfo_test(&self) { @@ -234,7 +233,7 @@ impl TestCx<'_> { gdb.args(debugger_opts); // FIXME(jieyouxu): don't pass an empty Path let cmdline = self.make_cmdline(&gdb, Utf8Path::new("")); - logv(self.config, format!("executing {}", cmdline)); + self.logv(format_args!("executing {cmdline}")); cmdline }; diff --git a/src/tools/compiletest/src/runtest/pretty.rs b/src/tools/compiletest/src/runtest/pretty.rs index e3b07f1d63d1e..2655772723352 100644 --- a/src/tools/compiletest/src/runtest/pretty.rs +++ b/src/tools/compiletest/src/runtest/pretty.rs @@ -1,14 +1,13 @@ use std::fs; use super::{ProcRes, ReadFrom, TestCx}; -use crate::util::logv; impl TestCx<'_> { pub(super) fn run_pretty_test(&self) { if self.props.pp_exact.is_some() { - logv(self.config, "testing for exact pretty-printing".to_owned()); + self.logv("testing for exact pretty-printing"); } else { - logv(self.config, "testing for converging pretty-printing".to_owned()); + self.logv("testing for converging pretty-printing"); } let rounds = match self.props.pp_exact { @@ -21,10 +20,7 @@ impl TestCx<'_> { let mut round = 0; while round < rounds { - logv( - self.config, - format!("pretty-printing round {} revision {:?}", round, self.revision), - ); + self.logv(format_args!("pretty-printing round {round} revision {:?}", self.revision)); let read_from = if round == 0 { ReadFrom::Path } else { ReadFrom::Stdin(srcs[round].to_owned()) }; diff --git a/src/tools/compiletest/src/util.rs b/src/tools/compiletest/src/util.rs index fb047548c456a..1f16a672a98e7 100644 --- a/src/tools/compiletest/src/util.rs +++ b/src/tools/compiletest/src/util.rs @@ -2,9 +2,6 @@ use std::env; use std::process::Command; use camino::{Utf8Path, Utf8PathBuf}; -use tracing::*; - -use crate::common::Config; #[cfg(test)] mod tests; @@ -26,14 +23,6 @@ fn path_div() -> &'static str { ";" } -pub fn logv(config: &Config, s: String) { - debug!("{}", s); - if config.verbose { - // Note: `./x test ... --verbose --no-capture` is needed to see this print. - println!("{}", s); - } -} - pub trait Utf8PathBufExt { /// Append an extension to the path, even if it already has one. fn with_extra_extension(&self, extension: &str) -> Utf8PathBuf; From c80cadee6424ac5ad8fe017c4ea0b18a5bff4b56 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 28 Aug 2025 14:24:13 +1000 Subject: [PATCH 3/4] Move module `compute_diff` into `compiletest::runtest` The code in this module is always called in the context of running an individual tests, and sometimes prints output that needs to be captured. Moving this module into `runtest` will make it easier to find and audit all of the print statements that need to be updated when overhauling output-capture. --- src/tools/compiletest/src/lib.rs | 1 - src/tools/compiletest/src/runtest.rs | 6 +++--- src/tools/compiletest/src/{ => runtest}/compute_diff.rs | 0 src/tools/compiletest/src/runtest/mir_opt.rs | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) rename src/tools/compiletest/src/{ => runtest}/compute_diff.rs (100%) diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 91ab40edc6028..8737fec80bb39 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -9,7 +9,6 @@ mod tests; pub mod common; -pub mod compute_diff; mod debuggers; pub mod diagnostics; pub mod directives; diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index aa463d7b25561..eea471bc6cbd1 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -21,15 +21,13 @@ use crate::common::{ UI_WINDOWS_SVG, expected_output_path, incremental_dir, output_base_dir, output_base_name, output_testname_unique, }; -use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff}; use crate::directives::TestProps; use crate::errors::{Error, ErrorKind, load_errors}; use crate::read2::{Truncated, read2_abbreviated}; +use crate::runtest::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff}; use crate::util::{Utf8PathBufExt, add_dylib_path, static_regex}; use crate::{ColorConfig, help, json, stamp_file_path, warning}; -mod debugger; - // Helper modules that implement test running logic for each test suite. // tidy-alphabetical-start mod assembly; @@ -48,6 +46,8 @@ mod rustdoc_json; mod ui; // tidy-alphabetical-end +mod compute_diff; +mod debugger; #[cfg(test)] mod tests; diff --git a/src/tools/compiletest/src/compute_diff.rs b/src/tools/compiletest/src/runtest/compute_diff.rs similarity index 100% rename from src/tools/compiletest/src/compute_diff.rs rename to src/tools/compiletest/src/runtest/compute_diff.rs diff --git a/src/tools/compiletest/src/runtest/mir_opt.rs b/src/tools/compiletest/src/runtest/mir_opt.rs index efdb131bf14a8..55043bf4bc26d 100644 --- a/src/tools/compiletest/src/runtest/mir_opt.rs +++ b/src/tools/compiletest/src/runtest/mir_opt.rs @@ -6,7 +6,7 @@ use miropt_test_tools::{MiroptTest, MiroptTestFile, files_for_miropt_test}; use tracing::debug; use super::{Emit, TestCx, WillExecute}; -use crate::compute_diff::write_diff; +use crate::runtest::compute_diff::write_diff; impl TestCx<'_> { pub(super) fn run_mir_opt_test(&self) { From 6340b97768cc3803d96de92c8571d49f79f3e2ed Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 29 Jul 2025 12:37:30 +1000 Subject: [PATCH 4/4] Don't print captures in `TestCx::normalize_platform_differences` This appears to have been leftover debugging code. If the capture information turns out to have still been useful, we can find a way to emit it in a way that doesn't interfere with overhauling compiletests's output capture system. --- src/tools/compiletest/src/runtest.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index eea471bc6cbd1..867624cc8fa97 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2678,8 +2678,8 @@ impl<'test> TestCx<'test> { // // It's not possible to detect paths in the error messages generally, but this is a // decent enough heuristic. - static_regex!( - r#"(?x) + let re = static_regex!( + r#"(?x) (?: # Match paths that don't include spaces. (?:\\[\pL\pN\.\-_']+)+\.\pL+ @@ -2687,11 +2687,8 @@ impl<'test> TestCx<'test> { # If the path starts with a well-known root, then allow spaces and no file extension. \$(?:DIR|SRC_DIR|TEST_BUILD_DIR|BUILD_DIR|LIB_DIR)(?:\\[\pL\pN\.\-_'\ ]+)+ )"# - ) - .replace_all(&output, |caps: &Captures<'_>| { - println!("{}", &caps[0]); - caps[0].replace(r"\", "/") - }) + ); + re.replace_all(&output, |caps: &Captures<'_>| caps[0].replace(r"\", "/")) .replace("\r\n", "\n") }