From 046451b1201e162e715d526e3187ea2061ec0620 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Fri, 22 Mar 2024 17:56:56 +0100
Subject: [PATCH 01/11] Add test `test_logfile_format()`.

---
 Cargo.lock                |  1 +
 library/test/Cargo.toml   |  3 ++
 library/test/src/tests.rs | 73 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/Cargo.lock b/Cargo.lock
index 87833e4edbd78..6a467eba9882e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -5515,6 +5515,7 @@ dependencies = [
  "libc",
  "panic_abort",
  "panic_unwind",
+ "rand",
  "std",
 ]
 
diff --git a/library/test/Cargo.toml b/library/test/Cargo.toml
index 4c42e3bae563e..afbd882559fba 100644
--- a/library/test/Cargo.toml
+++ b/library/test/Cargo.toml
@@ -10,3 +10,6 @@ core = { path = "../core" }
 panic_unwind = { path = "../panic_unwind" }
 panic_abort = { path = "../panic_abort" }
 libc = { version = "0.2.150", default-features = false }
+
+[dev-dependencies]
+rand = { version = "0.8.5" }
diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs
index 43a906ad298d1..e9bb05c8efc61 100644
--- a/library/test/src/tests.rs
+++ b/library/test/src/tests.rs
@@ -1,3 +1,7 @@
+use rand::RngCore;
+use std::fs;
+use std::path::PathBuf;
+
 use super::*;
 
 use crate::{
@@ -39,6 +43,36 @@ impl TestOpts {
     }
 }
 
