diff --git a/explorer/file_test.cpp b/explorer/file_test.cpp index 90791dcb85be1..55270c6e743b1 100644 --- a/explorer/file_test.cpp +++ b/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()); diff --git a/testing/file_test/README.md b/testing/file_test/README.md index 979f4a64bb51b..faa3ce56de19b 100644 --- a/testing/file_test/README.md +++ b/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 weare 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 ``` diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index a12588eb525d3..7ee11113d4309 100644 --- a/testing/file_test/file_test_base.cpp +++ b/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 { auto FileTestBase::DumpOutput() -> ErrorOr { 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,26 @@ 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_ptr> console_lock; + if (context.capture_console_output) { + if (output_mutex_) { + console_lock = + std::make_unique>(*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 +351,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 +808,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 { - 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 { + 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 +894,14 @@ auto FileTestBase::ProcessTestFile(TestContext& context) -> ErrorOr { } 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 +979,7 @@ static auto RunAutoupdate(llvm::StringRef exe_path, crc.DumpStackAndCleanupOnFailure = true; bool thread_crashed = !crc.RunSafely([&] { std::unique_ptr 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 lock(mutex); @@ -1014,7 +1049,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 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 +1063,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(); diff --git a/testing/file_test/file_test_base.h b/testing/file_test/file_test_base.h index ea8559f6b8606..6cb8c8d6e8b95 100644 --- a/testing/file_test/file_test_base.h +++ b/testing/file_test/file_test_base.h @@ -9,6 +9,7 @@ #include #include +#include #include "common/error.h" #include "common/ostream.h" @@ -64,7 +65,8 @@ class FileTestBase : public testing::Test { llvm::SmallVector> 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 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> 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 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 diff --git a/testing/file_test/file_test_base_test.cpp b/testing/file_test/file_test_base_test.cpp index 3e4ef6887934c..d758904326739 100644 --- a/testing/file_test/file_test_base_test.cpp +++ b/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& 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 { + 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 { @@ -225,6 +236,7 @@ auto FileTestBaseTest::Run(const llvm::SmallVector& test_args, llvm::StringSwitch(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) diff --git a/testing/file_test/testdata/capture_console_output.carbon b/testing/file_test/testdata/capture_console_output.carbon new file mode 100644 index 0000000000000..4b9687a0edabb --- /dev/null +++ b/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 diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index 64a73d60ccc3a..a6925c8ac5c05 100644 --- a/toolchain/driver/BUILD +++ b/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( diff --git a/toolchain/driver/clang_runner_test.cpp b/toolchain/driver/clang_runner_test.cpp index 7b14e03102c4c..9025ea08cf681 100644 --- a/toolchain/driver/clang_runner_test.cpp +++ b/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; }"); diff --git a/toolchain/driver/testdata/fail_clang_no_args.cpp b/toolchain/driver/testdata/fail_clang_no_args.cpp new file mode 100644 index 0000000000000..a131eb9716284 --- /dev/null +++ b/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 +// 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 diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 3fd54476f1e02..c9d13abf4b79b 100644 --- a/toolchain/testing/file_test.cpp +++ b/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)) {}