refactor: improve test logging structure

Signed-off-by: xizheyin <[email protected]>

Author: xizheyin

bootstrap/src/clean.rs

@@ -5,11 +5,23 @@ use crate::Run;
 
 /// Clean the build directory
 #[derive(Args, Debug)]
-pub struct CleanCommand {}
+pub struct CleanCommand {
+    #[arg(short, long)]
+    pub verbose: bool,
+}
 
 impl Run for CleanCommand {
+    const STEP_DISPLAY_NAME: &'static str = "clean";
+
     fn run(&self, manifest: &Manifest) {
+        self.log_action_start("cleaning", "build directory");
         let _ = std::fs::remove_dir_all("crates/target");
+        self.log_action_context("rm", "crates/target");
         let _ = std::fs::remove_dir_all(&manifest.out_dir);
+        self.log_action_context("rm", &manifest.out_dir.display());
+    }
+
+    fn verbose(&self) -> bool {
+        self.verbose
     }
 }

bootstrap/src/fmt.rs

@@ -10,9 +10,14 @@ use crate::Run;
 pub struct FmtCommand {
     #[arg(short, long)]
     pub check: bool,
+
+    #[arg(short, long)]
+    pub verbose: bool,
 }
 
 impl Run for FmtCommand {
+    const STEP_DISPLAY_NAME: &'static str = "FMT";
+
     fn run(&self, _manifest: &crate::manifest::Manifest) {
         self.perform(
             Command::new("cargo").arg("fmt").args(["--manifest-path", "bootstrap/Cargo.toml"]),
@@ -30,14 +35,18 @@ impl Run for FmtCommand {
             self.perform(Command::new("rustfmt").args(["--edition", "2021"]).arg(file.unwrap()));
         }
     }
+
+    fn verbose(&self) -> bool {
+        self.verbose
+    }
 }
 
 impl FmtCommand {
     pub fn perform(&self, command: &mut Command) {
         if self.check {
             command.arg("--check");
         }
-        log::debug!("running {:?}", command);
-        assert!(command.status().unwrap().success(), "failed to run {:?}", command);
+
+        self.command_status("format code", command);
     }
 }

bootstrap/src/log.rs

@@ -1,10 +1,13 @@
+use std::fmt::Display;
+use std::process::{self, ExitStatus};
+
 use clap::{Parser, Subcommand};
+use color_print::cprintln;
 
 use crate::manifest::Manifest;
 
 mod clean;
 mod fmt;
-mod log;
 mod manifest;
 mod rustc;
 mod test;
@@ -38,7 +41,65 @@ pub enum Command {
 }
 
 trait Run {
+    // The name like "BUILD" or "TEST" for logs.
+    const STEP_DISPLAY_NAME: &'static str;
     fn run(&self, manifest: &Manifest);
+
+    /// True if verbose output should be enabled.
+    fn verbose(&self) -> bool;
+
+    /// Record that the step has started a new action.
+    fn log_action_start(&self, action: &str, item: impl Display) {
+        let name = Self::STEP_DISPLAY_NAME;
+        cprintln!("<b>[{name}]</b> {action} <cyan>{item}</cyan>");
+    }
+
+    /// Record context associated with the current action. Only use if there has been a preceding
+    /// call to `log_action_start`.
+    fn log_action_context(&self, key: impl Display, value: impl Display) {
+        if self.verbose() {
+            cprintln!("       {key}: {value}");
+        }
+    }
+
+    /// Run a command and ensure it succeeds, capturing output.
+    fn command_output(&self, action: &str, command: &mut process::Command) -> process::Output {
+        if self.verbose() {
+            cprintln!("       {action}: {command:?}");
+        }
+
+        match command.output() {
+            // Command ran and completed successfully
+            Ok(output) if output.status.success() => {
+                if self.verbose() {
+                    cprintln!("       <g>success</g>");
+                }
+                output
+            }
+            // Command ran but did not complete
+            Ok(output) => panic!("command failed: {output:?}"),
+            Err(e) => panic!("command failed: {e:?}"),
+        }
+    }
+
+    /// Run a command and ensure it succeeds.
+    fn command_status(&self, action: &str, command: &mut process::Command) -> ExitStatus {
+        if self.verbose() {
+            cprintln!("       {}: {}", action, format!("{:?}", command).replace('"', ""));
+        }
+        match command.status() {
+            // Command ran and completed successfully
+            Ok(status) if status.success() => {
+                if self.verbose() {
+                    cprintln!("       <g>success</g>");
+                }
+                status
+            }
+            // Command ran but did not complete
+            Ok(status) => panic!("command failed: {status:?}"),
+            Err(e) => panic!("command failed: {e:?}"),
+        }
+    }
 }
 
 fn main() {
@@ -56,11 +117,17 @@ fn main() {
             test.verbose |= cli.verbose;
             test.run(&manifest)
         }
-        Command::Clean(clean) => clean.run(&manifest),
+        Command::Clean(mut clean) => {
+            clean.verbose |= cli.verbose;
+            clean.run(&manifest)
+        }
         Command::Rustc(mut rustc) => {
             rustc.verbose |= cli.verbose;
             rustc.run(&manifest)
         }
-        Command::Fmt(fmt) => fmt.run(&manifest),
+        Command::Fmt(mut fmt) => {
+            fmt.verbose |= cli.verbose;
+            fmt.run(&manifest)
+        }
     }
 }

