Rollup merge of #145982 - Zalathar:logv, r=jieyouxu

compiletest: Reduce the number of `println!` calls that don't have access to `TestCx`

In order to stop using `#![feature(internal_output_capture)]` in compiletest, we need to be able to capture the console output of individual tests run by the executor.

The approach I have planned is to have all test runners print “console” output into a trait object that is passed around as part of `TestCx`, since almost all test-runner code has easy access to that context. So `println!("foo")` will become `writeln!(self.stdout, "foo")`, and so on.

In order to make that viable, we need to avoid unnecessary printing in places that don't have easy access to `TestCx`. To do so, we can either get rid of unnecessary print statements, or rearrange the code to make the context available. This PR uses both approaches.

r? jieyouxu

Author: matthiaskrgr

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);
 }

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;
@@ -44,7 +43,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 +475,6 @@ pub fn parse_config(args: Vec<String>) -> 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<String>) -> &str {
     match *maybestr {
         None => "(none)",
@@ -538,6 +491,8 @@ pub fn opt_str2(maybestr: Option<String>) -> String {
 
 /// Called by `main` after the config has been parsed.
 pub fn run_tests(config: Arc<Config>) {
+    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.

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};
@@ -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::util::{Utf8PathBufExt, add_dylib_path, logv, static_regex};
+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;
 
@@ -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]
@@ -2666,20 +2678,17 @@ 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+
                 |
                   # 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")
     }
 

src/tools/compiletest/src/compute_diff.rs renamed to src/tools/compiletest/src/runtest/compute_diff.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
             };
 

src/tools/compiletest/src/runtest/debuginfo.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) {

src/tools/compiletest/src/runtest/mir_opt.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()) };
 

src/tools/compiletest/src/runtest/pretty.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;