Provide local paths for file tests. (#2830)

The intent of this change is that instead of paths looking like `explorer/testdata/foo/bar.carbon` (repo-relative), they're now just `bar.carbon` (local). The consequence is that paths should be a bit more durable in various environments, and just paths should be shorter and easier to read.

The explorer's prelude is an exception to this since it comes from data, rather than being the test target. Due to the change in approaches, it needs the regex again.

Uses #2829

testing/file_test/BUILD

@@ -19,7 +19,7 @@ cc_library(
 )
 
 file_test(
-    name = "file_test",
+    name = "file_test_base_test",
     srcs = ["file_test_base_test.cpp"],
     tests = ["example.carbon"],
     deps = [

testing/file_test/file_test_base.cpp

@@ -4,27 +4,49 @@
 
 #include "testing/file_test/file_test_base.h"
 
+#include <filesystem>
 #include <fstream>
 
 #include "common/check.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/InitLLVM.h"
 
-static std::string* subset_target = nullptr;
-
 namespace Carbon::Testing {
 
+// The length of the base directory.
+static int base_dir_len = 0;
+// The name of the `.subset` target.
+static std::string* subset_target = nullptr;
+// The original working directory for restoration after each test.
+static std::filesystem::path* orig_working_dir = nullptr;
+
 using ::testing::Eq;
 
+FileTestBase::FileTestBase(const std::filesystem::path& path) : path_(&path) {
+  // Run from the file's parent directory.
+  std::error_code ec;
+  std::filesystem::current_path(path.parent_path(), ec);
+  CARBON_CHECK(!ec) << ec.message();
+}
+
+FileTestBase::~FileTestBase() {
+  // Restore the original working directory.
+  std::error_code ec;
+  std::filesystem::current_path(*orig_working_dir, ec);
+  CARBON_CHECK(!ec) << ec.message();
+}
+
 void FileTestBase::RegisterTests(
-    const char* fixture_label, const std::vector<llvm::StringRef>& paths,
-    std::function<FileTestBase*(llvm::StringRef)> factory) {
+    const char* fixture_label, const std::vector<std::filesystem::path>& paths,
+    std::function<FileTestBase*(const std::filesystem::path&)> factory) {
   // Use RegisterTest instead of INSTANTIATE_TEST_CASE_P because of ordering
   // issues between container initialization and test instantiation by
   // InitGoogleTest.
-  for (auto path : paths) {
-    testing::RegisterTest(fixture_label, path.data(), nullptr, path.data(),
-                          __FILE__, __LINE__, [=]() { return factory(path); });
+  for (const auto& path : paths) {
+    std::string test_name = path.string().substr(base_dir_len);
+    testing::RegisterTest(fixture_label, test_name.c_str(), nullptr,
+                          test_name.c_str(), __FILE__, __LINE__,
+                          [=]() { return factory(path); });
   }
 }
 
@@ -48,7 +70,7 @@ auto FileTestBase::TestBody() -> void {
   // Load expected output.
   std::vector<testing::Matcher<std::string>> expected_stdout;
   std::vector<testing::Matcher<std::string>> expected_stderr;
-  std::ifstream file_content(path_.str());
+  std::ifstream file_content(path());
   int line_index = 0;
   std::string line_str;
   while (std::getline(file_content, line_str)) {
@@ -84,7 +106,8 @@ auto FileTestBase::TestBody() -> void {
   if (HasFailure()) {
     return;
   }
-  EXPECT_THAT(!filename().starts_with("fail_"), Eq(run_succeeded))
+  EXPECT_THAT(!llvm::StringRef(path().filename()).starts_with("fail_"),
+              Eq(run_succeeded))
       << "Tests should be prefixed with `fail_` if and only if running them "
          "is expected to fail.";
 
@@ -175,31 +198,8 @@ auto FileTestBase::TransformExpectation(int line_index, llvm::StringRef in)
   return testing::MatchesRegex(str);
 }
 
-auto FileTestBase::filename() -> llvm::StringRef {
-  auto last_slash = path_.rfind("/");
-  if (last_slash == llvm::StringRef::npos) {
-    return path_;
-  } else {
-    return path_.substr(last_slash + 1);
-  }
-}
-
 }  // namespace Carbon::Testing
 
-// Returns the name of the subset target.
-static auto GetSubsetTarget() -> std::string {
-  char* name = getenv("TEST_TARGET");
-  if (name == nullptr) {
-    return "<missing TEST_TARGET>";
-  }
-
-  if (llvm::StringRef(name).ends_with(".subset")) {
-    return name;
-  } else {
-    return std::string(name) + ".subset";
-  }
-}
-
 auto main(int argc, char** argv) -> int {
   testing::InitGoogleTest(&argc, argv);
   llvm::setBugReportMsg(
@@ -213,10 +213,42 @@ auto main(int argc, char** argv) -> int {
     return EXIT_FAILURE;
   }
 
-  std::string subset_target_storage = GetSubsetTarget();
-  ::subset_target = &subset_target_storage;
+  const char* target = getenv("TEST_TARGET");
+  CARBON_CHECK(target != nullptr);
 
-  std::vector<llvm::StringRef> paths(argv + 1, argv + argc);
+  // Configure the name of the subset target.
+  std::string subset_target_storage = target;
+  static constexpr char SubsetSuffix[] = ".subset";
+  if (!llvm::StringRef(subset_target_storage).ends_with(SubsetSuffix)) {
+    subset_target_storage += SubsetSuffix;
+  }
+  Carbon::Testing::subset_target = &subset_target_storage;
+
+  // Save the working directory for later restoration.
+  std::error_code ec;
+  std::filesystem::path orig_working_dir_storage =
+      std::filesystem::current_path(ec);
+  CARBON_CHECK(!ec) << ec.message();
+  Carbon::Testing::orig_working_dir = &orig_working_dir_storage;
+
+  // Configure the base directory for test names.
+  llvm::StringRef target_dir = target;
+  // Leaves one slash.
+  CARBON_CHECK(target_dir.consume_front("/"));
+  target_dir = target_dir.substr(0, target_dir.rfind(":"));
+  std::string base_dir =
+      orig_working_dir_storage.string() + target_dir.str() + "/";
+  Carbon::Testing::base_dir_len = base_dir.size();
+
+  // Register tests based on their absolute path.
+  std::vector<std::filesystem::path> paths;
+  for (int i = 1; i < argc; ++i) {
+    auto path = std::filesystem::absolute(argv[i], ec);
+    CARBON_CHECK(!ec) << argv[i] << ": " << ec.message();
+    CARBON_CHECK(llvm::StringRef(path.string()).starts_with(base_dir))
+        << "\n  " << path << "\n  should start with\n  " << base_dir;
+    paths.push_back(path);
+  }
   Carbon::Testing::RegisterFileTests(paths);
 
   return RUN_ALL_TESTS();

testing/file_test/file_test_base.h

@@ -8,6 +8,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <filesystem>
 #include <functional>
 #include <vector>
 
@@ -32,12 +33,14 @@ namespace Carbon::Testing {
 // `lit_autoupdate.py` automatically constructs compatible lines.
 class FileTestBase : public testing::Test {
  public:
-  explicit FileTestBase(const llvm::StringRef path) : path_(path) {}
+  explicit FileTestBase(const std::filesystem::path& path);
+  ~FileTestBase() override;
 
   // Used by children to register tests with gtest.
   static void RegisterTests(
-      const char* fixture_label, const std::vector<llvm::StringRef>& paths,
-      std::function<FileTestBase*(llvm::StringRef)> factory);
+      const char* fixture_label,
+      const std::vector<std::filesystem::path>& paths,
+      std::function<FileTestBase*(const std::filesystem::path&)> factory);
 
   // Implemented by children to run the test. Called by the TestBody
   // implementation, which will validate stdout and stderr. The return value
@@ -49,11 +52,8 @@ class FileTestBase : public testing::Test {
   // issues are a little easier to identify by the different line.
   auto TestBody() -> void final;
 
-  // Returns the filename of the file being tested.
-  auto filename() -> llvm::StringRef;
-
   // Returns the full path of the file being tested.
-  auto path() -> llvm::StringRef { return path_; };
+  auto path() -> const std::filesystem::path& { return *path_; };
 
  private:
   // Transforms an expectation on a given line from `FileCheck` syntax into a
@@ -61,11 +61,11 @@ class FileTestBase : public testing::Test {
   static auto TransformExpectation(int line_index, llvm::StringRef in)
       -> testing::Matcher<std::string>;
 
-  llvm::StringRef path_;
+  const std::filesystem::path* path_;
 };
 
 // Must be implemented by the individual file_test to initialize tests.
-extern auto RegisterFileTests(const std::vector<llvm::StringRef>& paths)
+extern auto RegisterFileTests(const std::vector<std::filesystem::path>& paths)
     -> void;
 
 }  // namespace Carbon::Testing

testing/file_test/file_test_base_test.cpp

@@ -17,11 +17,13 @@ namespace {
 
 class FileTestBaseTest : public FileTestBase {
  public:
-  explicit FileTestBaseTest(llvm::StringRef path) : FileTestBase(path) {}
+  explicit FileTestBaseTest(const std::filesystem::path& path)
+      : FileTestBase(path) {}
 
   auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr)
       -> bool override {
-    if (filename() == "example.carbon") {
+    auto filename = path().filename();
+    if (filename == "example.carbon") {
       stdout << "something\n"
                 "\n"
                 "8: Line delta\n"
@@ -29,11 +31,11 @@ class FileTestBaseTest : public FileTestBase {
                 "+*[]{}\n"
                 "Foo baz\n";
       return true;
-    } else if (filename() == "fail_example.carbon") {
+    } else if (filename == "fail_example.carbon") {
       stderr << "Oops\n";
       return false;
     } else {
-      ADD_FAILURE() << "Unexpected file: " << path().str();
+      ADD_FAILURE() << "Unexpected file: " << filename;
       return false;
     }
   }
@@ -41,10 +43,12 @@ class FileTestBaseTest : public FileTestBase {
 
 }  // namespace
 
-auto RegisterFileTests(const std::vector<llvm::StringRef>& paths) -> void {
-  FileTestBaseTest::RegisterTests(
-      "FileTestBaseTest", paths,
-      [](llvm::StringRef path) { return new FileTestBaseTest(path); });
+auto RegisterFileTests(const std::vector<std::filesystem::path>& paths)
+    -> void {
+  FileTestBaseTest::RegisterTests("FileTestBaseTest", paths,
+                                  [](const std::filesystem::path& path) {
+                                    return new FileTestBaseTest(path);
+                                  });
 }
 
 }  // namespace Carbon::Testing

testing/scripts/autoupdate_testdata_base.py

@@ -87,7 +87,7 @@ def parse_args() -> ParsedArgs:
     parser.add_argument(
         "--line_number_pattern",
         metavar="PATTERN",
-        default=r"(?P<prefix>/(?P<filename>\w+\.carbon):)"
+        default=r"(?P<prefix>(?P<filename>\w+\.carbon):)"
         r"(?P<line>\d+)(?P<suffix>(?:\D|$))",
         help="A regular expression which matches line numbers to update as its "
         "only group. Capture groups 'prefix', 'line', and 'suffix' are "
@@ -304,13 +304,16 @@ def get_matchable_test_output(
     """Runs the autoupdate command and returns the output lines."""
     # Run the autoupdate command to generate output.
     # (`bazel run` would serialize)
-    autoupdate_cmd = TOOLS[tool].replace("//", "./bazel-bin/").replace(":", "/")
+    autoupdate_cmd = Path.cwd().joinpath(
+        TOOLS[tool].replace("//", "./bazel-bin/").replace(":", "/")
+    )
     p = subprocess.run(
-        [autoupdate_cmd] + autoupdate_args + [test],
+        [str(autoupdate_cmd)] + autoupdate_args + [Path(test).name],
         env={"LLVM_SYMBOLIZER_PATH": llvm_symbolizer},
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
         encoding="utf-8",
+        cwd=str(Path(test).parent),
     )
 
     out_lines = label_output("STDOUT:", p.stdout)
@@ -324,10 +327,7 @@ def get_matchable_test_output(
             # `lit` uses full paths to the test file, so use a regex to ignore
             # paths when used.
             line = line.replace(test, f"{{{{.*}}}}/{test}")
-            line = bazel_runfiles.sub("{{.*}}/", line)
-        else:
-            # When not using `lit`, the runfiles path is removed.
-            line = bazel_runfiles.sub("", line)
+        line = bazel_runfiles.sub("{{.*}}/", line)
 
         for line_matcher, before, after in extra_check_replacements:
             if line_matcher.match(line):