Skip to content

Commit

Permalink
Remove ReadYourWrite utility struct
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@schneems
schneems committed Jan 24, 2024
1 parent 3f928e7 commit 8888f99
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 130 deletions.
65 changes: 33 additions & 32 deletions libherokubuildpack/src/output/build_log.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<state::Started, W> {
Expand Down Expand Up @@ -461,17 +467,15 @@ fn writeln_now<D: Write>(destination: &mut D, msg: impl AsRef<str>) {
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`")
Expand All @@ -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! {"
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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! {"
Expand All @@ -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! {"
Expand All @@ -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! {"
Expand All @@ -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)));
}
}
98 changes: 0 additions & 98 deletions libherokubuildpack/src/output/util.rs
View file
Original file line number Diff line number Diff line change
@@ -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<W>
where
W: Write + AsRef<[u8]>,
{
arc: Arc<Mutex<W>>,
}

impl<W> Clone for ReadYourWrite<W>
where
W: Write + AsRef<[u8]> + Debug,
{
fn clone(&self) -> Self {
Self {
arc: self.arc.clone(),
}
}
}

impl<W> Write for ReadYourWrite<W>
where
W: Write + AsRef<[u8]> + Debug,
{
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
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<W> ReadYourWrite<W>
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<W> {
Reader {
arc: self.arc.clone(),
}
}

#[must_use]
#[allow(dead_code)]
pub(crate) fn arc_io(&self) -> Arc<Mutex<W>> {
self.arc.clone()
}
}

pub(crate) struct Reader<W>
where
W: Write + AsRef<[u8]>,
{
arc: Arc<Mutex<W>>,
}

impl<W> Reader<W>
where
W: Write + AsRef<[u8]>,
{
#[allow(dead_code)]
pub(crate) fn read_lossy(&self) -> Result<String, PoisonError<MutexGuard<'_, W>>> {
let io = &self.arc.lock()?;

Ok(String::from_utf8_lossy(io.as_ref()).to_string())
}
}

impl<W> Deref for Reader<W>
where
W: Write + AsRef<[u8]>,
{
type Target = Arc<Mutex<W>>;

fn deref(&self) -> &Self::Target {
&self.arc
}
}

/// Iterator yielding every line in a string. The line includes newline character(s).
///
/// <https://stackoverflow.com/a/40457615>
Expand Down

0 comments on commit 8888f99

Please sign in to comment.