bootstrap/src/main.rs

@@ -1,9 +1,10 @@
-use anstream::eprintln as println;
-use color_print::cprintln;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
-use crate::log::Log;
+use anstream::eprintln as println;
+use color_print::cprintln;
+
+use crate::Run;
 
 #[derive(Debug)]
 pub struct Manifest {
@@ -12,111 +13,95 @@ pub struct Manifest {
     pub out_dir: PathBuf,
 }
 
-impl Log for Manifest {
-    fn log_step(&self, step_type: &str, name: &str, details: Vec<(&str, &str)>) {
-        if self.verbose {
-            cprintln!("<b>[BUILD]</b> {} {}", step_type, name);
-            for (label, value) in details {
-                cprintln!("       {}: {}", label, value);
-            }
+impl Manifest {
+    /// Builds the rustc codegen c library
+    pub fn prepare(&self) {
+        let prepare = PrepareAction { verbose: self.verbose };
+        prepare.run(&self);
+    }
+
+    /// The path to the rustc codegen c library
+    pub fn codegen_backend(&self) -> &'static Path {
+        if self.release {
+            Path::new("crates/target/release/librustc_codegen_c.so")
         } else {
-            cprintln!("<b>[BUILD]</b> {} {}", step_type, name);
+            Path::new("crates/target/debug/librustc_codegen_c.so")
         }
     }
 
-    fn is_verbose(&self) -> bool {
-        self.verbose
+    /// The command to run rustc with the codegen backend
+    pub fn rustc(&self) -> Command {
+        let mut command = Command::new("rustc");
+        command
+            .args(["--edition", "2021"])
+            .arg("-Z")
+            .arg(format!("codegen-backend={}", self.codegen_backend().display()))
+            .args(["-C", "panic=abort"])
+            .args(["-C", "lto=false"])
+            .arg(format!("-Lall={}", self.out_dir.display()))
+            .env("CFLAGS", "-Irust_runtime")
+            .arg("-lc")
+            .arg("-lrust_runtime");
+        if self.verbose {
+            command.env("RUST_BACKTRACE", "full");
+        }
+        command
     }
 }
 
-impl Manifest {
-    /// Builds the rustc codegen c library
-    pub fn prepare(&self) {
-        // New step
-        // Build codegen backend
-        self.log_step(
-            "preparing",
-            "codegen backend",
-            vec![("target", &self.codegen_backend().display().to_string())],
-        );
+struct PrepareAction {
+    verbose: bool,
+}
+
+impl Run for PrepareAction {
+    const STEP_DISPLAY_NAME: &'static str = "prepare";
+
+    fn run(&self, manifest: &Manifest) {
+        // action: Build codegen backend
+        self.log_action_start("building", "codegen backend");
+        self.log_action_context("target", manifest.codegen_backend().display());
 
         let mut command = Command::new("cargo");
         command.arg("build").args(["--manifest-path", "crates/Cargo.toml"]);
-        if self.verbose {
+        if manifest.verbose {
             command.args(["-v"]);
         }
-        if self.release {
+        if manifest.release {
             command.arg("--release");
         }
-        let status = command.status().unwrap();
-        let status = if self.verbose { Some(status) } else { None };
-        self.log_command("command", &command, &status);
-
-        // New step
-        // Build runtime library
-        self.log_step(
-            "librust_runtime",
-            "librust_runtime",
-            vec![("output", &self.out_dir.display().to_string())],
-        );
+        self.command_status("build", &mut command);
+
+        // action: Build runtime library
+        self.log_action_start("building", "librust_runtime");
+        self.log_action_context("output dir", &manifest.out_dir.to_path_buf().display());
 
         // cmd: Create output directory
-        match std::fs::create_dir_all(&self.out_dir) {
-            Ok(_) => (),
-            Err(e) => {
-                cprintln!("       <r>failed</r> to create output directory: {}", e);
-                std::process::exit(1);
-            }
+        if let Err(e) = std::fs::create_dir_all(&manifest.out_dir) {
+            cprintln!("       <r>failed</r> to create output directory: {}", e);
+            std::process::exit(1);
         }
+
         let cc = std::env::var("CC").unwrap_or("clang".to_string());
 
         // cmd: Compile runtime.c
         let mut command = Command::new(&cc);
         command
             .arg("rust_runtime/rust_runtime.c")
             .arg("-o")
-            .arg(self.out_dir.join("rust_runtime.o"))
+            .arg(manifest.out_dir.join("rust_runtime.o"))
             .arg("-c");
-        let status = command.status().unwrap();
-        let status = if self.verbose { Some(status) } else { None };
-        self.log_command("compile", &command, &status);
+        self.command_status("build", &mut command);
 
         // cmd: Create static library
         let mut command = Command::new("ar");
         command
             .arg("rcs")
-            .arg(self.out_dir.join("librust_runtime.a"))
-            .arg(self.out_dir.join("rust_runtime.o"));
-        let status = command.status().unwrap();
-        let status = if self.verbose { Some(status) } else { None };
-        self.log_command("archive", &command, &status);
-    }
-
-    /// The path to the rustc codegen c library
-    pub fn codegen_backend(&self) -> &'static Path {
-        if self.release {
-            Path::new("crates/target/release/librustc_codegen_c.so")
-        } else {
-            Path::new("crates/target/debug/librustc_codegen_c.so")
-        }
+            .arg(manifest.out_dir.join("librust_runtime.a"))
+            .arg(manifest.out_dir.join("rust_runtime.o"));
+        self.command_status("archive", &mut command);
     }
 
-    /// The command to run rustc with the codegen backend
-    pub fn rustc(&self) -> Command {
-        let mut command = Command::new("rustc");
-        command
-            .args(["--edition", "2021"])
-            .arg("-Z")
-            .arg(format!("codegen-backend={}", self.codegen_backend().display()))
-            .args(["-C", "panic=abort"])
-            .args(["-C", "lto=false"])
-            .arg(format!("-Lall={}", self.out_dir.display()))
-            .env("CFLAGS", "-Irust_runtime")
-            .arg("-lc")
-            .arg("-lrust_runtime");
-        if self.verbose {
-            command.env("RUST_BACKTRACE", "full");
-        }
-        command
+    fn verbose(&self) -> bool {
+        self.verbose
     }
 }