[COMPILETEST-UNTANGLE 2/N] Make some compiletest errors/warnings/help more visually obvious

src/tools/compiletest/src/common.rs

@@ -11,6 +11,7 @@ use serde::de::{Deserialize, Deserializer, Error as _};
 
 pub use self::Mode::*;
 use crate::executor::{ColorConfig, OutputFormat};
+use crate::fatal;
 use crate::util::{Utf8PathBufExt, add_dylib_path};
 
 macro_rules! string_enum {
@@ -783,11 +784,13 @@ fn rustc_output(config: &Config, args: &[&str], envs: HashMap<String, String>) -
 
     let output = match command.output() {
         Ok(output) => output,
-        Err(e) => panic!("error: failed to run {command:?}: {e}"),
+        Err(e) => {
+            fatal!("failed to run {command:?}: {e}");
+        }
     };
     if !output.status.success() {
-        panic!(
-            "error: failed to run {command:?}\n--- stdout\n{}\n--- stderr\n{}",
+        fatal!(
+            "failed to run {command:?}\n--- stdout\n{}\n--- stderr\n{}",
             String::from_utf8(output.stdout).unwrap(),
             String::from_utf8(output.stderr).unwrap(),
         );

src/tools/compiletest/src/diagnostics.rs

@@ -0,0 +1,46 @@
+//! Collection of diagnostics helpers for `compiletest` *itself*.
+
+#[macro_export]
+macro_rules! fatal {
+    ($($arg:tt)*) => {
+        let status = ::colored::Colorize::bright_red("FATAL: ");
+        let status = ::colored::Colorize::bold(status);
+        eprint!("{status}");
+        eprintln!($($arg)*);
+        // This intentionally uses a seemingly-redundant panic to include backtrace location.
+        //
+        // FIXME: in the long term, we should handle "logic bug in compiletest itself" vs "fatal
+        // user error" separately.
+        panic!("fatal error");
+    };
+}
+
+#[macro_export]
+macro_rules! error {
+    ($($arg:tt)*) => {
+        let status = ::colored::Colorize::red("ERROR: ");
+        let status = ::colored::Colorize::bold(status);
+        eprint!("{status}");
+        eprintln!($($arg)*);
+    };
+}
+
+#[macro_export]
+macro_rules! warning {
+    ($($arg:tt)*) => {
+        let status = ::colored::Colorize::yellow("WARNING: ");
+        let status = ::colored::Colorize::bold(status);
+        eprint!("{status}");
+        eprintln!($($arg)*);
+    };
+}
+
+#[macro_export]
+macro_rules! help {
+    ($($arg:tt)*) => {
+        let status = ::colored::Colorize::cyan("HELP: ");
+        let status = ::colored::Colorize::bold(status);
+        eprint!("{status}");
+        eprintln!($($arg)*);
+    };
+}

src/tools/compiletest/src/header.rs

@@ -15,6 +15,7 @@ use crate::errors::ErrorKind;
 use crate::executor::{CollectedTestDesc, ShouldPanic};
 use crate::header::auxiliary::{AuxProps, parse_and_update_aux};
 use crate::header::needs::CachedNeedsConditions;
+use crate::help;
 use crate::util::static_regex;
 
 pub(crate) mod auxiliary;
@@ -920,9 +921,9 @@ fn iter_header(
             if !is_known_directive {
                 *poisoned = true;
 
-                eprintln!(
-                    "error: detected unknown compiletest test directive `{}` in {}:{}",
-                    directive_line.raw_directive, testfile, line_number,
+                error!(
+                    "{testfile}:{line_number}: detected unknown compiletest test directive `{}`",
+                    directive_line.raw_directive,
                 );
 
                 return;
@@ -931,11 +932,11 @@ fn iter_header(
             if let Some(trailing_directive) = &trailing_directive {
                 *poisoned = true;
 
-                eprintln!(
-                    "error: detected trailing compiletest test directive `{}` in {}:{}\n \
-                      help: put the trailing directive in it's own line: `//@ {}`",
-                    trailing_directive, testfile, line_number, trailing_directive,
+                error!(
+                    "{testfile}:{line_number}: detected trailing compiletest test directive `{}`",
+                    trailing_directive,
                 );
+                help!("put the trailing directive in it's own line: `//@ {}`", trailing_directive);
 
                 return;
             }
@@ -1031,10 +1032,9 @@ impl Config {
         };
 
         let Some((regex, replacement)) = parse_normalize_rule(raw_value) else {
-            panic!(
-                "couldn't parse custom normalization rule: `{raw_directive}`\n\
-                help: expected syntax is: `{directive_name}: \"REGEX\" -> \"REPLACEMENT\"`"
-            );
+            error!("couldn't parse custom normalization rule: `{raw_directive}`");
+            help!("expected syntax is: `{directive_name}: \"REGEX\" -> \"REPLACEMENT\"`");
+            panic!("invalid normalization rule detected");
         };
         Some(NormalizeRule { kind, regex, replacement })
     }
@@ -1406,7 +1406,7 @@ pub(crate) fn make_test_description<R: Read>(
                             ignore_message = Some(reason.into());
                         }
                         IgnoreDecision::Error { message } => {
-                            eprintln!("error: {}:{line_number}: {message}", path);
+                            error!("{path}:{line_number}: {message}");
                             *poisoned = true;
                             return;
                         }

src/tools/compiletest/src/lib.rs

@@ -11,6 +11,7 @@ mod tests;
 pub mod common;
 pub mod compute_diff;
 mod debuggers;
+pub mod diagnostics;
 pub mod errors;
 mod executor;
 pub mod header;
@@ -33,7 +34,7 @@ use build_helper::git::{get_git_modified_files, get_git_untracked_files};
 use camino::{Utf8Path, Utf8PathBuf};
 use getopts::Options;
 use rayon::iter::{ParallelBridge, ParallelIterator};
-use tracing::*;
+use tracing::debug;
 use walkdir::WalkDir;
 
 use self::header::{EarlyProps, make_test_description};
@@ -651,10 +652,7 @@ pub(crate) fn collect_and_make_tests(config: Arc<Config>) -> Vec<CollectedTest>
     let common_inputs_stamp = common_inputs_stamp(&config);
     let modified_tests =
         modified_tests(&config, &config.src_test_suite_root).unwrap_or_else(|err| {
-            panic!(
-                "modified_tests got error from dir: {}, error: {}",
-                config.src_test_suite_root, err
-            )
+            fatal!("modified_tests: {}: {err}", config.src_test_suite_root);
         });
     let cache = HeadersCache::load(&config);
 
@@ -1108,22 +1106,17 @@ fn check_for_overlapping_test_paths(found_path_stems: &HashSet<Utf8PathBuf>) {
 
 pub fn early_config_check(config: &Config) {
     if !config.has_html_tidy && config.mode == Mode::Rustdoc {
-        eprintln!("warning: `tidy` (html-tidy.org) is not installed; diffs will not be generated");
+        warning!("`tidy` (html-tidy.org) is not installed; diffs will not be generated");
     }
 
     if !config.profiler_runtime && config.mode == Mode::CoverageRun {
         let actioned = if config.bless { "blessed" } else { "checked" };
-        eprintln!(
-            r#"
-WARNING: profiler runtime is not available, so `.coverage` files won't be {actioned}
-help: try setting `profiler = true` in the `[build]` section of `bootstrap.toml`"#
-        );
+        warning!("profiler runtime is not available, so `.coverage` files won't be {actioned}");
+        help!("try setting `profiler = true` in the `[build]` section of `bootstrap.toml`");
     }
 
     // `RUST_TEST_NOCAPTURE` is a libtest env var, but we don't callout to libtest.
     if env::var("RUST_TEST_NOCAPTURE").is_ok() {
-        eprintln!(
-            "WARNING: RUST_TEST_NOCAPTURE is not supported. Use the `--no-capture` flag instead."
-        );
+        warning!("`RUST_TEST_NOCAPTURE` is not supported; use the `--no-capture` flag instead");
     }
 }

src/tools/compiletest/src/runtest.rs

@@ -27,7 +27,7 @@ use crate::errors::{Error, ErrorKind, load_errors};
 use crate::header::TestProps;
 use crate::read2::{Truncated, read2_abbreviated};
 use crate::util::{Utf8PathBufExt, add_dylib_path, logv, static_regex};
-use crate::{ColorConfig, json, stamp_file_path};
+use crate::{ColorConfig, help, json, stamp_file_path, warning};
 
 mod debugger;
 
@@ -485,12 +485,15 @@ impl<'test> TestCx<'test> {
                 .windows(2)
                 .any(|args| args == cfg_arg || args[0] == arg || args[1] == arg)
             {
-                panic!(
-                    "error: redundant cfg argument `{normalized_revision}` is already created by the revision"
+                error!(
+                    "redundant cfg argument `{normalized_revision}` is already created by the \
+                    revision"
                 );
+                panic!("redundant cfg argument");
             }
             if self.config.builtin_cfg_names().contains(&normalized_revision) {
-                panic!("error: revision `{normalized_revision}` collides with a builtin cfg");
+                error!("revision `{normalized_revision}` collides with a built-in cfg");
+                panic!("revision collides with built-in cfg");
             }
             cmd.args(cfg_arg);
         }
@@ -2167,9 +2170,10 @@ impl<'test> TestCx<'test> {
             println!("{}", String::from_utf8_lossy(&output.stdout));
             eprintln!("{}", String::from_utf8_lossy(&output.stderr));
         } else {
-            eprintln!("warning: no pager configured, falling back to unified diff");
-            eprintln!(
-                "help: try configuring a git pager (e.g. `delta`) with `git config --global core.pager delta`"
+            warning!("no pager configured, falling back to unified diff");
+            help!(
+                "try configuring a git pager (e.g. `delta`) with \
+                `git config --global core.pager delta`"
             );
             let mut out = io::stdout();
             let mut diff = BufReader::new(File::open(&diff_filename).unwrap());

src/tools/compiletest/src/runtest/codegen_units.rs

@@ -1,8 +1,8 @@
 use std::collections::HashSet;
 
 use super::{Emit, TestCx, WillExecute};
-use crate::errors;
 use crate::util::static_regex;
+use crate::{errors, fatal};
 
 impl TestCx<'_> {
     pub(super) fn run_codegen_units_test(&self) {
@@ -100,7 +100,7 @@ impl TestCx<'_> {
         }
 
         if !(missing.is_empty() && unexpected.is_empty() && wrong_cgus.is_empty()) {
-            panic!();
+            fatal!("!(missing.is_empty() && unexpected.is_empty() && wrong_cgus.is_empty())");
         }
 
         #[derive(Clone, Eq, PartialEq)]