From 74708c6698598583b8b016e327f75a70c6fe2a37 Mon Sep 17 00:00:00 2001
From: Ed Morley <501702+edmorley@users.noreply.github.com>
Date: Thu, 17 Aug 2023 14:54:15 +0000
Subject: [PATCH] libcnb-test: Implement `fmt::Display` for `LogOutput` (#635)

Since:
- There was already duplication of formatting stderr/stdout output,
  which was otherwise going to get worse after #636.
- It may also be useful for end users when debugging, to save them
  having to manually print both stderr and stdout manually.

Prep for #482.
GUS-W-13966399.
---
 CHANGELOG.md                   |  1 +
 libcnb-test/src/log.rs         |  9 +++++++++
 libcnb-test/src/test_runner.rs | 12 +++++-------
 libcnb-test/src/util.rs        | 19 +++++++++----------
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 63731b73..413ba5ff 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ separate changelogs for each crate were used. If you need to refer to these old
 ### Added
 
 - `libcnb-package`: Add cross-compilation assistance for Linux `aarch64-unknown-linux-musl`. ([#577](https://github.com/heroku/libcnb.rs/pull/577))
+- `libcnb-test`: `LogOutput` now implements `std::fmt::Display`. ([#635](https://github.com/heroku/libcnb.rs/pull/635))
 
 ### Changed
 
diff --git a/libcnb-test/src/log.rs b/libcnb-test/src/log.rs
index 519b6717..6bb93726 100644
--- a/libcnb-test/src/log.rs
+++ b/libcnb-test/src/log.rs
@@ -1,6 +1,15 @@
+use std::fmt::Display;
+
 /// Log output from a command.
 #[derive(Debug, Default)]
 pub struct LogOutput {
     pub stdout: String,
     pub stderr: String,
 }
+
+impl Display for LogOutput {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let LogOutput { stdout, stderr } = self;
+        write!(f, "## stderr:\n\n{stderr}\n## stdout:\n\n{stdout}\n")
+    }
+}
diff --git a/libcnb-test/src/test_runner.rs b/libcnb-test/src/test_runner.rs
index d88c9c2d..67284e71 100644
--- a/libcnb-test/src/test_runner.rs
+++ b/libcnb-test/src/test_runner.rs
@@ -1,9 +1,7 @@
 use crate::docker::DockerRemoveImageCommand;
 use crate::pack::PackBuildCommand;
 use crate::util::CommandError;
-use crate::{
-    app, build, util, BuildConfig, BuildpackReference, LogOutput, PackResult, TestContext,
-};
+use crate::{app, build, util, BuildConfig, BuildpackReference, PackResult, TestContext};
 use std::borrow::Borrow;
 use std::env;
 use std::path::PathBuf;
@@ -119,10 +117,10 @@ impl TestRunner {
 
         let output = match (&config.expected_pack_result, pack_result) {
             (PackResult::Success, Ok(output)) => output,
-            (PackResult::Failure, Err(CommandError::NonZeroExitCode { stdout, stderr, .. })) => {
-                LogOutput { stdout, stderr }
+            (PackResult::Failure, Err(CommandError::NonZeroExitCode { log_output, .. })) => {
+                log_output
             }
-            (PackResult::Failure, Ok(LogOutput { stdout, stderr })) => {
+            (PackResult::Failure, Ok(log_output)) => {
                 // Ordinarily the Docker image created by `pack build` will either be cleaned up by
                 // `TestContext::Drop` later on, or will not have been created in the first place,
                 // if the `pack build` was not successful. However, in the case of an unexpectedly
@@ -130,7 +128,7 @@ impl TestRunner {
                 util::run_command(DockerRemoveImageCommand::new(image_name)).unwrap_or_else(
                     |command_err| panic!("Error removing Docker image:\n\n{command_err}"),
                 );
-                panic!("The pack build was expected to fail, but did not:\n\n## stderr:\n\n{stderr}\n## stdout:\n\n{stdout}\n");
+                panic!("The pack build was expected to fail, but did not:\n\n{log_output}");
             }
             (_, Err(command_err)) => {
                 panic!("Error performing pack build:\n\n{command_err}");
diff --git a/libcnb-test/src/util.rs b/libcnb-test/src/util.rs
index e0dd8eef..5c629269 100644
--- a/libcnb-test/src/util.rs
+++ b/libcnb-test/src/util.rs
@@ -40,17 +40,18 @@ pub(crate) fn run_command(command: impl Into<Command>) -> Result<LogOutput, Comm
             }
         })
         .and_then(|output| {
-            let stdout = String::from_utf8_lossy(&output.stdout).into_owned();
-            let stderr = String::from_utf8_lossy(&output.stderr).into_owned();
+            let log_output = LogOutput {
+                stdout: String::from_utf8_lossy(&output.stdout).into_owned(),
+                stderr: String::from_utf8_lossy(&output.stderr).into_owned(),
+            };
 
             if output.status.success() {
-                Ok(LogOutput { stdout, stderr })
+                Ok(log_output)
             } else {
                 Err(CommandError::NonZeroExitCode {
                     program,
                     exit_code: output.status.code(),
-                    stdout,
-                    stderr,
+                    log_output,
                 })
             }
         })
@@ -69,8 +70,7 @@ pub(crate) enum CommandError {
     NonZeroExitCode {
         exit_code: Option<i32>,
         program: String,
-        stdout: String,
-        stderr: String,
+        log_output: LogOutput,
     },
 }
 
@@ -89,11 +89,10 @@ impl Display for CommandError {
             CommandError::NonZeroExitCode {
                 program,
                 exit_code,
-                stdout,
-                stderr,
+                log_output,
             } => write!(
                 f,
-                "{program} command failed with exit code {}!\n\n## stderr:\n\n{stderr}\n## stdout:\n\n{stdout}\n",
+                "{program} command failed with exit code {}!\n\n{log_output}",
                 exit_code.map_or(String::from("<unknown>"), |exit_code| exit_code.to_string())
             ),
         }