When running FileTests, verify that autoupdate wouldn't make changes. (#3232)

Trying to more proactively catch when autoupdate is missed. Most of the
execution time of these tests should be in running the program under
test, not processing output, so this should have marginal overhead in
order to produce a useful reminder.

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;
 }
 

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

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));
   }
-}
-
-auto FileTestBase::Autoupdate() -> ErrorOr<bool> {
-  // 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();
+  // 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::RunAutoupdater(const TestContext& context, bool dry_run)
+    -> bool {
   if (!context.autoupdate_line_number) {
     return false;
   }
@@ -132,7 +142,21 @@ auto FileTestBase::Autoupdate() -> ErrorOr<bool> {
              GetDefaultFileRE(expected_filenames),
              GetLineNumberReplacements(expected_filenames),
              [&](std::string& line) { DoExtraCheckReplacements(line); })
-      .Run();
+      .Run(dry_run);
+}
+
+auto FileTestBase::Autoupdate() -> ErrorOr<bool> {
+  // 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(

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<testing::Matcher<std::string>>;
 
+  // Runs the FileTestAutoupdater, returning the result.
+  auto RunAutoupdater(const TestContext& context, bool dry_run) -> bool;
+
   llvm::StringRef test_name_;
 };