Merge filesystem logic into FileTest and update test files. (#3183)

two_files.carbon has odd STDOUT placement, but will pursue that
separately.

explorer/file_test.cpp

@@ -27,18 +27,8 @@ class ExplorerFileTest : public FileTestBase {
   }
 
   auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-           const llvm::SmallVector<TestFile>& test_files,
-           llvm::raw_pwrite_stream& stdout, llvm::raw_pwrite_stream& stderr)
-      -> ErrorOr<bool> override {
-    // Create the files in-memory.
-    llvm::vfs::InMemoryFileSystem fs;
-    for (const auto& test_file : test_files) {
-      if (!fs.addFile(test_file.filename, /*ModificationTime=*/0,
-                      llvm::MemoryBuffer::getMemBuffer(test_file.content))) {
-        return ErrorBuilder() << "File is repeated: " << test_file.filename;
-      }
-    }
-
+           llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
+           llvm::raw_pwrite_stream& stderr) -> ErrorOr<bool> override {
     // Add the prelude.
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> prelude =
         llvm::MemoryBuffer::getFile("explorer/data/prelude.carbon");
@@ -66,8 +56,7 @@ class ExplorerFileTest : public FileTestBase {
     return exit_code == EXIT_SUCCESS;
   }
 
-  auto ValidateRun(const llvm::SmallVector<TestFile>& /*test_files*/)
-      -> void override {
+  auto ValidateRun() -> void override {
     // Skip trace test check as they use stdout stream instead of
     // trace_stream_ostream
     if (absl::GetFlag(FLAGS_trace)) {

testing/file_test/BUILD

@@ -47,6 +47,7 @@ file_test(
     tests = glob(["testdata/**"]),
     deps = [
         ":file_test_base",
+        "//common:ostream",
         "@com_google_googletest//:gtest",
         "@llvm-project//llvm:Support",
     ],

testing/file_test/file_test_base.cpp

@@ -15,6 +15,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 ABSL_FLAG(std::vector<std::string>, file_tests, {},
           "A comma-separated list of tests for file_test infrastructure.");
@@ -61,7 +62,7 @@ auto FileTestBase::TestBody() -> void {
   TestContext context;
   auto run_result = ProcessTestFileAndRun(context);
   ASSERT_TRUE(run_result.ok()) << run_result.error();
-  ValidateRun(context.test_files);
+  ValidateRun();
   EXPECT_THAT(!llvm::StringRef(path().filename()).starts_with("fail_"),
               Eq(context.exit_with_success))
       << "Tests should be prefixed with `fail_` if and only if running them "
@@ -145,12 +146,22 @@ auto FileTestBase::ProcessTestFileAndRun(TestContext& context)
     test_args_ref.push_back(arg);
   }
 
+  // Create the files in-memory.
+  llvm::vfs::InMemoryFileSystem fs;
+  for (const auto& test_file : context.test_files) {
+    if (!fs.addFile(test_file.filename, /*ModificationTime=*/0,
+                    llvm::MemoryBuffer::getMemBuffer(
+                        test_file.content, test_file.filename,
+                        /*RequiresNullTerminator=*/false))) {
+      return ErrorBuilder() << "File is repeated: " << test_file.filename;
+    }
+  }
+
   // Capture trace streaming, but only when in debug mode.
   llvm::raw_svector_ostream stdout(context.stdout);
   llvm::raw_svector_ostream stderr(context.stderr);
-  CARBON_ASSIGN_OR_RETURN(
-      context.exit_with_success,
-      Run(test_args_ref, context.test_files, stdout, stderr));
+  CARBON_ASSIGN_OR_RETURN(context.exit_with_success,
+                          Run(test_args_ref, fs, stdout, stderr));
   return Success();
 }
 

testing/file_test/file_test_base.h

@@ -16,6 +16,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "testing/file_test/autoupdate.h"
 
 namespace Carbon::Testing {
@@ -45,7 +46,8 @@ class FileTestBase : public testing::Test {
   explicit FileTestBase(std::filesystem::path path) : path_(std::move(path)) {}
 
   // Implemented by children to run the test. For example, TestBody validates
-  // stdout and stderr.
+  // stdout and stderr. Children should use fs for file content, and may add
+  // more files.
   //
   // Any test expectations should be called from ValidateRun, not Run.
   //
@@ -53,15 +55,14 @@ class FileTestBase : public testing::Test {
   // should be true if a binary would return EXIT_SUCCESS, and false for
   // EXIT_FAILURE (which is a test success for `fail_*` tests).
   virtual auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-                   const llvm::SmallVector<TestFile>& test_files,
+                   llvm::vfs::InMemoryFileSystem& fs,
                    llvm::raw_pwrite_stream& stdout,
                    llvm::raw_pwrite_stream& stderr) -> ErrorOr<bool> = 0;
 
   // Implemented by children to do post-Run test expectations. Only called when
   // testing. Does not need to be provided if only CHECK test expectations are
   // used.
-  virtual auto ValidateRun(const llvm::SmallVector<TestFile>& /*test_files*/)
-      -> void {}
+  virtual auto ValidateRun() -> void {}
 
   // Returns default arguments. Only called when a file doesn't set ARGS.
   virtual auto GetDefaultArgs() -> llvm::SmallVector<std::string> = 0;

testing/file_test/file_test_base_test.cpp

@@ -7,33 +7,40 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "common/ostream.h"
 #include "llvm/ADT/StringExtras.h"
 
 namespace Carbon::Testing {
 namespace {
 
-using ::testing::AllOf;
-using ::testing::ElementsAre;
 using ::testing::Eq;
-using ::testing::Field;
-using ::testing::Matcher;
+
+// Helper to validate file content.
+static auto CheckFileContent(llvm::vfs::InMemoryFileSystem& fs,
+                             llvm::StringRef filename,
+                             llvm::StringRef expected_content)
+    -> ErrorOr<Success> {
+  auto file = fs.getBufferForFile(filename, /*FileSize=*/-1,
+                                  /*RequiresNullTerminator=*/false);
+  if (file.getError()) {
+    return ErrorBuilder() << "Missing " << filename;
+  }
+  if (file->get()->getBuffer() != expected_content) {
+    return ErrorBuilder() << "Unexpected file content for " << filename
+                          << ".\n--- Actual:\n"
+                          << file->get()->getBuffer() << "\n--- Expected:\n"
+                          << expected_content << "\n---";
+  }
+  return Success();
+}
 
 class FileTestBaseTest : public FileTestBase {
  public:
   using FileTestBase::FileTestBase;
 
-  static auto HasFilename(std::string filename) -> Matcher<TestFile> {
-    return Field("filename", &TestFile::filename, Eq(filename));
-  }
-
-  static auto HasContent(std::string content) -> Matcher<TestFile> {
-    return Field("content", &TestFile::content, Eq(content));
-  }
-
   auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-           const llvm::SmallVector<TestFile>& test_files,
-           llvm::raw_pwrite_stream& stdout, llvm::raw_pwrite_stream& stderr)
-      -> ErrorOr<bool> override {
+           llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
+           llvm::raw_pwrite_stream& stderr) -> ErrorOr<bool> override {
     if (!test_args.empty()) {
       llvm::ListSeparator sep;
       stdout << test_args.size() << " args: ";
@@ -43,7 +50,20 @@ class FileTestBaseTest : public FileTestBase {
       stdout << "\n";
     }
 
-    auto filename = path().filename();
+    auto filename = path().filename().string();
+    if (filename == "two_files.carbon") {
+      // Verify the split.
+      CARBON_RETURN_IF_ERROR(CheckFileContent(
+          fs, "a.carbon", "aaa\n// CHECK:STDOUT: a.carbon:[[@LINE-1]]: 1\n\n"));
+      CARBON_RETURN_IF_ERROR(CheckFileContent(
+          fs, "b.carbon", "bbb\n// CHECK:STDOUT: b.carbon:[[@LINE-1]]: 2\n"));
+    } else {
+      // Other files should be copied directly, so aren't as interesting.
+      if (!fs.exists(filename)) {
+        return ErrorBuilder() << "Missing file: " << filename;
+      }
+    }
+
     if (filename == "args.carbon") {
       return true;
     } else if (filename == "example.carbon") {
@@ -59,35 +79,14 @@ class FileTestBaseTest : public FileTestBase {
       stderr << "Oops\n";
       return false;
     } else if (filename == "two_files.carbon") {
-      int i = 0;
-      for (const auto& file : test_files) {
-        // Prints line numbers to validate per-file.
-        stdout << file.filename << ":2: " << ++i << "\n";
-      }
+      // Prints line numbers to validate per-file.
+      stdout << "a.carbon:1: 1\nb.carbon:1: 2\n";
       return true;
     } else {
       return ErrorBuilder() << "Unexpected file: " << filename;
     }
   }
 
-  auto ValidateRun(const llvm::SmallVector<TestFile>& test_files)
-      -> void override {
-    auto filename = path().filename();
-    if (filename == "two_files.carbon") {
-      EXPECT_THAT(
-          test_files,
-          ElementsAre(
-              AllOf(HasFilename("a.carbon"),
-                    HasContent(
-                        "// CHECK:STDOUT: a.carbon:[[@LINE+1]]: 1\naaa\n\n")),
-              AllOf(HasFilename("b.carbon"),
-                    HasContent(
-                        "// CHECK:STDOUT: b.carbon:[[@LINE+1]]: 2\nbbb\n"))));
-    } else {
-      EXPECT_THAT(test_files, ElementsAre(HasFilename(filename)));
-    }
-  }
-
   auto GetDefaultArgs() -> llvm::SmallVector<std::string> override {
     return {"default_args", "%s"};
   }

testing/file_test/testdata/args.carbon

@@ -4,4 +4,5 @@
 
 // ARGS: abc file=%t %s
 // AUTOUPDATE
+
 // CHECK:STDOUT: 3 args: `abc`, `file={{.+}}/temp_file`, `args.carbon`

testing/file_test/testdata/fail_example.carbon

@@ -3,5 +3,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 // AUTOUPDATE
-// CHECK:STDOUT: 2 args: `default_args`, `fail_example.carbon`
 // CHECK:STDERR: Oops
+
+// CHECK:STDOUT: 2 args: `default_args`, `fail_example.carbon`

testing/file_test/testdata/two_files.carbon

@@ -3,12 +3,12 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 // AUTOUPDATE
-// CHECK:STDOUT: 3 args: `default_args`, `a.carbon`, `b.carbon`
 
+// CHECK:STDOUT: 3 args: `default_args`, `a.carbon`, `b.carbon`
 // --- a.carbon
-// CHECK:STDOUT: a.carbon:[[@LINE+1]]: 1
 aaa
+// CHECK:STDOUT: a.carbon:[[@LINE-1]]: 1
 
 // --- b.carbon
-// CHECK:STDOUT: b.carbon:[[@LINE+1]]: 2
 bbb
+// CHECK:STDOUT: b.carbon:[[@LINE-1]]: 2

toolchain/driver/driver_file_test_base.h

@@ -9,7 +9,6 @@
 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "testing/file_test/file_test_base.h"
 #include "toolchain/driver/driver.h"
@@ -23,18 +22,8 @@ class DriverFileTestBase : public FileTestBase {
   using FileTestBase::FileTestBase;
 
   auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-           const llvm::SmallVector<TestFile>& test_files,
-           llvm::raw_pwrite_stream& stdout, llvm::raw_pwrite_stream& stderr)
-      -> ErrorOr<bool> override {
-    // Create the files in-memory.
-    llvm::vfs::InMemoryFileSystem fs;
-    for (const auto& test_file : test_files) {
-      if (!fs.addFile(test_file.filename, /*ModificationTime=*/0,
-                      llvm::MemoryBuffer::getMemBuffer(test_file.content))) {
-        return ErrorBuilder() << "File is repeated: " << test_file.filename;
-      }
-    }
-
+           llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
+           llvm::raw_pwrite_stream& stderr) -> ErrorOr<bool> override {
     Driver driver(fs, stdout, stderr);
     return driver.RunCommand(test_args);
   }