diff --git a/explorer/file_test.cpp b/explorer/file_test.cpp index 210bf4518340f..b743e7ef86e30 100644 --- a/explorer/file_test.cpp +++ b/explorer/file_test.cpp @@ -35,7 +35,7 @@ class ExplorerFileTest : public FileTestBase { auto Run(const llvm::SmallVector& test_args, llvm::IntrusiveRefCntPtr& fs, FILE* /*input_stream*/, llvm::raw_pwrite_stream& output_stream, - llvm::raw_pwrite_stream& error_stream) + llvm::raw_pwrite_stream& error_stream) const -> ErrorOr override { // Add the prelude. llvm::ErrorOr> prelude = @@ -58,21 +58,22 @@ class ExplorerFileTest : public FileTestBase { args.push_back(arg.data()); } + RawStringOstream trace_stream; int exit_code = ExplorerMain(args.size(), args.data(), /*install_path=*/"", PreludePath, output_stream, error_stream, - check_trace_output() ? output_stream : trace_stream_, *fs); + check_trace_output() ? output_stream : trace_stream, *fs); // Skip trace test check as they use stdout stream instead of // trace_stream_ostream - if (absl::GetFlag(FLAGS_trace) && trace_stream_.TakeStr().empty()) { + if (absl::GetFlag(FLAGS_trace) && trace_stream.TakeStr().empty()) { return Error("Tracing should always do something"); } return {{.success = exit_code == EXIT_SUCCESS}}; } - auto GetDefaultArgs() -> llvm::SmallVector override { + auto GetDefaultArgs() const -> llvm::SmallVector override { llvm::SmallVector args; if (absl::GetFlag(FLAGS_trace)) { args.push_back("--trace_file=-"); @@ -83,14 +84,15 @@ class ExplorerFileTest : public FileTestBase { } auto GetLineNumberReplacements(llvm::ArrayRef filenames) - -> llvm::SmallVector override { + const -> llvm::SmallVector override { if (check_trace_output()) { return {}; } return FileTestBase::GetLineNumberReplacements(filenames); } - auto DoExtraCheckReplacements(std::string& check_line) -> void override { + auto DoExtraCheckReplacements(std::string& check_line) const + -> void override { // Ignore the resulting column of EndOfFile because it's often the end of // the CHECK comment. RE2::GlobalReplace(&check_line, prelude_line_re_, @@ -106,11 +108,10 @@ class ExplorerFileTest : public FileTestBase { private: // Trace output is directly checked for a few tests. - auto check_trace_output() -> bool { + auto check_trace_output() const -> bool { return test_name().find("/trace/") != std::string::npos; } - RawStringOstream trace_stream_; RE2 prelude_line_re_; RE2 timing_re_; }; diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index 607353df09500..701ce6de99b22 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -268,7 +268,7 @@ auto FileTestCase::TestBody() -> void { } auto FileTestBase::GetLineNumberReplacements( - llvm::ArrayRef filenames) + llvm::ArrayRef filenames) const -> llvm::SmallVector { return {{.has_file = true, .re = std::make_shared( @@ -329,88 +329,147 @@ class FileTestEventListener : public testing::EmptyTestEventListener { llvm::MutableArrayRef tests_; }; +// Returns true if the main thread should be used to run tests. This is if +// either --dump_output is specified, or only 1 thread is needed to run tests. +static auto SingleThreaded(llvm::ArrayRef tests) -> bool { + if (absl::GetFlag(FLAGS_dump_output) || absl::GetFlag(FLAGS_threads) == 1) { + return true; + } + + bool found_test_to_run = false; + for (const auto& test : tests) { + if (!test.registered_test->should_run()) { + continue; + } + if (found_test_to_run) { + // At least two tests will run, so multi-threaded. + return false; + } + // Found the first test to run. + found_test_to_run = true; + } + // 0 or 1 test will be run, so single-threaded. + return false; +} + +// Runs the test in the section that would be inside a lock, possibly inside a +// CrashRecoveryContext. +static auto RunSingleTestHelper(FileTestInfo& test, FileTestBase& test_instance) + -> void { + // Add a crash trace entry with the single-file test command. + std::string test_command = GetBazelCommand(BazelMode::Test, test.test_name); + llvm::PrettyStackTraceString stack_trace_entry(test_command.c_str()); + + if (auto err = RunTestFile(test_instance, absl::GetFlag(FLAGS_dump_output), + **test.test_result); + !err.ok()) { + test.test_result = std::move(err).error(); + } +} + +// Runs a single test. Uses a CrashRecoveryContext, and returns false on a +// crash. +static auto RunSingleTest(FileTestInfo& test, bool single_threaded, + std::mutex& output_mutex) -> bool { + std::unique_ptr test_instance(test.factory_fn()); + + if (absl::GetFlag(FLAGS_dump_output)) { + std::unique_lock lock(output_mutex); + llvm::errs() << "\n--- Dumping: " << test.test_name << "\n\n"; + } + + // Load expected output. + test.test_result = ProcessTestFile(test_instance->test_name(), + absl::GetFlag(FLAGS_autoupdate)); + if (test.test_result->ok()) { + // Execution must be serialized for either serial tests or console + // output. + std::unique_lock output_lock; + + if ((*test.test_result)->capture_console_output || + !test_instance->AllowParallelRun()) { + output_lock = std::unique_lock(output_mutex); + } + + if (single_threaded) { + RunSingleTestHelper(test, *test_instance); + } else { + // Use a crash recovery context to try to get a stack trace when + // multiple threads may crash in parallel, which otherwise leads to the + // program aborting without printing a stack trace. + llvm::CrashRecoveryContext crc; + crc.DumpStackAndCleanupOnFailure = true; + if (!crc.RunSafely([&] { RunSingleTestHelper(test, *test_instance); })) { + return false; + } + } + } + + if (!test.test_result->ok()) { + std::unique_lock lock(output_mutex); + llvm::errs() << "\n" << test.test_result->error().message() << "\n"; + return true; + } + + test.autoupdate_differs = + RunAutoupdater(test_instance.get(), **test.test_result, + /*dry_run=*/!absl::GetFlag(FLAGS_autoupdate)); + + std::unique_lock lock(output_mutex); + if (absl::GetFlag(FLAGS_dump_output)) { + llvm::outs().flush(); + const TestFile& test_file = **test.test_result; + llvm::errs() << "\n--- Exit with success: " + << (test_file.run_result.success ? "true" : "false") + << "\n--- Autoupdate differs: " + << (test.autoupdate_differs ? "true" : "false") << "\n"; + } else { + llvm::errs() << (test.autoupdate_differs ? "!" : "."); + } + + return true; +} + auto FileTestEventListener::OnTestProgramStart( const testing::UnitTest& /*unit_test*/) -> void { - llvm::CrashRecoveryContext::Enable(); - llvm::DefaultThreadPool pool( - {.ThreadsRequested = absl::GetFlag(FLAGS_dump_output) - ? 1 - : absl::GetFlag(FLAGS_threads)}); + bool single_threaded = SingleThreaded(tests_); + + std::unique_ptr pool; + if (single_threaded) { + pool = std::make_unique(); + } else { + // Enable the CRC for use in `RunSingleTest`. + llvm::CrashRecoveryContext::Enable(); + pool = std::make_unique(llvm::ThreadPoolStrategy{ + .ThreadsRequested = absl::GetFlag(FLAGS_threads)}); + } if (!absl::GetFlag(FLAGS_dump_output)) { - llvm::errs() << "Running tests with " << pool.getMaxConcurrency() + llvm::errs() << "Running tests with " << pool->getMaxConcurrency() << " thread(s)\n"; } - // Guard access to both `llvm::errs` and `crashed`. - bool crashed = false; + // Guard access to output (stdout and stderr). std::mutex output_mutex; + std::atomic crashed = false; for (auto& test : tests_) { if (!test.registered_test->should_run()) { continue; } - pool.async([&output_mutex, &crashed, &test] { + pool->async([&] { // If any thread crashed, don't try running more. - { - std::unique_lock lock(output_mutex); - if (crashed) { - return; - } + if (crashed) { + return; } - // Use a crash recovery context to try to get a stack trace when - // multiple threads may crash in parallel, which otherwise leads to the - // program aborting without printing a stack trace. - llvm::CrashRecoveryContext crc; - crc.DumpStackAndCleanupOnFailure = true; - bool thread_crashed = !crc.RunSafely([&] { - std::unique_ptr test_instance(test.factory_fn()); - - // Add a crash trace entry with the single-file test command. - std::string test_command = - GetBazelCommand(BazelMode::Test, test.test_name); - llvm::PrettyStackTraceString stack_trace_entry(test_command.c_str()); - - if (absl::GetFlag(FLAGS_dump_output)) { - std::unique_lock lock(output_mutex); - llvm::errs() << "\n--- Dumping: " << test.test_name << "\n\n"; - } - - test.test_result = ProcessTestFileAndRun( - test_instance.get(), &output_mutex, - absl::GetFlag(FLAGS_dump_output), absl::GetFlag(FLAGS_autoupdate)); - - if (!test.test_result->ok()) { - std::unique_lock lock(output_mutex); - llvm::errs() << "\n" << test.test_result->error().message() << "\n"; - return; - } - - test.autoupdate_differs = - RunAutoupdater(test_instance.get(), **test.test_result, - /*dry_run=*/!absl::GetFlag(FLAGS_autoupdate)); - - std::unique_lock lock(output_mutex); - if (absl::GetFlag(FLAGS_dump_output)) { - llvm::outs().flush(); - const TestFile& test_file = **test.test_result; - llvm::errs() << "\n--- Exit with success: " - << (test_file.run_result.success ? "true" : "false") - << "\n--- Autoupdate differs: " - << (test.autoupdate_differs ? "true" : "false") << "\n"; - } else { - llvm::errs() << (test.autoupdate_differs ? "!" : "."); - } - }); - if (thread_crashed) { - std::unique_lock lock(output_mutex); + if (!RunSingleTest(test, single_threaded, output_mutex)) { crashed = true; } }); } - pool.wait(); + pool->wait(); if (crashed) { // Abort rather than returning so that we don't get a LeakSanitizer report. // We expect to have leaked memory if one or more of our tests crashed. diff --git a/testing/file_test/file_test_base.h b/testing/file_test/file_test_base.h index 4cd6d064e7025..7782de5343a2c 100644 --- a/testing/file_test/file_test_base.h +++ b/testing/file_test/file_test_base.h @@ -71,15 +71,15 @@ class FileTestBase { virtual auto Run(const llvm::SmallVector& test_args, llvm::IntrusiveRefCntPtr& fs, FILE* input_stream, llvm::raw_pwrite_stream& output_stream, - llvm::raw_pwrite_stream& error_stream) + llvm::raw_pwrite_stream& error_stream) const -> ErrorOr = 0; // Returns default arguments. Only called when a file doesn't set ARGS. - virtual auto GetDefaultArgs() -> llvm::SmallVector = 0; + virtual auto GetDefaultArgs() const -> llvm::SmallVector = 0; // Returns a map of string replacements to implement `%{key}` -> `value` in // arguments. - virtual auto GetArgReplacements() -> llvm::StringMap { + virtual auto GetArgReplacements() const -> llvm::StringMap { return {}; } @@ -87,18 +87,19 @@ class FileTestBase { // May return nullptr if unused. If GetLineNumberReplacements returns an entry // with has_file=false, this is required. virtual auto GetDefaultFileRE(llvm::ArrayRef /*filenames*/) - -> std::optional { + const -> std::optional { return std::nullopt; } // Returns replacement information for line numbers. See LineReplacement for // construction. virtual auto GetLineNumberReplacements( - llvm::ArrayRef filenames) + llvm::ArrayRef filenames) const -> llvm::SmallVector; // Optionally allows children to provide extra replacements for autoupdate. - virtual auto DoExtraCheckReplacements(std::string& /*check_line*/) -> void {} + virtual auto DoExtraCheckReplacements(std::string& /*check_line*/) const + -> void {} // Whether to allow running the test in parallel, particularly for autoupdate. // This can be overridden to force some tests to be run serially. At any given diff --git a/testing/file_test/file_test_base_test.cpp b/testing/file_test/file_test_base_test.cpp index 5a4ae5edaca8c..8fd0c9e3a5709 100644 --- a/testing/file_test/file_test_base_test.cpp +++ b/testing/file_test/file_test_base_test.cpp @@ -23,25 +23,25 @@ class FileTestBaseTest : public FileTestBase { auto Run(const llvm::SmallVector& test_args, llvm::IntrusiveRefCntPtr& fs, FILE* input_stream, llvm::raw_pwrite_stream& output_stream, - llvm::raw_pwrite_stream& error_stream) + llvm::raw_pwrite_stream& error_stream) const -> ErrorOr override; - auto GetArgReplacements() -> llvm::StringMap override { + auto GetArgReplacements() const -> llvm::StringMap override { return {{"replacement", "replaced"}}; } - auto GetDefaultArgs() -> llvm::SmallVector override { + auto GetDefaultArgs() const -> llvm::SmallVector override { return {"default_args", "%s"}; } - auto GetDefaultFileRE(llvm::ArrayRef filenames) + auto GetDefaultFileRE(llvm::ArrayRef filenames) const -> std::optional override { return std::make_optional( llvm::formatv(R"(file: ({0}))", llvm::join(filenames, "|"))); } auto GetLineNumberReplacements(llvm::ArrayRef filenames) - -> llvm::SmallVector override { + const -> llvm::SmallVector override { auto replacements = FileTestBase::GetLineNumberReplacements(filenames); auto filename = std::filesystem::path(test_name().str()).filename(); if (llvm::StringRef(filename).starts_with("file_only_re_")) { @@ -260,7 +260,7 @@ auto FileTestBaseTest::Run( const llvm::SmallVector& test_args, llvm::IntrusiveRefCntPtr& fs, FILE* input_stream, llvm::raw_pwrite_stream& output_stream, - llvm::raw_pwrite_stream& error_stream) -> ErrorOr { + llvm::raw_pwrite_stream& error_stream) const -> ErrorOr { PrintArgs(test_args, output_stream); auto filename = std::filesystem::path(test_name().str()).filename(); diff --git a/testing/file_test/run_test.cpp b/testing/file_test/run_test.cpp index 9adc0ddb0da7c..ffb83a46113ed 100644 --- a/testing/file_test/run_test.cpp +++ b/testing/file_test/run_test.cpp @@ -27,7 +27,6 @@ static constexpr llvm::StringLiteral StdinFilename = "STDIN"; // Does replacements in ARGS for %s and %t. static auto DoArgReplacements( - llvm::SmallVector& test_args, const llvm::StringMap& replacements, const llvm::SmallVector& split_files) -> ErrorOr { @@ -87,21 +86,15 @@ static auto DoArgReplacements( return Success(); } -auto ProcessTestFileAndRun(FileTestBase* test_base, std::mutex* output_mutex, - bool dump_output, bool running_autoupdate) - -> ErrorOr { - // Load expected output. - CARBON_ASSIGN_OR_RETURN( - TestFile test_file, - ProcessTestFile(test_base->test_name(), running_autoupdate)); - +auto RunTestFile(const FileTestBase& test_base, bool dump_output, + TestFile& test_file) -> ErrorOr { // Process arguments. if (test_file.test_args.empty()) { - test_file.test_args = test_base->GetDefaultArgs(); + test_file.test_args = test_base.GetDefaultArgs(); test_file.test_args.append(test_file.extra_args); } CARBON_RETURN_IF_ERROR(DoArgReplacements(test_file.test_args, - test_base->GetArgReplacements(), + test_base.GetArgReplacements(), test_file.file_splits)); // stdin needs to exist on-disk for compatibility. We'll use a pointer for it. @@ -147,13 +140,6 @@ auto ProcessTestFileAndRun(FileTestBase* test_base, std::mutex* output_mutex, llvm::PrettyStackTraceProgram stack_trace_entry( test_argv_for_stack_trace.size() - 1, test_argv_for_stack_trace.data()); - // Execution must be serialized for either serial tests or console output. - std::unique_lock output_lock; - if (output_mutex && - (test_file.capture_console_output || !test_base->AllowParallelRun())) { - output_lock = std::unique_lock(*output_mutex); - } - // Conditionally capture console output. We use a scope exit to ensure the // captures terminate even on run failures. if (test_file.capture_console_output) { @@ -168,10 +154,10 @@ auto ProcessTestFileAndRun(FileTestBase* test_base, std::mutex* output_mutex, llvm::raw_svector_ostream error_stream(test_file.actual_stderr); ErrorOr run_result = - dump_output ? test_base->Run(test_args_ref, fs, input_stream, - llvm::outs(), llvm::errs()) - : test_base->Run(test_args_ref, fs, input_stream, - output_stream, error_stream); + dump_output ? test_base.Run(test_args_ref, fs, input_stream, llvm::outs(), + llvm::errs()) + : test_base.Run(test_args_ref, fs, input_stream, + output_stream, error_stream); // Ensure stdout/stderr are always fetched, even when discarded on error. if (test_file.capture_console_output) { @@ -185,7 +171,7 @@ auto ProcessTestFileAndRun(FileTestBase* test_base, std::mutex* output_mutex, return std::move(run_result).error(); } test_file.run_result = std::move(*run_result); - return test_file; + return Success(); } } // namespace Carbon::Testing diff --git a/testing/file_test/run_test.h b/testing/file_test/run_test.h index d6025511ef139..1eb0545179d71 100644 --- a/testing/file_test/run_test.h +++ b/testing/file_test/run_test.h @@ -11,11 +11,9 @@ namespace Carbon::Testing { -// Processes the test file and runs the test. Returns an error if something -// went wrong. -auto ProcessTestFileAndRun(FileTestBase* test_base, std::mutex* output_mutex, - bool dump_output, bool running_autoupdate) - -> ErrorOr; +// Runs the test, updating `test_file`. +auto RunTestFile(const FileTestBase& test_base, bool dump_output, + TestFile& test_file) -> ErrorOr; } // namespace Carbon::Testing diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 77f44eea40c5e..bfc5316fc3709 100644 --- a/toolchain/testing/file_test.cpp +++ b/toolchain/testing/file_test.cpp @@ -27,29 +27,29 @@ class ToolchainFileTest : public FileTestBase { llvm::StringRef test_name); // Adds a replacement for `core_package_dir`. - auto GetArgReplacements() -> llvm::StringMap override; + auto GetArgReplacements() const -> llvm::StringMap override; // Loads files into the VFS and runs the driver. auto Run(const llvm::SmallVector& test_args, llvm::IntrusiveRefCntPtr& fs, FILE* input_stream, llvm::raw_pwrite_stream& output_stream, - llvm::raw_pwrite_stream& error_stream) + llvm::raw_pwrite_stream& error_stream) const -> ErrorOr override; // Sets different default flags based on the component being tested. - auto GetDefaultArgs() -> llvm::SmallVector override; + auto GetDefaultArgs() const -> llvm::SmallVector override; // Generally uses the parent implementation, with special handling for lex. - auto GetDefaultFileRE(llvm::ArrayRef filenames) + auto GetDefaultFileRE(llvm::ArrayRef filenames) const -> std::optional override; // Generally uses the parent implementation, with special handling for lex. auto GetLineNumberReplacements(llvm::ArrayRef filenames) - -> llvm::SmallVector override; + const -> llvm::SmallVector override; // Generally uses the parent implementation, with special handling for lex and // driver. - auto DoExtraCheckReplacements(std::string& check_line) -> void override; + auto DoExtraCheckReplacements(std::string& check_line) const -> void override; // Most tests can be run in parallel, but clangd has a global for its logging // system so we need language-server tests to be run in serial. @@ -59,7 +59,7 @@ class ToolchainFileTest : public FileTestBase { private: // Adds a file to the fs. - auto AddFile(llvm::vfs::InMemoryFileSystem& fs, llvm::StringRef path) + auto AddFile(llvm::vfs::InMemoryFileSystem& fs, llvm::StringRef path) const -> ErrorOr; // Controls whether `Run()` includes the prelude. @@ -94,7 +94,8 @@ ToolchainFileTest::ToolchainFileTest(llvm::StringRef exe_path, component_(GetComponent(test_name)), installation_(InstallPaths::MakeForBazelRunfiles(exe_path)) {} -auto ToolchainFileTest::GetArgReplacements() -> llvm::StringMap { +auto ToolchainFileTest::GetArgReplacements() const + -> llvm::StringMap { return {{"core_package_dir", installation_.core_package()}}; } @@ -102,7 +103,7 @@ auto ToolchainFileTest::Run( const llvm::SmallVector& test_args, llvm::IntrusiveRefCntPtr& fs, FILE* input_stream, llvm::raw_pwrite_stream& output_stream, - llvm::raw_pwrite_stream& error_stream) -> ErrorOr { + llvm::raw_pwrite_stream& error_stream) const -> ErrorOr { CARBON_ASSIGN_OR_RETURN(auto prelude, installation_.ReadPreludeManifest()); if (!is_no_prelude()) { for (const auto& file : prelude) { @@ -141,7 +142,8 @@ auto ToolchainFileTest::Run( return result; } -auto ToolchainFileTest::GetDefaultArgs() -> llvm::SmallVector { +auto ToolchainFileTest::GetDefaultArgs() const + -> llvm::SmallVector { llvm::SmallVector args = {"--include-diagnostic-kind"}; if (component_ == "format") { @@ -180,7 +182,7 @@ auto ToolchainFileTest::GetDefaultArgs() -> llvm::SmallVector { } auto ToolchainFileTest::GetDefaultFileRE( - llvm::ArrayRef filenames) -> std::optional { + llvm::ArrayRef filenames) const -> std::optional { if (component_ == "lex") { return std::make_optional( llvm::formatv(R"(^- filename: ({0})$)", llvm::join(filenames, "|"))); @@ -189,7 +191,7 @@ auto ToolchainFileTest::GetDefaultFileRE( } auto ToolchainFileTest::GetLineNumberReplacements( - llvm::ArrayRef filenames) + llvm::ArrayRef filenames) const -> llvm::SmallVector { auto replacements = FileTestBase::GetLineNumberReplacements(filenames); if (component_ == "lex") { @@ -201,7 +203,7 @@ auto ToolchainFileTest::GetLineNumberReplacements( return replacements; } -auto ToolchainFileTest::DoExtraCheckReplacements(std::string& check_line) +auto ToolchainFileTest::DoExtraCheckReplacements(std::string& check_line) const -> void { if (component_ == "driver") { // TODO: Disable token output, it's not interesting for these tests. @@ -221,7 +223,8 @@ auto ToolchainFileTest::DoExtraCheckReplacements(std::string& check_line) } auto ToolchainFileTest::AddFile(llvm::vfs::InMemoryFileSystem& fs, - llvm::StringRef path) -> ErrorOr { + llvm::StringRef path) const + -> ErrorOr { llvm::ErrorOr> file = llvm::MemoryBuffer::getFile(path); if (file.getError()) {