diff --git a/testing/file_test/autoupdate.cpp b/testing/file_test/autoupdate.cpp index 96e3cccc03aa..a7a441da896a 100644 --- a/testing/file_test/autoupdate.cpp +++ b/testing/file_test/autoupdate.cpp @@ -258,7 +258,7 @@ auto FileTestAutoupdater::StartSplitFile() -> void { ++non_check_line_; } -auto FileTestAutoupdater::Run() -> bool { +auto FileTestAutoupdater::Run(bool dry_run) -> bool { bool any_attached_stdout_lines = std::any_of( stdout_.lines.begin(), stdout_.lines.end(), [&](const CheckLine& line) { return line.line_number() != -1; }); @@ -330,8 +330,10 @@ auto FileTestAutoupdater::Run() -> bool { if (new_content == input_content_) { return false; } - std::ofstream out(file_test_path_); - out << new_content; + if (!dry_run) { + std::ofstream out(file_test_path_); + out << new_content; + } return true; } diff --git a/testing/file_test/autoupdate.h b/testing/file_test/autoupdate.h index 2d9971cc0e4e..173c6b8e0c9a 100644 --- a/testing/file_test/autoupdate.h +++ b/testing/file_test/autoupdate.h @@ -65,8 +65,9 @@ class FileTestAutoupdater { } } - // Automatically updates CHECKs in the provided file. Returns true if updated. - auto Run() -> bool; + // Automatically updates CHECKs in the provided file when dry_run=false. + // Returns true if generated file content differs from actual file content. + auto Run(bool dry_run) -> bool; private: // The file and line number that a CHECK line refers to, and the diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index e020132d61a4..c5eb7f3783ab 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -80,7 +80,19 @@ auto FileTestBase::TestBody() -> void { << "Tests should be prefixed with `fail_` if and only if running them " "is expected to fail."; - // Check results. + // Check results. Include a reminder of the autoupdate command for any + // stdout/stderr differences. + std::string update_message; + if (context.autoupdate_line_number) { + update_message = llvm::formatv( + "If these differences are expected, try the autoupdater:\n" + "\tbazel run {0} -- --autoupdate --file_tests={1}", + target, test_name_); + } else { + update_message = + "If these differences are expected, content must be updated manually."; + } + SCOPED_TRACE(update_message); if (context.check_subset) { EXPECT_THAT(SplitOutput(context.stdout), IsSupersetOf(context.expected_stdout)); @@ -93,19 +105,17 @@ auto FileTestBase::TestBody() -> void { EXPECT_THAT(SplitOutput(context.stderr), ElementsAreArray(context.expected_stderr)); } + + // If there are no other test failures, check if autoupdate would make + // changes. We don't do this when there _are_ failures because the + // SCOPED_TRACE already contains the autoupdate reminder. + if (!HasFailure() && RunAutoupdater(context, /*dry_run=*/true)) { + ADD_FAILURE() << "Autoupdate would make changes to the file content."; + } } -auto FileTestBase::Autoupdate() -> ErrorOr { - // Add a crash trace entry mentioning which file we're updating. - llvm::PrettyStackTraceFormat stack_trace_entry("performing autoupdate for %s", - test_name_); - - TestContext context; - auto run_result = ProcessTestFileAndRun(context); - if (!run_result.ok()) { - return ErrorBuilder() << "Error updating " << test_name_ << ": " - << run_result.error(); - } +auto FileTestBase::RunAutoupdater(const TestContext& context, bool dry_run) + -> bool { if (!context.autoupdate_line_number) { return false; } @@ -132,7 +142,21 @@ auto FileTestBase::Autoupdate() -> ErrorOr { GetDefaultFileRE(expected_filenames), GetLineNumberReplacements(expected_filenames), [&](std::string& line) { DoExtraCheckReplacements(line); }) - .Run(); + .Run(dry_run); +} + +auto FileTestBase::Autoupdate() -> ErrorOr { + // Add a crash trace entry mentioning which file we're updating. + llvm::PrettyStackTraceFormat stack_trace_entry("performing autoupdate for %s", + test_name_); + + TestContext context; + auto run_result = ProcessTestFileAndRun(context); + if (!run_result.ok()) { + return ErrorBuilder() << "Error updating " << test_name_ << ": " + << run_result.error(); + } + return RunAutoupdater(context, /*dry_run=*/false); } auto FileTestBase::GetLineNumberReplacements( diff --git a/testing/file_test/file_test_base.h b/testing/file_test/file_test_base.h index 37fd9fb088ed..9691c2c1ccee 100644 --- a/testing/file_test/file_test_base.h +++ b/testing/file_test/file_test_base.h @@ -151,6 +151,9 @@ class FileTestBase : public testing::Test { static auto TransformExpectation(int line_index, llvm::StringRef in) -> ErrorOr>; + // Runs the FileTestAutoupdater, returning the result. + auto RunAutoupdater(const TestContext& context, bool dry_run) -> bool; + llvm::StringRef test_name_; };