From 8888f99dc5d60ca7ac7ea98036129d78c924025f Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 24 Jan 2024 16:48:52 -0600 Subject: [PATCH] Remove ReadYourWrite utility struct The ReadYourWrite struct aided in testing as it implemented `Write` and produced a way to read the contents that were written. This struct was used for testing purposes and Manuel identified that it would be preferable to have a way to retrieve the Write IO struct directly. Previously this would have meant introducing a public interface to the build log trait as all type information was hidden behind a boxed trait return. As the trait implementation is removed we can now add a testing function that exposes the writer and can remove this single purpose class. Fun fact: Prior to introducing the ReadYourWrite struct, earlier versions of the code used a file to pass into the BuildLog as a `Write` that could later be read since the filename was not consumed and could be read even once the `BuildLog` and associated file handle are not accessible. --- libherokubuildpack/src/output/build_log.rs | 65 +++++++------- libherokubuildpack/src/output/util.rs | 98 ---------------------- 2 files changed, 33 insertions(+), 130 deletions(-) diff --git a/libherokubuildpack/src/output/build_log.rs b/libherokubuildpack/src/output/build_log.rs index 2d5a6847..6396ab8d 100644 --- a/libherokubuildpack/src/output/build_log.rs +++ b/libherokubuildpack/src/output/build_log.rs @@ -114,11 +114,17 @@ where } } - pub fn finish_logging(mut self) { + #[must_use] + fn finish_logging_writer(mut self) -> W { let elapsed = style::time::human(&self.data.started.elapsed()); let details = style::details(format!("finished in {elapsed}")); writeln_now(&mut self.io, style::section(format!("Done {details}"))); + self.io + } + + pub fn finish_logging(self) { + let _ = self.finish_logging_writer(); } pub fn announce(self) -> AnnounceLog { @@ -461,17 +467,15 @@ fn writeln_now(destination: &mut D, msg: impl AsRef) { mod test { use super::*; use crate::command::CommandExt; - use crate::output::style::{self, strip_control_codes}; - use crate::output::util::{strip_trailing_whitespace, ReadYourWrite}; + use crate::output::style::strip_control_codes; + use crate::output::util::strip_trailing_whitespace; use indoc::formatdoc; use libcnb_test::assert_contains; use pretty_assertions::assert_eq; #[test] fn test_captures() { - let writer = ReadYourWrite::writer(Vec::new()); - let reader = writer.reader(); - + let writer = Vec::new(); let mut stream = BuildLog::new(writer) .buildpack_name("Heroku Ruby Buildpack") .section("Ruby version `3.1.3` from `Gemfile.lock`") @@ -484,7 +488,10 @@ mod test { let value = "stuff".to_string(); writeln!(stream.io(), "{value}").unwrap(); - stream.finish_timed_stream().end_section().finish_logging(); + let io = stream + .finish_timed_stream() + .end_section() + .finish_logging_writer(); let expected = formatdoc! {" @@ -503,15 +510,13 @@ mod test { assert_eq!( expected, - strip_trailing_whitespace(style::strip_control_codes(reader.read_lossy().unwrap())) + strip_trailing_whitespace(strip_control_codes(String::from_utf8_lossy(&io))) ); } #[test] fn test_streaming_a_command() { - let writer = ReadYourWrite::writer(Vec::new()); - let reader = writer.reader(); - + let writer = Vec::new(); let mut stream = BuildLog::new(writer) .buildpack_name("Streaming buildpack demo") .section("Command streaming") @@ -522,20 +527,20 @@ mod test { .output_and_write_streams(stream.io(), stream.io()) .unwrap(); - stream.finish_timed_stream().end_section().finish_logging(); + let io = stream + .finish_timed_stream() + .end_section() + .finish_logging_writer(); - let actual = - strip_trailing_whitespace(style::strip_control_codes(reader.read_lossy().unwrap())); + let actual = strip_trailing_whitespace(strip_control_codes(String::from_utf8_lossy(&io))); assert_contains!(actual, " hello world\n"); } #[test] fn warning_step_padding() { - let writer = ReadYourWrite::writer(Vec::new()); - let reader = writer.reader(); - - BuildLog::new(writer) + let writer = Vec::new(); + let io = BuildLog::new(writer) .buildpack_name("RCT") .section("Guest thoughs") .step("The scenery here is wonderful") @@ -545,7 +550,7 @@ mod test { .step("The jumping fountains are great") .step("The music is nice here") .end_section() - .finish_logging(); + .finish_logging_writer(); let expected = formatdoc! {" @@ -562,28 +567,26 @@ mod test { - Done (finished in < 0.1s) "}; - assert_eq!(expected, strip_control_codes(reader.read_lossy().unwrap())); + assert_eq!(expected, strip_control_codes(String::from_utf8_lossy(&io))); } #[test] fn double_warning_step_padding() { - let writer = ReadYourWrite::writer(Vec::new()); - let reader = writer.reader(); - + let writer = Vec::new(); let logger = BuildLog::new(writer) .buildpack_name("RCT") .section("Guest thoughs") .step("The scenery here is wonderful") .announce(); - logger + let io = logger .warning("It's too crowded here") .warning("I'm tired") .end_announce() .step("The jumping fountains are great") .step("The music is nice here") .end_section() - .finish_logging(); + .finish_logging_writer(); let expected = formatdoc! {" @@ -601,22 +604,20 @@ mod test { - Done (finished in < 0.1s) "}; - assert_eq!(expected, strip_control_codes(reader.read_lossy().unwrap())); + assert_eq!(expected, strip_control_codes(String::from_utf8_lossy(&io))); } #[test] fn announce_and_exit_makes_no_whitespace() { - let writer = ReadYourWrite::writer(Vec::new()); - let reader = writer.reader(); - - BuildLog::new(writer) + let writer = Vec::new(); + let io = BuildLog::new(writer) .buildpack_name("Quick and simple") .section("Start") .step("Step") .announce() // <== Here .end_announce() // <== Here .end_section() - .finish_logging(); + .finish_logging_writer(); let expected = formatdoc! {" @@ -627,6 +628,6 @@ mod test { - Done (finished in < 0.1s) "}; - assert_eq!(expected, strip_control_codes(reader.read_lossy().unwrap())); + assert_eq!(expected, strip_control_codes(String::from_utf8_lossy(&io))); } } diff --git a/libherokubuildpack/src/output/util.rs b/libherokubuildpack/src/output/util.rs index ee82a7b9..ff4bf5ee 100644 --- a/libherokubuildpack/src/output/util.rs +++ b/libherokubuildpack/src/output/util.rs @@ -1,106 +1,8 @@ use lazy_static::lazy_static; -use std::fmt::Debug; -use std::io::Write; -use std::ops::Deref; -use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; - lazy_static! { static ref TRAILING_WHITESPACE_RE: regex::Regex = regex::Regex::new(r"\s+$").expect("clippy"); } -/// Threadsafe writer that can be read from -/// -/// Useful for testing -#[derive(Debug)] -pub(crate) struct ReadYourWrite -where - W: Write + AsRef<[u8]>, -{ - arc: Arc>, -} - -impl Clone for ReadYourWrite -where - W: Write + AsRef<[u8]> + Debug, -{ - fn clone(&self) -> Self { - Self { - arc: self.arc.clone(), - } - } -} - -impl Write for ReadYourWrite -where - W: Write + AsRef<[u8]> + Debug, -{ - fn write(&mut self, buf: &[u8]) -> std::io::Result { - let mut writer = self.arc.lock().expect("Internal error"); - writer.write(buf) - } - - fn flush(&mut self) -> std::io::Result<()> { - let mut writer = self.arc.lock().expect("Internal error"); - writer.flush() - } -} - -impl ReadYourWrite -where - W: Write + AsRef<[u8]>, -{ - #[allow(dead_code)] - pub(crate) fn writer(writer: W) -> Self { - Self { - arc: Arc::new(Mutex::new(writer)), - } - } - - #[must_use] - #[allow(dead_code)] - pub(crate) fn reader(&self) -> Reader { - Reader { - arc: self.arc.clone(), - } - } - - #[must_use] - #[allow(dead_code)] - pub(crate) fn arc_io(&self) -> Arc> { - self.arc.clone() - } -} - -pub(crate) struct Reader -where - W: Write + AsRef<[u8]>, -{ - arc: Arc>, -} - -impl Reader -where - W: Write + AsRef<[u8]>, -{ - #[allow(dead_code)] - pub(crate) fn read_lossy(&self) -> Result>> { - let io = &self.arc.lock()?; - - Ok(String::from_utf8_lossy(io.as_ref()).to_string()) - } -} - -impl Deref for Reader -where - W: Write + AsRef<[u8]>, -{ - type Target = Arc>; - - fn deref(&self) -> &Self::Target { - &self.arc - } -} - /// Iterator yielding every line in a string. The line includes newline character(s). /// ///