+// These implementations of TempDir and tmpdir are forked from rust/library/std/src/sys_common/io.rs.
+struct TempDir(PathBuf);
+
+impl TempDir {
+    fn join(&self, path: &str) -> PathBuf {
+        let TempDir(ref p) = *self;
+        p.join(path)
+    }
+}
+
+impl Drop for TempDir {
+    fn drop(&mut self) {
+        let TempDir(ref p) = *self;
+        let result = fs::remove_dir_all(p);
+        // Avoid panicking while panicking as this causes the process to
+        // immediately abort, without displaying test results.
+        if !thread::panicking() {
+            result.unwrap();
+        }
+    }
+}
+
+fn tmpdir() -> TempDir {
+    let p = env::temp_dir();
+    let mut r = rand::thread_rng();
+    let ret = p.join(&format!("rust-{}", r.next_u32()));
+    fs::create_dir(&ret).unwrap();
+    TempDir(ret)
+}
+
 fn one_ignored_one_unignored_test() -> Vec<TestDescAndFn> {
     vec![
         TestDescAndFn {
@@ -922,3 +956,42 @@ fn test_dyn_bench_returning_err_fails_when_run_as_test() {
     let result = rx.recv().unwrap().result;
     assert_eq!(result, TrFailed);
 }
+
+#[test]
+fn test_logfile_format() {
+    let desc = TestDescAndFn {
+        desc: TestDesc {
+            name: StaticTestName("whatever"),
+            ignore: false,
+            ignore_message: None,
+            source_file: "",
+            start_line: 0,
+            start_col: 0,
+            end_line: 0,
+            end_col: 0,
+            should_panic: ShouldPanic::No,
+            compile_fail: false,
+            no_run: false,
+            test_type: TestType::Unknown,
+        },
+        testfn: DynTestFn(Box::new(move || Ok(()))),
+    };
+
+    let tmpdir = tmpdir();
+    let output_path = &tmpdir.join("output.txt");
+
+    let opts = TestOpts {
+        run_tests: true,
+        logfile: Some(output_path.clone()),
+        format: OutputFormat::Pretty,
+        ..TestOpts::new()
+    };
+    run_tests_console(&opts, vec![desc]).unwrap();
+
+    let contents = fs::read_to_string(output_path).expect("`--logfile` did not create file");
+
+    // Split output at line breaks to make the comparison platform-agnostic regarding newline style.
+    let contents_lines = contents.as_str().lines().collect::<Vec<&str>>();
+
+    assert_eq!(contents_lines, vec!["ok whatever"]);
+}

From 2c2d594afe4e244e10faf6120d48e7a3a48413db Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 17:37:59 +0100
Subject: [PATCH 02/11] Add test `test_discovery_logfile_format()`.

---
 library/test/src/tests.rs | 41 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs
index e9bb05c8efc61..3ff64b6ea594e 100644
--- a/library/test/src/tests.rs
+++ b/library/test/src/tests.rs
@@ -5,6 +5,7 @@ use std::path::PathBuf;
 use super::*;
 
 use crate::{
+    console::list_tests_console,
     console::OutputLocation,
     formatters::PrettyFormatter,
     test::{
@@ -957,6 +958,46 @@ fn test_dyn_bench_returning_err_fails_when_run_as_test() {
     assert_eq!(result, TrFailed);
 }
 
+#[test]
+fn test_discovery_logfile_format() {
+    let desc = TestDescAndFn {
+        desc: TestDesc {
+            name: StaticTestName("whatever"),
+            ignore: false,
+            ignore_message: None,
+            source_file: "",
+            start_line: 0,
+            start_col: 0,
+            end_line: 0,
+            end_col: 0,
+            should_panic: ShouldPanic::No,
+            compile_fail: false,
+            no_run: false,
+            test_type: TestType::Unknown,
+        },
+        testfn: DynTestFn(Box::new(move || Ok(()))),
+    };
+
+    let tmpdir = tmpdir();
+    let output_path = &tmpdir.join("output.txt");
+
+    let opts = TestOpts {
+        run_tests: true,
+        logfile: Some(output_path.clone()),
+        format: OutputFormat::Pretty,
+        list: true,
+        ..TestOpts::new()
+    };
+    list_tests_console(&opts, vec![desc]).unwrap();
+
+    let contents = fs::read_to_string(output_path).expect("`--logfile` did not create file");
+
+    // Split output at line breaks to make the comparison platform-agnostic regarding newline style.
+    let contents_lines = contents.as_str().lines().collect::<Vec<&str>>();
+
+    assert_eq!(contents_lines, vec!["test whatever"]);
+}
+
 #[test]
 fn test_logfile_format() {
     let desc = TestDescAndFn {

From 41e7fe32665a4f13e24e2b7e3aaba7f1cfbc5064 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 12:59:02 +0100
Subject: [PATCH 03/11] Decouple the logic for terminal coloring from
 PrettyFormatter.

PrettyFormatter doesn't need to check the OutputLocation enum value anymore. This improves the separation of concerns.
---
 library/test/src/console.rs           | 23 +++++++++++++++++++++++
 library/test/src/formatters/pretty.rs | 25 ++++---------------------
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index f3918ba333aa0..16dec65b3c98e 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -41,6 +41,29 @@ impl<T: Write> Write for OutputLocation<T> {
     }
 }
 
+impl<T: Write> OutputLocation<T> {
+    pub fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
+        match self {
+            OutputLocation::Pretty(ref mut term) => {
+                term.fg(color)?;
+                term.write_all(word.as_bytes())?;
+                term.reset()?;
+            }
+            OutputLocation::Raw(ref mut stdout) => {
+                stdout.write_all(word.as_bytes())?;
+            }
+        }
+
+        self.flush()
+    }
+
+    pub fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
+        let s = s.as_ref();
+        self.write_all(s.as_bytes())?;
+        self.flush()
+    }
+}
+
 pub struct ConsoleTestDiscoveryState {
     pub log_out: Option<File>,
     pub tests: usize,
diff --git a/library/test/src/formatters/pretty.rs b/library/test/src/formatters/pretty.rs
index 22654a3400b44..2abbea9a8d1c3 100644
--- a/library/test/src/formatters/pretty.rs
+++ b/library/test/src/formatters/pretty.rs
@@ -69,29 +69,12 @@ impl<T: Write> PrettyFormatter<T> {
         self.write_pretty(result, color)
     }
 
-    pub fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
-        match self.out {
-            OutputLocation::Pretty(ref mut term) => {
-                if self.use_color {
-                    term.fg(color)?;
-                }
-                term.write_all(word.as_bytes())?;
-                if self.use_color {
-                    term.reset()?;
-                }
-                term.flush()
-            }
-            OutputLocation::Raw(ref mut stdout) => {
-                stdout.write_all(word.as_bytes())?;
-                stdout.flush()
-            }
-        }
+    fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
+        if self.use_color { self.out.write_pretty(word, color) } else { self.out.write_plain(word) }
     }
 
-    pub fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
-        let s = s.as_ref();
-        self.out.write_all(s.as_bytes())?;
-        self.out.flush()
+    fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
+        self.out.write_plain(s)
     }
 
     fn write_time(

From 7e62f942ff2b54a088e14139f2781179c7a7be46 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 14:35:24 +0100
Subject: [PATCH 04/11] Simplify access to output results in unit test
 `should_sort_failures_before_printing_them`.

---
 library/test/src/formatters/pretty.rs | 5 -----
 library/test/src/tests.rs             | 8 +++-----
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/library/test/src/formatters/pretty.rs b/library/test/src/formatters/pretty.rs
index 2abbea9a8d1c3..6a99958557af2 100644
--- a/library/test/src/formatters/pretty.rs
+++ b/library/test/src/formatters/pretty.rs
@@ -32,11 +32,6 @@ impl<T: Write> PrettyFormatter<T> {
         PrettyFormatter { out, use_color, max_name_len, is_multithreaded, time_options }
     }
 
-    #[cfg(test)]
-    pub fn output_location(&self) -> &OutputLocation<T> {
-        &self.out
-    }
-
     pub fn write_ok(&mut self) -> io::Result<()> {
         self.write_short_result("ok", term::color::GREEN)
     }
diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs
index 3ff64b6ea594e..360f5af2806a9 100644
--- a/library/test/src/tests.rs
+++ b/library/test/src/tests.rs
@@ -893,7 +893,8 @@ fn should_sort_failures_before_printing_them() {
         test_type: TestType::Unknown,
     };
 
-    let mut out = PrettyFormatter::new(OutputLocation::Raw(Vec::new()), false, 10, false, None);
+    let mut output = Vec::new();
+    let mut out = PrettyFormatter::new(OutputLocation::Raw(&mut output), false, 10, false, None);
 
     let st = console::ConsoleTestState {
         log_out: None,
@@ -913,11 +914,8 @@ fn should_sort_failures_before_printing_them() {
     };
 
     out.write_failures(&st).unwrap();
-    let s = match out.output_location() {
-        &OutputLocation::Raw(ref m) => String::from_utf8_lossy(&m[..]),
-        &OutputLocation::Pretty(_) => unreachable!(),
-    };
 
+    let s = String::from_utf8_lossy(&output[..]);
     let apos = s.find("a").unwrap();
     let bpos = s.find("b").unwrap();
     assert!(apos < bpos);

From d6e3d19d2897c129f123a525727f03a2a6b36e37 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 14:38:39 +0100
Subject: [PATCH 05/11] Remove generic parameters from `Output` trait.

Rationale:

* In a next commit, we would store a trait reference in
  `PrettyFormatter<T>::out` instead of using a concrete type
  `OutputLocation<T>`.
* If the trait had generic methods, we would get compilation error
  "cannot be made into an object" in that next commit.
---
 library/test/src/console.rs           | 5 ++---
 library/test/src/formatters/pretty.rs | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index 16dec65b3c98e..29bb210bd2baa 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -57,9 +57,8 @@ impl<T: Write> OutputLocation<T> {
         self.flush()
     }
 
-    pub fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
-        let s = s.as_ref();
-        self.write_all(s.as_bytes())?;
+    pub fn write_plain(&mut self, word: &str) -> io::Result<()> {
+        self.write_all(word.as_bytes())?;
         self.flush()
     }
 }
diff --git a/library/test/src/formatters/pretty.rs b/library/test/src/formatters/pretty.rs
index 6a99958557af2..693f58010f5f3 100644
--- a/library/test/src/formatters/pretty.rs
+++ b/library/test/src/formatters/pretty.rs
@@ -69,6 +69,7 @@ impl<T: Write> PrettyFormatter<T> {
     }
 
     fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
+        let s = s.as_ref();
         self.out.write_plain(s)
     }
 

From 691e5e120f11807bee79e335453d31fd023a338e Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 15:36:54 +0100
Subject: [PATCH 06/11] Hide implementation details of `OutputLocation<T>` from
 PrettyFormatter.

---
 library/test/src/console.rs           | 19 ++++++++++++-------
 library/test/src/formatters/pretty.rs | 14 +++++++-------
 library/test/src/tests.rs             |  3 ++-
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index 29bb210bd2baa..00bb043c5818a 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -19,6 +19,11 @@ use super::{
     types::{NamePadding, TestDesc, TestDescAndFn},
 };
 
+pub trait Output {
+    fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()>;
+    fn write_plain(&mut self, word: &str) -> io::Result<()>;
+}
+
 /// Generic wrapper over stdout.
 pub enum OutputLocation<T> {
     Pretty(Box<term::StdoutTerminal>),
@@ -41,8 +46,8 @@ impl<T: Write> Write for OutputLocation<T> {
     }
 }
 
-impl<T: Write> OutputLocation<T> {
-    pub fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
+impl<T: Write> Output for OutputLocation<T> {
+    fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
         match self {
             OutputLocation::Pretty(ref mut term) => {
                 term.fg(color)?;
@@ -57,7 +62,7 @@ impl<T: Write> OutputLocation<T> {
         self.flush()
     }
 
-    pub fn write_plain(&mut self, word: &str) -> io::Result<()> {
+    fn write_plain(&mut self, word: &str) -> io::Result<()> {
         self.write_all(word.as_bytes())?;
         self.flush()
     }
@@ -193,14 +198,14 @@ impl ConsoleTestState {
 
 // List the tests to console, and optionally to logfile. Filters are honored.
 pub fn list_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Result<()> {
-    let output = match term::stdout() {
+    let mut output = match term::stdout() {
         None => OutputLocation::Raw(io::stdout().lock()),
         Some(t) => OutputLocation::Pretty(t),
     };
 
     let mut out: Box<dyn OutputFormatter> = match opts.format {
         OutputFormat::Pretty | OutputFormat::Junit => {
-            Box::new(PrettyFormatter::new(output, false, 0, false, None))
+            Box::new(PrettyFormatter::new(&mut output, false, 0, false, None))
         }
         OutputFormat::Terse => Box::new(TerseFormatter::new(output, false, 0, false)),
         OutputFormat::Json => Box::new(JsonFormatter::new(output)),
@@ -306,7 +311,7 @@ fn on_test_event(
 /// A simple console test runner.
 /// Runs provided tests reporting process and results to the stdout.
 pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Result<bool> {
-    let output = match term::stdout() {
+    let mut output = match term::stdout() {
         None => OutputLocation::Raw(io::stdout()),
         Some(t) => OutputLocation::Pretty(t),
     };
@@ -321,7 +326,7 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Resu
 
     let mut out: Box<dyn OutputFormatter> = match opts.format {
         OutputFormat::Pretty => Box::new(PrettyFormatter::new(
-            output,
+            &mut output,
             opts.use_color(),
             max_name_len,
             is_multithreaded,
diff --git a/library/test/src/formatters/pretty.rs b/library/test/src/formatters/pretty.rs
index 693f58010f5f3..9411d8d9523ae 100644
--- a/library/test/src/formatters/pretty.rs
+++ b/library/test/src/formatters/pretty.rs
@@ -1,17 +1,17 @@
-use std::{io, io::prelude::Write};
+use std::io;
 
 use super::OutputFormatter;
 use crate::{
     bench::fmt_bench_samples,
-    console::{ConsoleTestDiscoveryState, ConsoleTestState, OutputLocation},
+    console::{ConsoleTestDiscoveryState, ConsoleTestState, Output},
     term,
     test_result::TestResult,
     time,
     types::TestDesc,
 };
 
-pub(crate) struct PrettyFormatter<T> {
-    out: OutputLocation<T>,
+pub(crate) struct PrettyFormatter<'a> {
+    out: &'a mut dyn Output,
     use_color: bool,
     time_options: Option<time::TestTimeOptions>,
 
@@ -21,9 +21,9 @@ pub(crate) struct PrettyFormatter<T> {
     is_multithreaded: bool,
 }
 
-impl<T: Write> PrettyFormatter<T> {
+impl<'a> PrettyFormatter<'a> {
     pub fn new(
-        out: OutputLocation<T>,
+        out: &'a mut dyn Output,
         use_color: bool,
         max_name_len: usize,
         is_multithreaded: bool,
@@ -159,7 +159,7 @@ impl<T: Write> PrettyFormatter<T> {
     }
 }
 
-impl<T: Write> OutputFormatter for PrettyFormatter<T> {
+impl OutputFormatter for PrettyFormatter<'_> {
     fn write_discovery_start(&mut self) -> io::Result<()> {
         Ok(())
     }
diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs
index 360f5af2806a9..fc96a06549162 100644
--- a/library/test/src/tests.rs
+++ b/library/test/src/tests.rs
@@ -894,7 +894,8 @@ fn should_sort_failures_before_printing_them() {
     };
 
     let mut output = Vec::new();
-    let mut out = PrettyFormatter::new(OutputLocation::Raw(&mut output), false, 10, false, None);
+    let mut raw = OutputLocation::Raw(&mut output);
+    let mut out = PrettyFormatter::new(&mut raw, false, 10, false, None);
 
     let st = console::ConsoleTestState {
         log_out: None,

From 55200c88ee3b50edb321a12e7d4cfb560d2254a3 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 16:11:35 +0100
Subject: [PATCH 07/11] Hide implementation details of `OutputLocation<T>` from
 JsonFormatter.

---
 library/test/src/console.rs         |  4 ++--
 library/test/src/formatters/json.rs | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index 00bb043c5818a..bafa50f809a70 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -208,7 +208,7 @@ pub fn list_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Res
             Box::new(PrettyFormatter::new(&mut output, false, 0, false, None))
         }
         OutputFormat::Terse => Box::new(TerseFormatter::new(output, false, 0, false)),
-        OutputFormat::Json => Box::new(JsonFormatter::new(output)),
+        OutputFormat::Json => Box::new(JsonFormatter::new(&mut output)),
     };
     let mut st = ConsoleTestDiscoveryState::new(opts)?;
 
@@ -335,7 +335,7 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Resu
         OutputFormat::Terse => {
             Box::new(TerseFormatter::new(output, opts.use_color(), max_name_len, is_multithreaded))
         }
-        OutputFormat::Json => Box::new(JsonFormatter::new(output)),
+        OutputFormat::Json => Box::new(JsonFormatter::new(&mut output)),
         OutputFormat::Junit => Box::new(JunitFormatter::new(output)),
     };
     let mut st = ConsoleTestState::new(opts)?;
diff --git a/library/test/src/formatters/json.rs b/library/test/src/formatters/json.rs
index 47c4e7757e40c..88a4e013d267a 100644
--- a/library/test/src/formatters/json.rs
+++ b/library/test/src/formatters/json.rs
@@ -1,19 +1,19 @@
-use std::{borrow::Cow, io, io::prelude::Write};
+use std::{borrow::Cow, io};
 
 use super::OutputFormatter;
 use crate::{
-    console::{ConsoleTestDiscoveryState, ConsoleTestState, OutputLocation},
+    console::{ConsoleTestDiscoveryState, ConsoleTestState, Output},
     test_result::TestResult,
     time,
     types::TestDesc,
 };
 
-pub(crate) struct JsonFormatter<T> {
-    out: OutputLocation<T>,
+pub(crate) struct JsonFormatter<'a> {
+    out: &'a mut dyn Output,
 }
 
-impl<T: Write> JsonFormatter<T> {
-    pub fn new(out: OutputLocation<T>) -> Self {
+impl<'a> JsonFormatter<'a> {
+    pub fn new(out: &'a mut dyn Output) -> Self {
         Self { out }
     }
 
@@ -23,7 +23,7 @@ impl<T: Write> JsonFormatter<T> {
         // by issuing `write_all` calls line-by-line.
         assert_eq!(s.chars().last(), Some('\n'));
 
-        self.out.write_all(s.as_ref())
+        self.out.write_plain(s)
     }
 
     fn write_event(
@@ -56,7 +56,7 @@ impl<T: Write> JsonFormatter<T> {
     }
 }
 
-impl<T: Write> OutputFormatter for JsonFormatter<T> {
+impl OutputFormatter for JsonFormatter<'_> {
     fn write_discovery_start(&mut self) -> io::Result<()> {
         self.writeln_message(concat!(r#"{ "type": "suite", "event": "discovery" }"#, "\n"))
     }

From 0632c730eebb819a22437eb1c598e36c3c8395d4 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 16:21:56 +0100
Subject: [PATCH 08/11] Hide implementation details of `OutputLocation<T>` from
 JunitFormatter.

---
 library/test/src/console.rs          |  2 +-
 library/test/src/formatters/junit.rs | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index bafa50f809a70..f437fb0709cd2 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -336,7 +336,7 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Resu
             Box::new(TerseFormatter::new(output, opts.use_color(), max_name_len, is_multithreaded))
         }
         OutputFormat::Json => Box::new(JsonFormatter::new(&mut output)),
-        OutputFormat::Junit => Box::new(JunitFormatter::new(output)),
+        OutputFormat::Junit => Box::new(JunitFormatter::new(&mut output)),
     };
     let mut st = ConsoleTestState::new(opts)?;
 
diff --git a/library/test/src/formatters/junit.rs b/library/test/src/formatters/junit.rs
index a211ebf1ded16..5058342e79e1a 100644
--- a/library/test/src/formatters/junit.rs
+++ b/library/test/src/formatters/junit.rs
@@ -1,28 +1,28 @@
-use std::io::{self, prelude::Write};
+use std::io;
 use std::time::Duration;
 
 use super::OutputFormatter;
 use crate::{
-    console::{ConsoleTestDiscoveryState, ConsoleTestState, OutputLocation},
+    console::{ConsoleTestDiscoveryState, ConsoleTestState, Output},
     test_result::TestResult,
     time,
     types::{TestDesc, TestType},
 };
 
-pub struct JunitFormatter<T> {
-    out: OutputLocation<T>,
+pub struct JunitFormatter<'a> {
+    out: &'a mut dyn Output,
     results: Vec<(TestDesc, TestResult, Duration, Vec<u8>)>,
 }
 
-impl<T: Write> JunitFormatter<T> {
-    pub fn new(out: OutputLocation<T>) -> Self {
+impl<'a> JunitFormatter<'a> {
+    pub fn new(out: &'a mut dyn Output) -> Self {
         Self { out, results: Vec::new() }
     }
 
     fn write_message(&mut self, s: &str) -> io::Result<()> {
         assert!(!s.contains('\n'));
 
-        self.out.write_all(s.as_ref())
+        self.out.write_plain(s)
     }
 }
 
@@ -38,7 +38,7 @@ fn str_to_cdata(s: &str) -> String {
     format!("<![CDATA[{}]]>", escaped_output)
 }
 
-impl<T: Write> OutputFormatter for JunitFormatter<T> {
+impl OutputFormatter for JunitFormatter<'_> {
     fn write_discovery_start(&mut self) -> io::Result<()> {
         Err(io::Error::new(io::ErrorKind::NotFound, "Not yet implemented!"))
     }
@@ -179,7 +179,7 @@ impl<T: Write> OutputFormatter for JunitFormatter<T> {
         self.write_message("</testsuite>")?;
         self.write_message("</testsuites>")?;
 
-        self.out.write_all(b"\n")?;
+        self.out.write_plain("\n")?;
 
         Ok(state.failed == 0)
     }

From 2e624477e30289734194f0ef909115126da93320 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 16:26:51 +0100
Subject: [PATCH 09/11] Hide implementation details of `OutputLocation<T>` from
 TerseFormatter.

---
 library/test/src/console.rs          | 11 +++++---
 library/test/src/formatters/terse.rs | 38 ++++++++--------------------
 2 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index f437fb0709cd2..d9ce415d147e1 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -207,7 +207,7 @@ pub fn list_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Res
         OutputFormat::Pretty | OutputFormat::Junit => {
             Box::new(PrettyFormatter::new(&mut output, false, 0, false, None))
         }
-        OutputFormat::Terse => Box::new(TerseFormatter::new(output, false, 0, false)),
+        OutputFormat::Terse => Box::new(TerseFormatter::new(&mut output, false, 0, false)),
         OutputFormat::Json => Box::new(JsonFormatter::new(&mut output)),
     };
     let mut st = ConsoleTestDiscoveryState::new(opts)?;
@@ -332,9 +332,12 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Resu
             is_multithreaded,
             opts.time_options,
         )),
-        OutputFormat::Terse => {
-            Box::new(TerseFormatter::new(output, opts.use_color(), max_name_len, is_multithreaded))
-        }
+        OutputFormat::Terse => Box::new(TerseFormatter::new(
+            &mut output,
+            opts.use_color(),
+            max_name_len,
+            is_multithreaded,
+        )),
         OutputFormat::Json => Box::new(JsonFormatter::new(&mut output)),
         OutputFormat::Junit => Box::new(JunitFormatter::new(&mut output)),
     };
diff --git a/library/test/src/formatters/terse.rs b/library/test/src/formatters/terse.rs
index 875c66e5fa32c..fa20641505f9e 100644
--- a/library/test/src/formatters/terse.rs
+++ b/library/test/src/formatters/terse.rs
@@ -1,9 +1,9 @@
-use std::{io, io::prelude::Write};
+use std::io;
 
 use super::OutputFormatter;
 use crate::{
     bench::fmt_bench_samples,
-    console::{ConsoleTestDiscoveryState, ConsoleTestState, OutputLocation},
+    console::{ConsoleTestDiscoveryState, ConsoleTestState, Output},
     term,
     test_result::TestResult,
     time,
@@ -15,8 +15,8 @@ use crate::{
 // result chars leaves 12 chars for a progress count like " 11704/12853".
 const QUIET_MODE_MAX_COLUMN: usize = 88;
 
-pub(crate) struct TerseFormatter<T> {
-    out: OutputLocation<T>,
+pub(crate) struct TerseFormatter<'a> {
+    out: &'a mut dyn Output,
     use_color: bool,
     is_multithreaded: bool,
     /// Number of columns to fill when aligning names
@@ -27,9 +27,9 @@ pub(crate) struct TerseFormatter<T> {
     total_test_count: usize,
 }
 
-impl<T: Write> TerseFormatter<T> {
+impl<'a> TerseFormatter<'a> {
     pub fn new(
-        out: OutputLocation<T>,
+        out: &'a mut dyn Output,
         use_color: bool,
         max_name_len: usize,
         is_multithreaded: bool,
@@ -98,29 +98,13 @@ impl<T: Write> TerseFormatter<T> {
         Ok(())
     }
 
-    pub fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
-        match self.out {
-            OutputLocation::Pretty(ref mut term) => {
-                if self.use_color {
-                    term.fg(color)?;
-                }
-                term.write_all(word.as_bytes())?;
-                if self.use_color {
-                    term.reset()?;
-                }
-                term.flush()
-            }
-            OutputLocation::Raw(ref mut stdout) => {
-                stdout.write_all(word.as_bytes())?;
-                stdout.flush()
-            }
-        }
+    fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
+        if self.use_color { self.out.write_pretty(word, color) } else { self.out.write_plain(word) }
     }
 
-    pub fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
+    fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
         let s = s.as_ref();
-        self.out.write_all(s.as_bytes())?;
-        self.out.flush()
+        self.out.write_plain(s)
     }
 
     pub fn write_outputs(&mut self, state: &ConsoleTestState) -> io::Result<()> {
@@ -187,7 +171,7 @@ impl<T: Write> TerseFormatter<T> {
     }
 }
 
-impl<T: Write> OutputFormatter for TerseFormatter<T> {
+impl OutputFormatter for TerseFormatter<'_> {
     fn write_discovery_start(&mut self) -> io::Result<()> {
         Ok(())
     }

From 06cef6a96a2e578137bcd79bbe4eac3c266b6fad Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 17:08:02 +0100
Subject: [PATCH 10/11] Reimplement logfile output via `console::Output` trait.

Make logfile output more similar to normal test output.
---
 library/test/src/console.rs | 49 ++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index d9ce415d147e1..ac75c7936c22a 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -4,6 +4,7 @@ use std::fs::File;
 use std::io;
 use std::io::prelude::Write;
 use std::time::Instant;
+use std::vec;
 
 use super::{
     bench::fmt_bench_samples,
@@ -34,14 +35,14 @@ impl<T: Write> Write for OutputLocation<T> {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
         match *self {
             OutputLocation::Pretty(ref mut term) => term.write(buf),
-            OutputLocation::Raw(ref mut stdout) => stdout.write(buf),
+            OutputLocation::Raw(ref mut stdout_or_file) => stdout_or_file.write(buf),
         }
     }
 
     fn flush(&mut self) -> io::Result<()> {
         match *self {
             OutputLocation::Pretty(ref mut term) => term.flush(),
-            OutputLocation::Raw(ref mut stdout) => stdout.flush(),
+            OutputLocation::Raw(ref mut stdout_or_file) => stdout_or_file.flush(),
         }
     }
 }
@@ -68,8 +69,30 @@ impl<T: Write> Output for OutputLocation<T> {
     }
 }
 
+struct OutputMultiplexer {
+    pub outputs: Vec<Box<dyn Output>>,
+}
+
+impl Output for OutputMultiplexer {
+    fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
+        for output in &mut self.outputs {
+            output.write_pretty(word, color)?;
+        }
+
+        Ok(())
+    }
+
+    fn write_plain(&mut self, word: &str) -> io::Result<()> {
+        for output in &mut self.outputs {
+            output.write_plain(word)?;
+        }
+
+        Ok(())
+    }
+}
+
 pub struct ConsoleTestDiscoveryState {
-    pub log_out: Option<File>,
+    log_out: OutputMultiplexer,
     pub tests: usize,
     pub benchmarks: usize,
     pub ignored: usize,
@@ -77,9 +100,12 @@ pub struct ConsoleTestDiscoveryState {
 
 impl ConsoleTestDiscoveryState {
     pub fn new(opts: &TestOpts) -> io::Result<ConsoleTestDiscoveryState> {
-        let log_out = match opts.logfile {
-            Some(ref path) => Some(File::create(path)?),
-            None => None,
+        let mut log_out = OutputMultiplexer { outputs: vec![] };
+        match opts.logfile {
+            Some(ref path) => {
+                log_out.outputs.push(Box::new(OutputLocation::Raw(File::create(path)?)))
+            }
+            None => (),
         };
 
         Ok(ConsoleTestDiscoveryState { log_out, tests: 0, benchmarks: 0, ignored: 0 })
@@ -90,14 +116,9 @@ impl ConsoleTestDiscoveryState {
         S: AsRef<str>,
         F: FnOnce() -> S,
     {
-        match self.log_out {
-            None => Ok(()),
-            Some(ref mut o) => {
-                let msg = msg();
-                let msg = msg.as_ref();
-                o.write_all(msg.as_bytes())
-            }
-        }
+        let msg = msg();
+        let msg = msg.as_ref();
+        self.log_out.write_plain(msg)
     }
 }
 

From 1c78c9c9a6e2e3a7baf43a70d0a55a350f9c51e7 Mon Sep 17 00:00:00 2001
From: Alexander Potashev <aspotashev@google.com>
Date: Thu, 28 Mar 2024 20:50:21 +0100
Subject: [PATCH 11/11] At test discovery, write to logfile in the same format
 as to stdout.

Example of affected command:

```
cargo test -- --logfile=/tmp/1.log --list
```
---
 library/test/src/console.rs | 67 ++++++++++++++++++++-----------------
 library/test/src/tests.rs   |  2 +-
 2 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/library/test/src/console.rs b/library/test/src/console.rs
index ac75c7936c22a..8eaf30a88a4a8 100644
--- a/library/test/src/console.rs
+++ b/library/test/src/console.rs
@@ -3,6 +3,7 @@
 use std::fs::File;
 use std::io;
 use std::io::prelude::Write;
+use std::path::PathBuf;
 use std::time::Instant;
 use std::vec;
 
@@ -73,6 +74,33 @@ struct OutputMultiplexer {
     pub outputs: Vec<Box<dyn Output>>,
 }
 
+impl OutputMultiplexer {
+    pub fn new(lock_stdout: bool, logfile: &Option<PathBuf>) -> io::Result<Self> {
+        let mut outputs: Vec<Box<dyn Output>> = vec![];
+
+        if lock_stdout {
+            let output = match term::stdout() {
+                None => OutputLocation::Raw(io::stdout().lock()),
+                Some(t) => OutputLocation::Pretty(t),
+            };
+            outputs.push(Box::new(output))
+        } else {
+            let output = match term::stdout() {
+                None => OutputLocation::Raw(io::stdout()),
+                Some(t) => OutputLocation::Pretty(t),
+            };
+            outputs.push(Box::new(output))
+        }
+
+        match logfile {
+            Some(ref path) => outputs.push(Box::new(OutputLocation::Raw(File::create(path)?))),
+            None => (),
+        };
+
+        Ok(Self { outputs })
+    }
+}
+
 impl Output for OutputMultiplexer {
     fn write_pretty(&mut self, word: &str, color: term::color::Color) -> io::Result<()> {
         for output in &mut self.outputs {
@@ -92,33 +120,14 @@ impl Output for OutputMultiplexer {
 }
 
 pub struct ConsoleTestDiscoveryState {
-    log_out: OutputMultiplexer,
     pub tests: usize,
     pub benchmarks: usize,
     pub ignored: usize,
 }
 
 impl ConsoleTestDiscoveryState {
-    pub fn new(opts: &TestOpts) -> io::Result<ConsoleTestDiscoveryState> {
-        let mut log_out = OutputMultiplexer { outputs: vec![] };
-        match opts.logfile {
-            Some(ref path) => {
-                log_out.outputs.push(Box::new(OutputLocation::Raw(File::create(path)?)))
-            }
-            None => (),
-        };
-
-        Ok(ConsoleTestDiscoveryState { log_out, tests: 0, benchmarks: 0, ignored: 0 })
-    }
-
-    pub fn write_log<F, S>(&mut self, msg: F) -> io::Result<()>
-    where
-        S: AsRef<str>,
-        F: FnOnce() -> S,
-    {
-        let msg = msg();
-        let msg = msg.as_ref();
-        self.log_out.write_plain(msg)
+    pub fn new() -> io::Result<ConsoleTestDiscoveryState> {
+        Ok(ConsoleTestDiscoveryState { tests: 0, benchmarks: 0, ignored: 0 })
     }
 }
 
@@ -219,21 +228,18 @@ impl ConsoleTestState {
 
 // List the tests to console, and optionally to logfile. Filters are honored.
 pub fn list_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Result<()> {
-    let mut output = match term::stdout() {
-        None => OutputLocation::Raw(io::stdout().lock()),
-        Some(t) => OutputLocation::Pretty(t),
-    };
-
+    let mut multiplexer = OutputMultiplexer::new(true, &opts.logfile)?;
     let mut out: Box<dyn OutputFormatter> = match opts.format {
         OutputFormat::Pretty | OutputFormat::Junit => {
-            Box::new(PrettyFormatter::new(&mut output, false, 0, false, None))
+            Box::new(PrettyFormatter::new(&mut multiplexer, false, 0, false, None))
         }
-        OutputFormat::Terse => Box::new(TerseFormatter::new(&mut output, false, 0, false)),
-        OutputFormat::Json => Box::new(JsonFormatter::new(&mut output)),
+        OutputFormat::Terse => Box::new(TerseFormatter::new(&mut multiplexer, false, 0, false)),
+        OutputFormat::Json => Box::new(JsonFormatter::new(&mut multiplexer)),
     };
-    let mut st = ConsoleTestDiscoveryState::new(opts)?;
 
     out.write_discovery_start()?;
+
+    let mut st = ConsoleTestDiscoveryState::new()?;
     for test in filter_tests(opts, tests).into_iter() {
         use crate::TestFn::*;
 
@@ -253,7 +259,6 @@ pub fn list_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Res
         st.ignored += if desc.ignore { 1 } else { 0 };
 
         out.write_test_discovered(&desc, fntype)?;
-        st.write_log(|| format!("{fntype} {}\n", desc.name))?;
     }
 
     out.write_discovery_finish(&st)
diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs
index fc96a06549162..005cabd3c3885 100644
--- a/library/test/src/tests.rs
+++ b/library/test/src/tests.rs
@@ -994,7 +994,7 @@ fn test_discovery_logfile_format() {
     // Split output at line breaks to make the comparison platform-agnostic regarding newline style.
     let contents_lines = contents.as_str().lines().collect::<Vec<&str>>();
 
-    assert_eq!(contents_lines, vec!["test whatever"]);
+    assert_eq!(contents_lines, vec!["whatever: test", "", "1 test, 0 benchmarks"]);
 }
 
 #[test]