From 6586179c8d19a01fabaf2c7eabfdeaee47062f9b Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 7 Jun 2023 08:56:20 -0700 Subject: [PATCH] Add support for splitting a test file. (#2876) I'm looking at this as I start thinking about handling `import`. Syntax is based on llvm's `split-file` tool. The `std::vector` -> `llvm::SmallVector` switch is minor, I'm doing it here because I had to touch everything anyways and I think for tests I'll lean slightly more towards the toolchain's way of doing things versus explorer's. --- explorer/file_test.cpp | 13 +- testing/file_test/BUILD | 2 +- testing/file_test/file_test_base.cpp | 122 +++++++++++++----- testing/file_test/file_test_base.h | 24 +++- testing/file_test/file_test_base_test.cpp | 31 ++++- .../file_test/{ => testdata}/example.carbon | 0 .../{ => testdata}/fail_example.carbon | 0 testing/file_test/testdata/two_files.carbon | 9 ++ toolchain/lexer/lexer_file_test.cpp | 12 +- toolchain/lowering/lowering_file_test.cpp | 12 +- toolchain/parser/parse_tree_file_test.cpp | 12 +- toolchain/semantics/semantics_file_test.cpp | 12 +- 12 files changed, 190 insertions(+), 59 deletions(-) rename testing/file_test/{ => testdata}/example.carbon (100%) rename testing/file_test/{ => testdata}/fail_example.carbon (100%) create mode 100644 testing/file_test/testdata/two_files.carbon diff --git a/explorer/file_test.cpp b/explorer/file_test.cpp index 7b3e5268a3d5..9aced92ee1a0 100644 --- a/explorer/file_test.cpp +++ b/explorer/file_test.cpp @@ -32,8 +32,15 @@ class ParseAndExecuteTestFile : public FileTestBase { } } - auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) + auto RunWithFiles(const llvm::SmallVector& test_files, + llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) -> bool override { + if (test_files.size() != 1) { + ADD_FAILURE() << "Only 1 file is supported: " << test_files.size() + << " provided"; + return false; + } + // Capture trace streaming, but only when in debug mode. TraceStream trace_stream; std::string trace_stream_str; @@ -74,8 +81,8 @@ class ParseAndExecuteTestFile : public FileTestBase { } // namespace -extern auto RegisterFileTests(const std::vector& paths) - -> void { +extern auto RegisterFileTests( + const llvm::SmallVector& paths) -> void { ParseAndExecuteTestFile::RegisterTests( "ParseAndExecuteTestFile", paths, [=](const std::filesystem::path& path) { return new ParseAndExecuteTestFile(path, /*trace=*/false); diff --git a/testing/file_test/BUILD b/testing/file_test/BUILD index 43615a82f7eb..1dfa3eb89127 100644 --- a/testing/file_test/BUILD +++ b/testing/file_test/BUILD @@ -21,7 +21,7 @@ cc_library( file_test( name = "file_test_base_test", srcs = ["file_test_base_test.cpp"], - tests = ["example.carbon"], + tests = glob(["testdata/**"]), deps = [ ":file_test_base", "@com_google_googletest//:gtest", diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index 0f4b3ad6b640..8a7f446f9641 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -8,6 +8,7 @@ #include #include "common/check.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/InitLLVM.h" @@ -22,12 +23,7 @@ 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(const std::filesystem::path& path) : path_(&path) {} FileTestBase::~FileTestBase() { // Restore the original working directory. @@ -37,7 +33,8 @@ FileTestBase::~FileTestBase() { } void FileTestBase::RegisterTests( - const char* fixture_label, const std::vector& paths, + const char* fixture_label, + const llvm::SmallVector& paths, std::function factory) { // Use RegisterTest instead of INSTANTIATE_TEST_CASE_P because of ordering // issues between container initialization and test instantiation by @@ -52,13 +49,13 @@ void FileTestBase::RegisterTests( // Splits outputs to string_view because gtest handles string_view by default. static auto SplitOutput(llvm::StringRef output) - -> std::vector { + -> llvm::SmallVector { if (output.empty()) { return {}; } llvm::SmallVector lines; llvm::StringRef(output).split(lines, "\n"); - return std::vector(lines.begin(), lines.end()); + return llvm::SmallVector(lines.begin(), lines.end()); } // Runs a test and compares output. This keeps output split by line so that @@ -71,15 +68,86 @@ auto FileTestBase::TestBody() -> void { llvm::errs() << "\nTo test this file alone, run:\n bazel test " << *subset_target << " --test_arg=" << test_file << "\n\n"; + SetTmpAsWorkingDir(); + // Load expected output. - std::vector> expected_stdout; - std::vector> expected_stderr; + llvm::SmallVector test_files; + auto cleanup_test_files = llvm::make_scope_exit([&]() { + // Remove all the created files. Even if no files are split, there will be a + // symlink to remove. + for (const auto& file : test_files) { + std::filesystem::remove(file); + } + }); + llvm::SmallVector> expected_stdout; + llvm::SmallVector> expected_stderr; + ProcessTestFile(test_files, expected_stdout, expected_stderr); + if (HasFailure()) { + return; + } + + // Capture trace streaming, but only when in debug mode. + std::string stdout; + std::string stderr; + llvm::raw_string_ostream stdout_ostream(stdout); + llvm::raw_string_ostream stderr_ostream(stderr); + bool run_succeeded = RunWithFiles(test_files, stdout_ostream, stderr_ostream); + if (HasFailure()) { + return; + } + 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."; + + // Check results. + EXPECT_THAT(SplitOutput(stdout), ElementsAreArray(expected_stdout)); + EXPECT_THAT(SplitOutput(stderr), ElementsAreArray(expected_stderr)); +} + +auto FileTestBase::SetTmpAsWorkingDir() -> void { + char* tmpdir = getenv("TEST_TMPDIR"); + CARBON_CHECK(tmpdir); + + // Run from the tmpdir. + std::error_code ec; + std::filesystem::current_path(tmpdir, ec); + CARBON_CHECK(!ec) << ec.message(); +} + +auto FileTestBase::ProcessTestFile( + llvm::SmallVector& test_files, + llvm::SmallVector>& expected_stdout, + llvm::SmallVector>& expected_stderr) -> void { std::ifstream file_content(path()); + std::ofstream split_file; + bool found_content_pre_split = false; int line_index = 0; std::string line_str; while (std::getline(file_content, line_str)) { - ++line_index; llvm::StringRef line = line_str; + if (line.consume_front("// ---")) { + ASSERT_FALSE(found_content_pre_split) + << "When using split files, there must be no content before the " + "first split file."; + test_files.push_back(line.trim().str()); + const auto& test_file = test_files.back(); + ASSERT_FALSE(std::filesystem::path(test_file).has_parent_path()) + << "Only filenames are supported, not subdirectories: " << test_file; + if (split_file.is_open()) { + split_file.close(); + } + split_file.open(test_file); + ASSERT_TRUE(split_file.is_open()) << "Failed to open " << test_file; + line_index = 0; + continue; + } else if (split_file.is_open()) { + split_file << line_str << "\n"; + } else if (!line.starts_with("//") && !line.trim().empty()) { + found_content_pre_split = true; + } + ++line_index; + line = line.ltrim(); if (!line.consume_front("// CHECK")) { continue; @@ -93,6 +161,16 @@ auto FileTestBase::TestBody() -> void { } } + if (test_files.empty()) { + // Symlink the main test file and provide it in test_files. + test_files.push_back(path().filename()); + std::error_code ec; + std::filesystem::create_symlink(path(), path().filename(), ec); + CARBON_CHECK(!ec) << ec.message(); + } else { + split_file.close(); + } + // Assume there is always a suffix `\n` in output. if (!expected_stdout.empty()) { expected_stdout.push_back(testing::StrEq("")); @@ -100,24 +178,6 @@ auto FileTestBase::TestBody() -> void { if (!expected_stderr.empty()) { expected_stderr.push_back(testing::StrEq("")); } - - // Capture trace streaming, but only when in debug mode. - std::string stdout; - std::string stderr; - llvm::raw_string_ostream stdout_ostream(stdout); - llvm::raw_string_ostream stderr_ostream(stderr); - bool run_succeeded = RunOverFile(stdout_ostream, stderr_ostream); - if (HasFailure()) { - return; - } - 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."; - - // Check results. - EXPECT_THAT(SplitOutput(stdout), ElementsAreArray(expected_stdout)); - EXPECT_THAT(SplitOutput(stderr), ElementsAreArray(expected_stderr)); } auto FileTestBase::TransformExpectation(int line_index, llvm::StringRef in) @@ -245,7 +305,7 @@ auto main(int argc, char** argv) -> int { Carbon::Testing::base_dir_len = base_dir.size(); // Register tests based on their absolute path. - std::vector paths; + llvm::SmallVector paths; for (int i = 1; i < argc; ++i) { auto path = std::filesystem::absolute(argv[i], ec); CARBON_CHECK(!ec) << argv[i] << ": " << ec.message(); diff --git a/testing/file_test/file_test_base.h b/testing/file_test/file_test_base.h index ee11b3221d6f..3ce5700bf203 100644 --- a/testing/file_test/file_test_base.h +++ b/testing/file_test/file_test_base.h @@ -12,6 +12,7 @@ #include #include +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/raw_ostream.h" @@ -19,7 +20,7 @@ namespace Carbon::Testing { // A framework for testing files. Children implement `RegisterTestFiles` with // calls to `RegisterTests` using a factory that constructs the child. -// `RunOverFile` must also be implemented and will be called as part of +// `RunWithFiles` must also be implemented and will be called as part of // individual test executions. This framework includes a `main` implementation, // so users must not provide one. // @@ -39,14 +40,15 @@ class FileTestBase : public testing::Test { // Used by children to register tests with gtest. static void RegisterTests( const char* fixture_label, - const std::vector& paths, + const llvm::SmallVector& paths, std::function factory); // Implemented by children to run the test. Called by the TestBody // implementation, which will validate stdout and stderr. The return value // should be false when "fail_" is in the filename. - virtual auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) - -> bool = 0; + virtual auto RunWithFiles(const llvm::SmallVector& test_files, + llvm::raw_ostream& stdout, + llvm::raw_ostream& stderr) -> bool = 0; // Runs a test and compares output. This keeps output split by line so that // issues are a little easier to identify by the different line. @@ -56,6 +58,16 @@ class FileTestBase : public testing::Test { auto path() -> const std::filesystem::path& { return *path_; }; private: + // Sets TEST_TMPDIR as the working directory. + auto SetTmpAsWorkingDir() -> void; + + // Processes the test input, producing test files and expected output. + auto ProcessTestFile( + llvm::SmallVector& test_files, + llvm::SmallVector>& expected_stdout, + llvm::SmallVector>& expected_stderr) + -> void; + // Transforms an expectation on a given line from `FileCheck` syntax into a // standard regex matcher. static auto TransformExpectation(int line_index, llvm::StringRef in) @@ -65,8 +77,8 @@ class FileTestBase : public testing::Test { }; // Must be implemented by the individual file_test to initialize tests. -extern auto RegisterFileTests(const std::vector& paths) - -> void; +extern auto RegisterFileTests( + const llvm::SmallVector& paths) -> void; } // namespace Carbon::Testing diff --git a/testing/file_test/file_test_base_test.cpp b/testing/file_test/file_test_base_test.cpp index c44d097004cc..bf5ea98e7eaf 100644 --- a/testing/file_test/file_test_base_test.cpp +++ b/testing/file_test/file_test_base_test.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include "llvm/ADT/StringRef.h" @@ -15,15 +16,19 @@ namespace Carbon::Testing { namespace { +using ::testing::ElementsAre; + class FileTestBaseTest : public FileTestBase { public: explicit FileTestBaseTest(const std::filesystem::path& path) : FileTestBase(path) {} - auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) + auto RunWithFiles(const llvm::SmallVector& test_files, + llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) -> bool override { auto filename = path().filename(); if (filename == "example.carbon") { + EXPECT_THAT(test_files, ElementsAre("example.carbon")); stdout << "something\n" "\n" "8: Line delta\n" @@ -32,8 +37,30 @@ class FileTestBaseTest : public FileTestBase { "Foo baz\n"; return true; } else if (filename == "fail_example.carbon") { + EXPECT_THAT(test_files, ElementsAre("fail_example.carbon")); 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 << ": " << ++i << "\n"; + + // Make sure the split files have appropriate content. + std::ifstream file_in(file); + std::stringstream content; + content << file_in.rdbuf(); + if (file == "a.carbon") { + EXPECT_THAT(content.str(), + testing::Eq("// CHECK:STDOUT: a.carbon: [[@LINE+0]]\n\n")) + << "Checking " << file; + } else { + EXPECT_THAT(content.str(), + testing::Eq("// CHECK:STDOUT: b.carbon: [[@LINE+1]]\n")) + << "Checking " << file; + } + } + return true; } else { ADD_FAILURE() << "Unexpected file: " << filename; return false; @@ -43,7 +70,7 @@ class FileTestBaseTest : public FileTestBase { } // namespace -auto RegisterFileTests(const std::vector& paths) +auto RegisterFileTests(const llvm::SmallVector& paths) -> void { FileTestBaseTest::RegisterTests("FileTestBaseTest", paths, [](const std::filesystem::path& path) { diff --git a/testing/file_test/example.carbon b/testing/file_test/testdata/example.carbon similarity index 100% rename from testing/file_test/example.carbon rename to testing/file_test/testdata/example.carbon diff --git a/testing/file_test/fail_example.carbon b/testing/file_test/testdata/fail_example.carbon similarity index 100% rename from testing/file_test/fail_example.carbon rename to testing/file_test/testdata/fail_example.carbon diff --git a/testing/file_test/testdata/two_files.carbon b/testing/file_test/testdata/two_files.carbon new file mode 100644 index 000000000000..11757a8604f0 --- /dev/null +++ b/testing/file_test/testdata/two_files.carbon @@ -0,0 +1,9 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +// --- a.carbon +// CHECK:STDOUT: a.carbon: [[@LINE+0]] + +// --- b.carbon +// CHECK:STDOUT: b.carbon: [[@LINE+1]] diff --git a/toolchain/lexer/lexer_file_test.cpp b/toolchain/lexer/lexer_file_test.cpp index 0776c3a064e9..dabce801291d 100644 --- a/toolchain/lexer/lexer_file_test.cpp +++ b/toolchain/lexer/lexer_file_test.cpp @@ -20,17 +20,21 @@ class LexerFileTest : public FileTestBase { explicit LexerFileTest(const std::filesystem::path& path) : FileTestBase(path) {} - auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) + auto RunWithFiles(const llvm::SmallVector& test_files, + llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) -> bool override { + llvm::SmallVector args({"dump", "tokens"}); + for (const auto& file : test_files) { + args.push_back(file); + } Driver driver(stdout, stderr); - return driver.RunFullCommand( - {"dump", "tokens", path().filename().string()}); + return driver.RunFullCommand(args); } }; } // namespace -auto RegisterFileTests(const std::vector& paths) +auto RegisterFileTests(const llvm::SmallVector& paths) -> void { LexerFileTest::RegisterTests("LexerFileTest", paths, [](const std::filesystem::path& path) { diff --git a/toolchain/lowering/lowering_file_test.cpp b/toolchain/lowering/lowering_file_test.cpp index d0f8858c8ba6..a2716cee37ba 100644 --- a/toolchain/lowering/lowering_file_test.cpp +++ b/toolchain/lowering/lowering_file_test.cpp @@ -20,17 +20,21 @@ class LoweringFileTest : public FileTestBase { explicit LoweringFileTest(const std::filesystem::path& path) : FileTestBase(path) {} - auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) + auto RunWithFiles(const llvm::SmallVector& test_files, + llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) -> bool override { + llvm::SmallVector args({"dump", "llvm-ir"}); + for (const auto& file : test_files) { + args.push_back(file); + } Driver driver(stdout, stderr); - return driver.RunFullCommand( - {"dump", "llvm-ir", path().filename().string()}); + return driver.RunFullCommand(args); } }; } // namespace -auto RegisterFileTests(const std::vector& paths) +auto RegisterFileTests(const llvm::SmallVector& paths) -> void { LoweringFileTest::RegisterTests("LoweringFileTest", paths, [=](const std::filesystem::path& path) { diff --git a/toolchain/parser/parse_tree_file_test.cpp b/toolchain/parser/parse_tree_file_test.cpp index bea908733fdd..393066ae0a8b 100644 --- a/toolchain/parser/parse_tree_file_test.cpp +++ b/toolchain/parser/parse_tree_file_test.cpp @@ -20,17 +20,21 @@ class ParserFileTest : public FileTestBase { explicit ParserFileTest(const std::filesystem::path& path) : FileTestBase(path) {} - auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) + auto RunWithFiles(const llvm::SmallVector& test_files, + llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) -> bool override { + llvm::SmallVector args({"dump", "parse-tree"}); + for (const auto& file : test_files) { + args.push_back(file); + } Driver driver(stdout, stderr); - return driver.RunFullCommand( - {"dump", "parse-tree", path().filename().string()}); + return driver.RunFullCommand(args); } }; } // namespace -auto RegisterFileTests(const std::vector& paths) +auto RegisterFileTests(const llvm::SmallVector& paths) -> void { ParserFileTest::RegisterTests("ParserFileTest", paths, [](const std::filesystem::path& path) { diff --git a/toolchain/semantics/semantics_file_test.cpp b/toolchain/semantics/semantics_file_test.cpp index d00307a3fca5..74b76f4da974 100644 --- a/toolchain/semantics/semantics_file_test.cpp +++ b/toolchain/semantics/semantics_file_test.cpp @@ -20,17 +20,21 @@ class SemanticsFileTest : public FileTestBase { explicit SemanticsFileTest(const std::filesystem::path& path) : FileTestBase(path) {} - auto RunOverFile(llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) + auto RunWithFiles(const llvm::SmallVector& test_files, + llvm::raw_ostream& stdout, llvm::raw_ostream& stderr) -> bool override { + llvm::SmallVector args({"dump", "semantics-ir"}); + for (const auto& file : test_files) { + args.push_back(file); + } Driver driver(stdout, stderr); - return driver.RunFullCommand( - {"dump", "semantics-ir", path().filename().string()}); + return driver.RunFullCommand(args); } }; } // namespace -auto RegisterFileTests(const std::vector& paths) +auto RegisterFileTests(const llvm::SmallVector& paths) -> void { SemanticsFileTest::RegisterTests("SemanticsFileTest", paths, [](const std::filesystem::path& path) {