Make the file_test binary work without custom environment variables. (#5442)

Instead of crashing when run outside of `bazel`, make the toolchain's
`file_test` binary work properly when no test-specific environment
variables are set. This makes it a lot easier to run `file_test` under a
debugger.

There are two main changes here:

- Don't crash if `$TEST_TMPDIR` is unset. Instead, fall back to LLVM's
temporary directory (typically `$TMPDIR`). We already did this in some
places in tests. We now do it in more places.
- Don't fall back to a target label of `<target>` in the reproduction
commands if `$TEST_TARGET` is unset, because this causes all the tests
to fail because their output doesn't match the expected output due to a
differing bazel run command. Instead explicitly specify the target from
the `FileTestBase`-derived class.

Infrastructure for this has been added generally, but only rolled out to
the toolchain `file_test` binary for now.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

testing/base/file_helpers.cpp

@@ -14,6 +14,14 @@
 
 namespace Carbon::Testing {
 
+auto GetTempDirectory() -> std::filesystem::path {
+  if (char* tmpdir_env = getenv("TEST_TMPDIR"); tmpdir_env != nullptr) {
+    return tmpdir_env;
+  } else {
+    return std::filesystem::temp_directory_path();
+  }
+}
+
 auto ReadFile(std::filesystem::path path) -> ErrorOr<std::string> {
   std::ifstream file_stream(path);
   if (file_stream.fail()) {
@@ -29,13 +37,7 @@ auto ReadFile(std::filesystem::path path) -> ErrorOr<std::string> {
 
 auto WriteTestFile(llvm::StringRef name, llvm::StringRef contents)
     -> ErrorOr<std::filesystem::path> {
-  std::filesystem::path test_tmpdir;
-  if (char* tmpdir_env = getenv("TEST_TMPDIR"); tmpdir_env != nullptr) {
-    test_tmpdir = std::string(tmpdir_env);
-  } else {
-    test_tmpdir = std::filesystem::temp_directory_path();
-  }
-
+  std::filesystem::path test_tmpdir = GetTempDirectory();
   const auto* unit_test = ::testing::UnitTest::GetInstance();
   const auto* test_info = unit_test->current_test_info();
   std::filesystem::path test_file =

testing/base/file_helpers.h

@@ -12,6 +12,10 @@
 
 namespace Carbon::Testing {
 
+// Returns a directory that should be used to hold temporary files created by
+// test execution.
+auto GetTempDirectory() -> std::filesystem::path;
+
 // Reads a file to string.
 auto ReadFile(std::filesystem::path path) -> ErrorOr<std::string>;
 

testing/file_test/BUILD

@@ -19,6 +19,7 @@ cc_library(
         "//common:check",
         "//common:ostream",
         "//common:raw_string_ostream",
+        "//testing/base:file_helpers",
         "@abseil-cpp//absl/strings",
         "@abseil-cpp//absl/strings:string_view",
         "@llvm-project//llvm:Support",

testing/file_test/autoupdate.cpp

@@ -16,6 +16,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "testing/base/file_helpers.h"
 
 namespace Carbon::Testing {
 
@@ -149,10 +150,9 @@ auto FileTestAutoupdater::BuildCheckLines(llvm::StringRef output,
     file_to_number_map.insert({name, number});
   }
 
-  // %t substitution means we may see TEST_TMPDIR in output.
-  char* tmpdir_env = getenv("TEST_TMPDIR");
-  CARBON_CHECK(tmpdir_env != nullptr);
-  llvm::StringRef tmpdir = tmpdir_env;
+  // %t substitution means we may see the temporary directory's path in output.
+  std::filesystem::path tmpdir_path = GetTempDirectory();
+  llvm::StringRef tmpdir = tmpdir_path.native();
 
   llvm::SmallVector<llvm::StringRef> lines(llvm::split(output, '\n'));
   // It's typical that output ends with a newline, but we don't want to add a
@@ -190,7 +190,7 @@ auto FileTestAutoupdater::BuildCheckLines(llvm::StringRef output,
       check_line.append("{{}}");
     }
 
-    // Ignore TEST_TMPDIR in output.
+    // Ignore mentions of the temporary directory in output.
     if (auto pos = check_line.find(tmpdir); pos != std::string::npos) {
       check_line.replace(pos, tmpdir.size(), "{{.+}}");
     }

testing/file_test/file_test_base.cpp

@@ -78,7 +78,7 @@ struct FileTestInfo {
   std::string test_name;
 
   // A factory function for creating the test object.
-  std::function<auto()->FileTestBase*> factory_fn;
+  std::function<auto()->std::unique_ptr<FileTestBase>> factory_fn;
 
   // gtest's information about the test.
   ::testing::TestInfo* registered_test;
@@ -134,21 +134,11 @@ static auto CompareFailPrefix(llvm::StringRef filename, bool success) -> void {
   }
 }
 
-// Modes for GetBazelCommand.
-enum class BazelMode : uint8_t {
-  Autoupdate,
-  Dump,
-  Test,
-};
-
 // Returns the requested bazel command string for the given execution mode.
-static auto GetBazelCommand(BazelMode mode, llvm::StringRef test_name)
-    -> std::string {
+auto FileTestBase::GetBazelCommand(BazelMode mode) -> std::string {
   RawStringOstream args;
-
-  const char* target = getenv("TEST_TARGET");
   args << "bazel " << ((mode == BazelMode::Test) ? "test" : "run") << " "
-       << (target ? target : "<target>") << " ";
+       << GetBazelLabel() << " ";
 
   switch (mode) {
     case BazelMode::Autoupdate:
@@ -165,10 +155,15 @@ static auto GetBazelCommand(BazelMode mode, llvm::StringRef test_name)
   }
 
   args << "--file_tests=";
-  args << test_name;
+  args << test_name();
   return args.TakeStr();
 }
 
+auto FileTestBase::GetBazelLabel() -> std::string {
+  const char* target = getenv("TEST_TARGET");
+  return target ? target : "<target>";
+}
+
 // Runs the FileTestAutoupdater, returning the result.
 static auto RunAutoupdater(FileTestBase* test_base, const TestFile& test_file,
                            bool dry_run) -> bool {
@@ -193,8 +188,8 @@ static auto RunAutoupdater(FileTestBase* test_base, const TestFile& test_file,
 
   return FileTestAutoupdater(
              std::filesystem::absolute(test_base->test_name().str()),
-             GetBazelCommand(BazelMode::Test, test_base->test_name()),
-             GetBazelCommand(BazelMode::Dump, test_base->test_name()),
+             test_base->GetBazelCommand(FileTestBase::BazelMode::Test),
+             test_base->GetBazelCommand(FileTestBase::BazelMode::Dump),
              test_file.input_content, filenames,
              *test_file.autoupdate_line_number, test_file.autoupdate_split,
              test_file.non_check_lines, test_file.actual_stdout,
@@ -268,7 +263,8 @@ auto FileTestCase::TestBody() -> void {
 
   if (HasFailure()) {
     llvm::errs() << "\nTo test this file alone, run:\n  "
-                 << GetBazelCommand(BazelMode::Test, test_info_->test_name)
+                 << test_info_->factory_fn()->GetBazelCommand(
+                        FileTestBase::BazelMode::Test)
                  << "\n\n";
     if (!test_file.autoupdate_line_number) {
       llvm::errs() << "\nThis test is NOAUTOUPDATE.\n\n";
@@ -276,8 +272,8 @@ auto FileTestCase::TestBody() -> void {
   }
   if (test_info_->autoupdate_differs) {
     ADD_FAILURE() << "Autoupdate would make changes to the file content. Run:\n"
-                  << GetBazelCommand(BazelMode::Autoupdate,
-                                     test_info_->test_name);
+                  << test_info_->factory_fn()->GetBazelCommand(
+                         FileTestBase::BazelMode::Autoupdate);
   }
 }
 
@@ -372,7 +368,8 @@ static auto RunSingleTestHelper(FileTestInfo& test, FileTestBase& test_instance)
     -> void {
   Timer timer;
   // Add a crash trace entry with the single-file test command.
-  std::string test_command = GetBazelCommand(BazelMode::Test, test.test_name);
+  std::string test_command =
+      test_instance.GetBazelCommand(FileTestBase::BazelMode::Test);
   llvm::PrettyStackTraceString stack_trace_entry(test_command.c_str());
 
   if (auto err = RunTestFile(test_instance, absl::GetFlag(FLAGS_dump_output),

testing/file_test/file_test_base.h

@@ -108,6 +108,19 @@ class FileTestBase {
   virtual auto AllowParallelRun() const -> bool { return true; }
 
   // Returns the name of the test (relative to the repo root).
+  // Returns a bazel label that can be used to invoke this test.
+  virtual auto GetBazelLabel() -> std::string;
+
+  // Modes for GetBazelCommand.
+  enum class BazelMode : uint8_t {
+    Autoupdate,
+    Dump,
+    Test,
+  };
+
+  // Returns the requested bazel command string for the given execution mode.
+  auto GetBazelCommand(BazelMode mode) -> std::string;
+
   auto test_name() const -> llvm::StringRef { return test_name_; }
 
  private:
@@ -120,8 +133,8 @@ struct FileTestFactory {
   const char* name;
 
   // A factory function for tests.
-  std::function<
-      auto(llvm::StringRef exe_path, llvm::StringRef test_name)->FileTestBase*>
+  std::function<auto(llvm::StringRef exe_path, llvm::StringRef test_name)
+                    ->std::unique_ptr<FileTestBase>>
       factory_fn;
 };
 
@@ -139,7 +152,7 @@ extern auto GetFileTestFactory() -> FileTestFactory;
 #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);                          \
+              return std::make_unique<Name>(exe_path, test_name);            \
             }};                                                              \
   }
 

testing/file_test/run_test.cpp

@@ -14,6 +14,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "testing/base/file_helpers.h"
 #include "testing/file_test/file_test_base.h"
 #include "testing/file_test/test_file.h"
 
@@ -62,8 +63,7 @@ static auto DoArgReplacements(llvm::SmallVector<std::string>& test_args,
         break;
       }
       case 't': {
-        char* tmpdir = getenv("TEST_TMPDIR");
-        CARBON_CHECK(tmpdir != nullptr);
+        std::filesystem::path tmpdir = GetTempDirectory();
         it->replace(percent, 2, llvm::formatv("{0}/temp_file", tmpdir));
         break;
       }