Refactor stdout and stderr capturing to a library (#4970)

This should make it easy to add more tests which need the specific
behavior here. It also isolates where we reach into GoogleTest's
internals to a single common place.

testing/base/BUILD

@@ -45,6 +45,17 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "capture_std_streams",
+    testonly = 1,
+    srcs = ["capture_std_streams.cpp"],
+    hdrs = ["capture_std_streams.h"],
+    deps = [
+        "//common:ostream",
+        "@googletest//:gtest",
+    ],
+)
+
 cc_library(
     name = "source_gen_lib",
     testonly = 1,

testing/base/capture_std_streams.cpp

@@ -0,0 +1,34 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "testing/base/capture_std_streams.h"
+
+#include <gtest/gtest.h>
+
+#include <fstream>
+#include <sstream>
+
+#include "common/ostream.h"
+
+namespace Carbon::Testing::Internal {
+
+// While these are marked as "internal" APIs, they seem to work and be pretty
+// widely used for their exact documented behavior.
+using ::testing::internal::CaptureStderr;
+using ::testing::internal::CaptureStdout;
+using ::testing::internal::GetCapturedStderr;
+using ::testing::internal::GetCapturedStdout;
+
+auto BeginStdStreamCapture() -> void {
+  CaptureStderr();
+  CaptureStdout();
+}
+auto EndStdStreamCapture(std::string& out, std::string& err) -> void {
+  // No need to flush stderr.
+  err = GetCapturedStderr();
+  llvm::outs().flush();
+  out = GetCapturedStdout();
+}
+
+}  // namespace Carbon::Testing::Internal

testing/base/capture_std_streams.h

@@ -0,0 +1,37 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_TESTING_BASE_CAPTURE_STD_STREAMS_H_
+#define CARBON_TESTING_BASE_CAPTURE_STD_STREAMS_H_
+
+#include <string>
+
+namespace Carbon::Testing {
+
+// Implementation details.
+namespace Internal {
+auto BeginStdStreamCapture() -> void;
+auto EndStdStreamCapture(std::string& out, std::string& err) -> void;
+}  // namespace Internal
+
+// Calls the provided function while capturing both `stdout` and `stderr`. The
+// `out` and `err` strings are set to whatever is captured after the function
+// returns.
+//
+// Note that any output that is captured will not be visible when running, which
+// can make debugging tests that use this routine difficult. Make sure to either
+// print back out or otherwise expose any of the contents of the captured output
+// that are needed when debugging.
+template <typename FnT>
+static auto CallWithCapturedOutput(std::string& out, std::string& err,
+                                   FnT function) {
+  Internal::BeginStdStreamCapture();
+  auto result = function();
+  Internal::EndStdStreamCapture(out, err);
+  return result;
+}
+
+}  // namespace Carbon::Testing
+
+#endif  // CARBON_TESTING_BASE_CAPTURE_STD_STREAMS_H_

toolchain/driver/BUILD

@@ -49,6 +49,7 @@ cc_test(
         "//common:check",
         "//common:ostream",
         "//common:raw_string_ostream",
+        "//testing/base:capture_std_streams",
         "//testing/base:global_exe_path",
         "//testing/base:gtest_main",
         "@googletest//:gtest",

toolchain/driver/clang_runner_test.cpp

@@ -19,6 +19,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Program.h"
 #include "llvm/TargetParser/Host.h"
+#include "testing/base/capture_std_streams.h"
 #include "testing/base/global_exe_path.h"
 
 namespace Carbon {
@@ -27,31 +28,6 @@ namespace {
 using ::testing::HasSubstr;
 using ::testing::StrEq;
 
-// While these are marked as "internal" APIs, they seem to work and be pretty
-// widely used for their exact documented behavior.
-using ::testing::internal::CaptureStderr;
-using ::testing::internal::CaptureStdout;
-using ::testing::internal::GetCapturedStderr;
-using ::testing::internal::GetCapturedStdout;
-
-// Calls the provided lambda with `stderr` and `stdout` captured and saved into
-// the provided output parameters. The lambda's result is returned. It is
-// important to not put anything inside the lambda whose output would be useful
-// in interpreting test errors such as Google Test assertions as their output
-// will end up captured as well.
-template <typename CallableT>
-static auto RunWithCapturedOutput(std::string& out, std::string& err,
-                                  CallableT callable) {
-  CaptureStderr();
-  CaptureStdout();
-  auto result = callable();
-  // No need to flush stderr.
-  err = GetCapturedStderr();
-  llvm::outs().flush();
-  out = GetCapturedStdout();
-  return result;
-}
-
 TEST(ClangRunnerTest, Version) {
   RawStringOstream test_os;
   const auto install_paths =
@@ -62,8 +38,8 @@ TEST(ClangRunnerTest, Version) {
 
   std::string out;
   std::string err;
-  EXPECT_TRUE(RunWithCapturedOutput(out, err,
-                                    [&] { return runner.Run({"--version"}); }));
+  EXPECT_TRUE(Testing::CallWithCapturedOutput(
+      out, err, [&] { return runner.Run({"--version"}); }));
   // The arguments to Clang should be part of the verbose log.
   EXPECT_THAT(test_os.TakeStr(), HasSubstr("--version"));
 
@@ -132,12 +108,12 @@ TEST(ClangRunnerTest, LinkCommandEcho) {
   ClangRunner runner(&install_paths, target, vfs, &verbose_out);
   std::string out;
   std::string err;
-  EXPECT_TRUE(RunWithCapturedOutput(out, err,
-                                    [&] {
-                                      return runner.Run({"-###", "-o", "binary",
-                                                         foo_file.string(),
-                                                         bar_file.string()});
-                                    }))
+  EXPECT_TRUE(Testing::CallWithCapturedOutput(
+      out, err,
+      [&] {
+        return runner.Run(
+            {"-###", "-o", "binary", foo_file.string(), bar_file.string()});
+      }))
       << "Verbose output from runner:\n"
       << verbose_out.TakeStr() << "\n";
   verbose_out.clear();
@@ -166,12 +142,12 @@ TEST(ClangRunnerTest, DashC) {
   ClangRunner runner(&install_paths, target, vfs, &verbose_out);
   std::string out;
   std::string err;
-  EXPECT_TRUE(RunWithCapturedOutput(out, err,
-                                    [&] {
-                                      return runner.Run(
-                                          {"-c", test_file.string(), "-o",
-                                           test_output.string()});
-                                    }))
+  EXPECT_TRUE(Testing::CallWithCapturedOutput(
+      out, err,
+      [&] {
+        return runner.Run(
+            {"-c", test_file.string(), "-o", test_output.string()});
+      }))
       << "Verbose output from runner:\n"
       << verbose_out.TakeStr() << "\n";
   verbose_out.clear();
@@ -199,12 +175,12 @@ TEST(ClangRunnerTest, BuitinHeaders) {
   ClangRunner runner(&install_paths, target, vfs, &verbose_out);
   std::string out;
   std::string err;
-  EXPECT_TRUE(RunWithCapturedOutput(out, err,
-                                    [&] {
-                                      return runner.Run(
-                                          {"-c", test_file.string(), "-o",
-                                           test_output.string()});
-                                    }))
+  EXPECT_TRUE(Testing::CallWithCapturedOutput(
+      out, err,
+      [&] {
+        return runner.Run(
+            {"-c", test_file.string(), "-o", test_output.string()});
+      }))
       << "Verbose output from runner:\n"
       << verbose_out.TakeStr() << "\n";
   verbose_out.clear();