Add support for capturing console output to FileTest. (#4339)

One of the things that ClangRunnerTest is doing is capturing
stderr/stdout because clang prints to it directly. This adds support for
that to FileTest.

I'm renaming the current `capture_output` field to `dump_output` because
the name is ambiguous after this change, and the flag is already named
`--dump_output`. It's still not great, but at least it's more distinct.

Note ClangRunner still doesn't use the vfs; that still needs work. I'm
just moving the NoArgs test over as a trivial test of the functionality.

explorer/file_test.cpp

@@ -19,8 +19,8 @@ namespace {
 class ExplorerFileTest : public FileTestBase {
  public:
   explicit ExplorerFileTest(llvm::StringRef /*exe_path*/,
-                            llvm::StringRef test_name)
-      : FileTestBase(test_name),
+                            std::mutex* output_mutex, llvm::StringRef test_name)
+      : FileTestBase(output_mutex, test_name),
         prelude_line_re_(R"(prelude.carbon:(\d+))"),
         timing_re_(R"((Time elapsed in \w+: )\d+(ms))") {
     CARBON_CHECK(prelude_line_re_.ok(), "{0}", prelude_line_re_.error());

testing/file_test/README.md

@@ -148,6 +148,19 @@ Supported comment markers are:
     ARGS can be specified at most once. If not provided, the FileTestBase child
     is responsible for providing default arguments.
 
+-   ```
+    // SET-CAPTURE-CONSOLE-OUTPUT
+    ```
+
+    By default, stderr and stdout are expected to be piped through provided
+    streams. Adding this causes the test's own stderr and stdout to be captured
+    and added as well.
+
+    This should be avoided because we are partly ensuring that streams are an
+    API, but is helpful when wrapping Clang, where stderr is used directly.
+
+    SET-CAPTURE-CONSOLE-OUTPUT can be specified at most once.
+
 -   ```
     // SET-CHECK-SUBSET
     ```

testing/file_test/file_test_base.cpp

@@ -18,6 +18,7 @@
 #include "common/error.h"
 #include "common/exe_path.h"
 #include "common/init_llvm.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CrashRecoveryContext.h"
@@ -45,6 +46,13 @@ ABSL_FLAG(bool, dump_output, false,
 
 namespace Carbon::Testing {
 
+// 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;
+
 using ::testing::Matcher;
 using ::testing::MatchesRegex;
 using ::testing::StrEq;
@@ -248,7 +256,7 @@ auto FileTestBase::Autoupdate() -> ErrorOr<bool> {
 
 auto FileTestBase::DumpOutput() -> ErrorOr<Success> {
   TestContext context;
-  context.capture_output = false;
+  context.dump_output = true;
   std::string banner(79, '=');
   banner.append("\n");
   llvm::errs() << banner << "= " << test_name_ << "\n";
@@ -316,6 +324,25 @@ auto FileTestBase::ProcessTestFileAndRun(TestContext& context)
   llvm::PrettyStackTraceProgram stack_trace_entry(
       test_argv_for_stack_trace.size() - 1, test_argv_for_stack_trace.data());
 
+  // Conditionally capture console output. We use a scope exit to ensure the
+  // captures terminate even on run failures.
+  std::unique_lock<std::mutex> output_lock;
+  if (context.capture_console_output) {
+    if (output_mutex_) {
+      output_lock = std::unique_lock<std::mutex>(*output_mutex_);
+    }
+    CaptureStderr();
+    CaptureStdout();
+  }
+  auto add_output_on_exit = llvm::make_scope_exit([&]() {
+    if (context.capture_console_output) {
+      // No need to flush stderr.
+      llvm::outs().flush();
+      context.stdout += GetCapturedStdout();
+      context.stderr += GetCapturedStderr();
+    }
+  });
+
   // Prepare string streams to capture output. In order to address casting
   // constraints, we split calls to Run as a ternary based on whether we want to
   // capture output.
@@ -323,9 +350,9 @@ auto FileTestBase::ProcessTestFileAndRun(TestContext& context)
   llvm::raw_svector_ostream stderr(context.stderr);
   CARBON_ASSIGN_OR_RETURN(
       context.run_result,
-      context.capture_output
-          ? Run(test_args_ref, fs, stdout, stderr)
-          : Run(test_args_ref, fs, llvm::outs(), llvm::errs()));
+      context.dump_output ? Run(test_args_ref, fs, llvm::outs(), llvm::errs())
+                          : Run(test_args_ref, fs, stdout, stderr));
+
   return Success();
 }
 
@@ -780,17 +807,17 @@ static auto TryConsumeAutoupdate(int line_index, llvm::StringRef line_trimmed,
   return true;
 }
 
-// Processes SET-CHECK-SUBSET lines when found. Returns true if the line is
-// consumed.
-static auto TryConsumeSetCheckSubset(llvm::StringRef line_trimmed,
-                                     bool* check_subset) -> ErrorOr<bool> {
-  if (line_trimmed != "// SET-CHECK-SUBSET") {
+// Processes SET-* lines when found. Returns true if the line is consumed.
+static auto TryConsumeSetFlag(llvm::StringRef line_trimmed,
+                              llvm::StringLiteral flag_name, bool* flag)
+    -> ErrorOr<bool> {
+  if (!line_trimmed.consume_front("// ") || line_trimmed != flag_name) {
     return false;
   }
-  if (*check_subset) {
-    return ErrorBuilder() << "SET-CHECK-SUBSET was specified multiple times";
+  if (*flag) {
+    return ErrorBuilder() << flag_name << " was specified multiple times";
   }
-  *check_subset = true;
+  *flag = true;
   return true;
 }
 
@@ -866,7 +893,14 @@ auto FileTestBase::ProcessTestFile(TestContext& context) -> ErrorOr<Success> {
     }
     CARBON_ASSIGN_OR_RETURN(
         is_consumed,
-        TryConsumeSetCheckSubset(line_trimmed, &context.check_subset));
+        TryConsumeSetFlag(line_trimmed, "SET-CAPTURE-CONSOLE-OUTPUT",
+                          &context.capture_console_output));
+    if (is_consumed) {
+      continue;
+    }
+    CARBON_ASSIGN_OR_RETURN(is_consumed,
+                            TryConsumeSetFlag(line_trimmed, "SET-CHECK-SUBSET",
+                                              &context.check_subset));
     if (is_consumed) {
       continue;
     }
@@ -944,7 +978,7 @@ static auto RunAutoupdate(llvm::StringRef exe_path,
       crc.DumpStackAndCleanupOnFailure = true;
       bool thread_crashed = !crc.RunSafely([&] {
         std::unique_ptr<FileTestBase> test(
-            test_factory.factory_fn(exe_path, test_name));
+            test_factory.factory_fn(exe_path, &mutex, test_name));
         auto result = test->Autoupdate();
 
         std::unique_lock<std::mutex> lock(mutex);
@@ -1014,7 +1048,7 @@ static auto Main(int argc, char** argv) -> int {
   } else if (absl::GetFlag(FLAGS_dump_output)) {
     for (const auto& test_name : tests) {
       std::unique_ptr<FileTestBase> test(
-          test_factory.factory_fn(exe_path, test_name));
+          test_factory.factory_fn(exe_path, nullptr, test_name));
       auto result = test->DumpOutput();
       if (!result.ok()) {
         llvm::errs() << "\n" << result.error().message() << "\n";
@@ -1028,7 +1062,7 @@ static auto Main(int argc, char** argv) -> int {
           test_factory.name, test_name.data(), nullptr, test_name.data(),
           __FILE__, __LINE__,
           [&test_factory, &exe_path, test_name = test_name]() {
-            return test_factory.factory_fn(exe_path, test_name);
+            return test_factory.factory_fn(exe_path, nullptr, test_name);
           });
     }
     return RUN_ALL_TESTS();

testing/file_test/file_test_base.h

@@ -9,6 +9,7 @@
 #include <gtest/gtest.h>
 
 #include <functional>
+#include <mutex>
 
 #include "common/error.h"
 #include "common/ostream.h"
@@ -64,7 +65,8 @@ class FileTestBase : public testing::Test {
     llvm::SmallVector<std::pair<std::string, bool>> per_file_success;
   };
 
-  explicit FileTestBase(llvm::StringRef test_name) : test_name_(test_name) {}
+  explicit FileTestBase(std::mutex* output_mutex, llvm::StringRef test_name)
+      : output_mutex_(output_mutex), test_name_(test_name) {}
 
   // Implemented by children to run the test. For example, TestBody validates
   // stdout and stderr. Children should use fs for file content, and may add
@@ -149,12 +151,16 @@ class FileTestBase : public testing::Test {
     // The location of the autoupdate marker, for autoupdated files.
     std::optional<int> autoupdate_line_number;
 
+    // Whether to capture stderr and stdout that would head to console,
+    // generated from SET-CAPTURE-CONSOLE-OUTPUT.
+    bool capture_console_output = false;
+
     // Whether checks are a subset, generated from SET-CHECK-SUBSET.
     bool check_subset = false;
 
-    // Output is typically captured for tests and autoupdate, but not when
-    // dumping to console.
-    bool capture_output = true;
+    // Whether `--dump_output` is specified, causing `Run` output to go to the
+    // console. Output is typically captured for tests and autoupdate.
+    bool dump_output = false;
 
     // stdout and stderr based on CHECK lines in the file.
     llvm::SmallVector<testing::Matcher<std::string>> expected_stdout;
@@ -182,6 +188,11 @@ class FileTestBase : public testing::Test {
   // Runs the FileTestAutoupdater, returning the result.
   auto RunAutoupdater(const TestContext& context, bool dry_run) -> bool;
 
+  // An optional mutex for output. If provided, it will be locked during `Run`
+  // when stderr/stdout are being captured (SET-CAPTURE-CONSOLE-OUTPUT), in
+  // order to avoid output conflicts.
+  std::mutex* output_mutex_;
+
   llvm::StringRef test_name_;
 };
 
@@ -190,8 +201,10 @@ struct FileTestFactory {
   // The test fixture name.
   const char* name;
 
-  // A factory function for tests.
+  // A factory function for tests. The output_mutex is optional; see
+  // `FileTestBase::output_mutex_`.
   std::function<FileTestBase*(llvm::StringRef exe_path,
+                              std::mutex* output_mutex,
                               llvm::StringRef test_name)>
       factory_fn;
 };
@@ -207,11 +220,12 @@ struct FileTestFactory {
 extern auto GetFileTestFactory() -> FileTestFactory;
 
 // Provides a standard GetFileTestFactory implementation.
-#define CARBON_FILE_TEST_FACTORY(Name)                                       \
-  auto GetFileTestFactory() -> FileTestFactory {                             \
-    return {#Name, [](llvm::StringRef exe_path, llvm::StringRef test_name) { \
-              return new Name(exe_path, test_name);                          \
-            }};                                                              \
+#define CARBON_FILE_TEST_FACTORY(Name)                                    \
+  auto GetFileTestFactory() -> FileTestFactory {                          \
+    return {#Name, [](llvm::StringRef exe_path, std::mutex* output_mutex, \
+                      llvm::StringRef test_name) {                        \
+              return new Name(exe_path, output_mutex, test_name);         \
+            }};                                                           \
   }
 
 }  // namespace Carbon::Testing

testing/file_test/file_test_base_test.cpp

@@ -17,8 +17,9 @@ namespace {
 
 class FileTestBaseTest : public FileTestBase {
  public:
-  FileTestBaseTest(llvm::StringRef /*exe_path*/, llvm::StringRef test_name)
-      : FileTestBase(test_name) {}
+  FileTestBaseTest(llvm::StringRef /*exe_path*/, std::mutex* output_mutex,
+                   llvm::StringRef test_name)
+      : FileTestBase(output_mutex, test_name) {}
 
   auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
            llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
@@ -108,6 +109,16 @@ static auto TestAlternatingFiles(TestParams& params)
   return {{.success = true}};
 }
 
+// Does printing and returns expected results for capture_console_output.carbon.
+static auto TestCaptureConsoleOutput(TestParams& params)
+    -> ErrorOr<FileTestBaseTest::RunResult> {
+  llvm::errs() << "llvm::errs\n";
+  params.stderr << "params.stderr\n";
+  llvm::outs() << "llvm::outs\n";
+  params.stdout << "params.stdout\n";
+  return {{.success = true}};
+}
+
 // Does printing and returns expected results for example.carbon.
 static auto TestExample(TestParams& params)
     -> ErrorOr<FileTestBaseTest::RunResult> {
@@ -225,6 +236,7 @@ auto FileTestBaseTest::Run(const llvm::SmallVector<llvm::StringRef>& test_args,
       llvm::StringSwitch<std::function<ErrorOr<RunResult>(TestParams&)>>(
           filename.string())
           .Case("alternating_files.carbon", &TestAlternatingFiles)
+          .Case("capture_console_output.carbon", &TestCaptureConsoleOutput)
           .Case("example.carbon", &TestExample)
           .Case("fail_example.carbon", &TestFailExample)
           .Case("file_only_re_one_file.carbon", &TestFileOnlyREOneFile)

testing/file_test/testdata/capture_console_output.carbon

@@ -0,0 +1,16 @@
+// 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
+
+// SET-CAPTURE-CONSOLE-OUTPUT
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //testing/file_test:file_test_base_test --test_arg=--file_tests=testing/file_test/testdata/capture_console_output.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //testing/file_test:file_test_base_test -- --dump_output --file_tests=testing/file_test/testdata/capture_console_output.carbon
+// CHECK:STDERR: params.stderr
+// CHECK:STDERR: llvm::errs
+
+// CHECK:STDOUT: 2 args: `default_args`, `capture_console_output.carbon`
+// CHECK:STDOUT: params.stdout
+// CHECK:STDOUT: llvm::outs

toolchain/driver/BUILD

@@ -10,7 +10,10 @@ package(default_visibility = ["//visibility:public"])
 
 filegroup(
     name = "testdata",
-    data = glob(["testdata/**/*.carbon"]),
+    data = glob([
+        "testdata/**/*.carbon",
+        "testdata/**/*.cpp",
+    ]),
 )
 
 cc_library(

toolchain/driver/clang_runner_test.cpp

@@ -152,23 +152,6 @@ TEST(ClangRunnerTest, LinkCommandEcho) {
   EXPECT_THAT(out, StrEq(""));
 }
 
-TEST(ClangRunnerTest, NoArgs) {
-  const auto install_paths =
-      InstallPaths::MakeForBazelRunfiles(Testing::GetExePath());
-  std::string verbose_out;
-  llvm::raw_string_ostream verbose_os(verbose_out);
-  std::string target = llvm::sys::getDefaultTargetTriple();
-  ClangRunner runner(&install_paths, target, &verbose_os);
-  std::string out;
-  std::string err;
-  EXPECT_FALSE(RunWithCapturedOutput(out, err, [&] { return runner.Run({}); }))
-      << "Verbose output from runner:\n"
-      << verbose_out << "\n";
-
-  EXPECT_THAT(out, StrEq(""));
-  EXPECT_THAT(err, HasSubstr("error: no input files"));
-}
-
 TEST(ClangRunnerTest, DashC) {
   std::filesystem::path test_file =
       WriteTestFile("test.cpp", "int test() { return 0; }");

toolchain/driver/testdata/fail_clang_no_args.cpp

@@ -0,0 +1,14 @@
+// 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
+//
+// ARGS: clang --
+//
+// SET-CAPTURE-CONSOLE-OUTPUT
+// clang-format off
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_clang_no_args.cpp
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_clang_no_args.cpp
+// CHECK:STDERR: error: no input files

toolchain/testing/file_test.cpp

@@ -23,9 +23,9 @@ namespace {
 // phase subdirectories.
 class ToolchainFileTest : public FileTestBase {
  public:
-  explicit ToolchainFileTest(llvm::StringRef exe_path,
+  explicit ToolchainFileTest(llvm::StringRef exe_path, std::mutex* output_mutex,
                              llvm::StringRef test_name)
-      : FileTestBase(test_name),
+      : FileTestBase(output_mutex, test_name),
         component_(GetComponent(test_name)),
         installation_(InstallPaths::MakeForBazelRunfiles(exe_path)) {}