[M] Change in flashrom[master]: flashrom_tester: Remove --output log redirect option

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71773 )

 ( 

2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: flashrom_tester: Remove --output log redirect option
......................................................................

flashrom_tester: Remove --output log redirect option

Always print logs to stdout. User can redirect logs in the normal way if
they wish.

BUG=b:194245688
BRANCH=None
TEST=clippy, unit

Change-Id: I5eab8169644a16ba31b203e8607853c459f92978
Signed-off-by: Evan Benn <[e-mail]>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/71773
Reviewed-by: Nikolai Artemiev <[e-mail]>
Reviewed-by: Peter Marheine <[e-mail]>
Reviewed-by: Edward O'Callaghan <[e-mail]>
Tested-by: build bot (Jenkins) <[e-mail]>
---
A util/flashrom_tester/.cargo/config.toml
M util/flashrom_tester/Cargo.toml
M util/flashrom_tester/src/logger.rs
M util/flashrom_tester/src/main.rs
4 files changed, 51 insertions(+), 66 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Edward O'Callaghan: Looks good to me, approved
  Peter Marheine: Looks good to me, but someone else must approve
  Nikolai Artemiev: Looks good to me, but someone else must approve




diff --git a/util/flashrom_tester/.cargo/config.toml b/util/flashrom_tester/.cargo/config.toml
new file mode 100644
index 0000000..8af59dd
--- /dev/null
+++ b/util/flashrom_tester/.cargo/config.toml
@@ -0,0 +1,2 @@
+[env]
+RUST_TEST_THREADS = "1"
diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml
index 948553a..e6ed9c0 100644
--- a/util/flashrom_tester/Cargo.toml
+++ b/util/flashrom_tester/Cargo.toml
@@ -30,6 +30,9 @@
 [build-dependencies]
 built = { version = "0.5", features = ["chrono"] }
 
+[dev-dependencies]
+gag = "1"
+
 [features]
 # Features required to build the CLI binary but not the library
 cli = ["chrono", "clap"]
diff --git a/util/flashrom_tester/src/logger.rs b/util/flashrom_tester/src/logger.rs
index b700c19..c9c3640 100644
--- a/util/flashrom_tester/src/logger.rs
+++ b/util/flashrom_tester/src/logger.rs
@@ -35,67 +35,42 @@
 
 use flashrom_tester::types;
 use std::io::Write;
-use std::path::PathBuf;
-use std::sync::Mutex;
 
-struct Logger<W: Write + Send> {
+struct Logger {
     level: log::LevelFilter,
-    target: LogTarget<W>,
     color: types::Color,
 }
 
-enum LogTarget<W>
-where
-    W: Write,
-{
-    Terminal,
-    Write(Mutex<W>),
-}
-
-impl<W: Write + Send> log::Log for Logger<W> {
+impl log::Log for Logger {
     fn enabled(&self, metadata: &log::Metadata) -> bool {
         metadata.level() <= self.level
     }
 
     fn log(&self, record: &log::Record) {
-        fn log_internal<W: Write>(
-            mut w: W,
-            record: &log::Record,
-            color: &types::Color,
-        ) -> std::io::Result<()> {
-            let now = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Micros, true);
-            write!(w, "{}{} ", color.magenta, now)?;
-            write!(w, "{}[ {} ]{} ", color.yellow, record.level(), color.reset)?;
-            writeln!(w, "{}", record.args())
-        }
-
         // Write errors deliberately ignored
-        let _ = match self.target {
-            LogTarget::Terminal => {
-                let stdout = std::io::stdout();
-                let mut lock = stdout.lock();
-                log_internal(&mut lock, record, &self.color)
-            }
-            LogTarget::Write(ref mutex) => {
-                let mut lock = mutex.lock().unwrap();
-                log_internal(&mut *lock, record, &self.color)
-            }
-        };
+        let stdout = std::io::stdout();
+        let mut lock = stdout.lock();
+        let now = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Micros, true);
+        let _ = write!(lock, "{}{} ", self.color.magenta, now);
+        let _ = write!(
+            lock,
+            "{}[ {} ]{} ",
+            self.color.yellow,
+            record.level(),
+            self.color.reset
+        );
+        let _ = writeln!(lock, "{}", record.args());
     }
 
     fn flush(&self) {
         // Flush errors deliberately ignored
-        let _ = match self.target {
-            LogTarget::Terminal => std::io::stdout().flush(),
-            LogTarget::Write(ref w) => w.lock().unwrap().flush(),
-        };
+        let _ = std::io::stdout().flush();
     }
 }
 
-pub fn init(to_file: Option<PathBuf>, debug: bool) {
+pub fn init(debug: bool) {
     let mut logger = Logger {
         level: log::LevelFilter::Info,
-        target: LogTarget::Terminal,
         color: if atty::is(atty::Stream::Stdout) {
             types::COLOR
         } else {
@@ -106,31 +81,23 @@
     if debug {
         logger.level = log::LevelFilter::Debug;
     }
-    if let Some(path) = to_file {
-        logger.target = LogTarget::Write(Mutex::new(
-            std::fs::File::create(path).expect("Unable to open log file for writing"),
-        ));
-        logger.color = types::NOCOLOR;
-    }
-
     log::set_max_level(logger.level);
     log::set_boxed_logger(Box::new(logger)).unwrap();
 }
 
 #[cfg(test)]
 mod tests {
-    use super::{LogTarget, Logger};
+    use std::io::Read;
+
+    use super::Logger;
     use flashrom_tester::types;
     use log::{Level, LevelFilter, Log, Record};
-    use std::sync::Mutex;
 
     fn run_records(records: &[Record]) -> String {
-        let mut buf = Vec::<u8>::new();
+        let buf = gag::BufferRedirect::stdout().unwrap();
         {
-            let lock = Mutex::new(&mut buf);
             let logger = Logger {
                 level: LevelFilter::Info,
-                target: LogTarget::Write(lock),
                 color: types::COLOR,
             };
 
@@ -140,7 +107,9 @@
                 }
             }
         }
-        String::from_utf8(buf).unwrap()
+        let mut ret = String::new();
+        buf.into_inner().read_to_string(&mut ret).unwrap();
+        ret
     }
 
     /// Log messages have the expected format
diff --git a/util/flashrom_tester/src/main.rs b/util/flashrom_tester/src/main.rs
index 44429ba..b8a2581 100644
--- a/util/flashrom_tester/src/main.rs
+++ b/util/flashrom_tester/src/main.rs
@@ -41,7 +41,6 @@
 use clap::{App, Arg};
 use flashrom::{FlashChip, Flashrom, FlashromCmd, FlashromLib};
 use flashrom_tester::{tester, tests};
-use std::path::PathBuf;
 use std::sync::atomic::AtomicBool;
 
 pub mod built_info {
@@ -92,13 +91,6 @@
                 .help("Print the layout file's contents before running tests"),
         )
         .arg(
-            Arg::with_name("log-file")
-                .short("o")
-                .long("log-file")
-                .takes_value(true)
-                .help("Write logs to a file rather than stdout"),
-        )
-        .arg(
             Arg::with_name("log_debug")
                 .short("d")
                 .long("debug")
@@ -121,10 +113,7 @@
         )
         .get_matches();
 
-    logger::init(
-        matches.value_of_os("log-file").map(PathBuf::from),
-        matches.is_present("log_debug"),
-    );
+    logger::init(matches.is_present("log_debug"));
     debug!("Args parsed and logging initialized OK");
 
     debug!("Collecting crossystem info");

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/71773
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978
Gerrit-Change-Number: 71773
Gerrit-PatchSet: 4
Gerrit-Owner: Evan Benn <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: Nikolai Artemiev <[e-mail]>
Gerrit-Reviewer: Peter Marheine <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-MessageType: merged