From 89757d72320772b7c0fa66f5dbf4c9c840a0cefa Mon Sep 17 00:00:00 2001
From: Andrew Lamb <alamb@influxdata.com>
Date: Wed, 30 Jun 2021 09:20:11 -0400
Subject: [PATCH] fix: do not print test output to logs except on failure
 (#1840)

* fix: do not print test output to logs except on failure

* docs: update CONTRIBUTING.md
---
 CONTRIBUTING.md                |  8 ++++----
 tests/common/server_fixture.rs | 24 +++++++++++++++---------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 0a47351266..249edcae67 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -120,15 +120,15 @@ RUST_LOG=debug,hyper::proto::h1=info,h2=info cargo test --workspace
 
 ### End-to-End Tests
 
-There are end-to-end tests that spin up a server and make requests via the client library and API. They can be found in `tests/end_to_end_cases` 
+There are end-to-end tests that spin up a server and make requests via the client library and API. They can be found in `tests/end_to_end_cases`
 
 They are run by `cargo test --workspace` command but can be run exclusively with:
 
 ```
 cargo test --test end-to-end
-``` 
+```
 
-Each server writes its logs to a temporary file and this is printed to stdout on shutdown, bypassing the default test log capturing. 
+Each server writes its logs to a temporary file and this is captured when the server shutsdown.
 
 If you are debugging a failing end-to-end test, you will likely want to run with `--nocapture` to also get the logs from the test execution in addition to the server:
 
@@ -139,7 +139,7 @@ cargo test --test end-to-end -- my_failing_test --nocapture
 If running multiple tests in parallel:
 
 * The output may be interleaved
-* Multiple tests may share the same server instance
+* Multiple tests may share the same server instance and thus the server logs may be captured in the output of a different test than the one that is failing.
 
 When debugging a failing test it is therefore recommended you run a single test, or disable parallel test execution
 
diff --git a/tests/common/server_fixture.rs b/tests/common/server_fixture.rs
index 0c949e8f00..e2f2b1a491 100644
--- a/tests/common/server_fixture.rs
+++ b/tests/common/server_fixture.rs
@@ -504,7 +504,7 @@ impl std::fmt::Display for TestServer {
 
 impl Drop for TestServer {
     fn drop(&mut self) {
-        use std::io::{Read, Write};
+        use std::io::Read;
 
         let mut server_lock = self
             .server_process
@@ -521,23 +521,29 @@ impl Drop for TestServer {
             .wait()
             .expect("Should have been able to wait for shutdown");
 
-        let mut out = std::io::stdout();
         let mut f = std::fs::File::open(&server_lock.log_path).expect("failed to open log file");
         let mut buffer = [0_u8; 8 * 1024];
 
-        writeln!(out, "****************").unwrap();
-        writeln!(out, "Start TestServer Output").unwrap();
-        writeln!(out, "****************").unwrap();
+        println!("****************");
+        println!("Start TestServer Output");
+        println!("****************");
 
         while let Ok(read) = f.read(&mut buffer) {
             if read == 0 {
                 break;
             }
-            out.write_all(&buffer[..read]).unwrap();
+            if let Ok(str) = std::str::from_utf8(&buffer[..read]) {
+                print!("{}", str);
+            } else {
+                println!(
+                    "\n\n-- ERROR IN TRANSFER -- please see {:?} for raw contents ---\n\n",
+                    &server_lock.log_path
+                );
+            }
         }
 
-        writeln!(out, "****************").unwrap();
-        writeln!(out, "End TestServer Output").unwrap();
-        writeln!(out, "****************").unwrap();
+        println!("****************");
+        println!("End TestServer Output");
+        println!("****************");
     }
 }