diff --git a/.vscode/gdb_launch.json b/.vscode/gdb_launch.json index cb9ddf8a311d..966d70e5654d 100644 --- a/.vscode/gdb_launch.json +++ b/.vscode/gdb_launch.json @@ -8,7 +8,10 @@ "program": "bazel-bin/toolchain/testing/file_test", "programArgs": "--file_tests=${relativeFile}", "cwd": "${workspaceFolder}", - "env": { "TEST_TMPDIR": "/tmp" } + "env": { + "TEST_TARGET": "//toolchain/testing:file_test", + "TEST_TMPDIR": "/tmp" + } }, { "type": "by-gdb", diff --git a/.vscode/lldb_launch.json b/.vscode/lldb_launch.json index 007c16662665..c977bc10aa6f 100644 --- a/.vscode/lldb_launch.json +++ b/.vscode/lldb_launch.json @@ -10,12 +10,11 @@ "debuggerRoot": "${workspaceFolder}", "initCommands": [ "command script import external/+llvm_project+llvm-project/llvm/utils/lldbDataFormatters.py", - "settings set target.source-map \".\" \"${workspaceFolder}\"" - ], - "env": { "TEST_TMPDIR": "/tmp" }, - "sourceMap": { - "/proc/self/cwd/": "${workspaceFolder}" - } + "settings set target.source-map \".\" \"${workspaceFolder}\"", + "settings set target.source-map \"/proc/self/cwd\" \"${workspaceFolder}\"", + "env TEST_TARGET=//toolchain/testing:file_test", + "env TEST_TMPDIR=/tmp" + ] }, { "type": "lldb-dap", @@ -32,7 +31,8 @@ "debuggerRoot": "${workspaceFolder}", "initCommands": [ "command script import external/+llvm_project+llvm-project/llvm/utils/lldbDataFormatters.py", - "settings set target.source-map \".\" \"${workspaceFolder}\"" + "settings set target.source-map \".\" \"${workspaceFolder}\"", + "settings set target.source-map \"/proc/self/cwd\" \"${workspaceFolder}\"" ] } ] diff --git a/bazel/manifest/defs.bzl b/bazel/manifest/defs.bzl index 926ec1f514c3..04695d628742 100644 --- a/bazel/manifest/defs.bzl +++ b/bazel/manifest/defs.bzl @@ -4,9 +4,7 @@ """Rule for producing a manifest for a filegroup.""" -def _manifest(ctx): - out = ctx.actions.declare_file(ctx.label.name) - +def _get_files(ctx): files = [] for src in ctx.attr.srcs: files.extend([f.path for f in src[DefaultInfo].files.to_list()]) @@ -17,11 +15,16 @@ def _manifest(ctx): if ctx.attr.strip_package_dir: package_dir = ctx.label.package + "/" - content = [f.removeprefix(package_dir) for f in files] + files_stripped = [f.removeprefix(package_dir) for f in files] else: - content = files + files_stripped = files - ctx.actions.write(out, "\n".join(content) + "\n") + return files_stripped + +def _manifest(ctx): + out = ctx.actions.declare_file(ctx.label.name) + files = _get_files(ctx) + ctx.actions.write(out, "\n".join(files) + "\n") return [ DefaultInfo( @@ -30,6 +33,7 @@ def _manifest(ctx): ), ] +# Produces the manifest as a series of lines. manifest = rule( implementation = _manifest, attrs = { @@ -37,3 +41,38 @@ manifest = rule( "strip_package_dir": attr.bool(default = False), }, ) + +def _manifest_as_cpp(ctx): + out = ctx.actions.declare_file(ctx.label.name) + files = _get_files(ctx) + lines = [ + "// Auto-generated by manifest_as_cpp.", + "const char* {0}[] = {{".format(ctx.attr.var_name), + ] + lines += [ + " \"{0}\",".format(file) + for file in files + ] + lines += [ + " nullptr,", + "};", + ] + ctx.actions.write(out, "\n".join(lines) + "\n") + + return [ + DefaultInfo( + files = depset(direct = [out]), + runfiles = ctx.runfiles(files = [out]), + ), + ] + +# Produces the manifest as a nullptr-terminated `const char* var_name[]`. +# Use with `extern const char* var_name[];`. +manifest_as_cpp = rule( + implementation = _manifest_as_cpp, + attrs = { + "srcs": attr.label_list(allow_files = True, mandatory = True), + "strip_package_dir": attr.bool(default = False), + "var_name": attr.string(mandatory = True), + }, +) diff --git a/explorer/BUILD b/explorer/BUILD index e93d4b12859f..1c47ba0ae251 100644 --- a/explorer/BUILD +++ b/explorer/BUILD @@ -58,8 +58,11 @@ cc_binary( ":main", "//common:check", "//common:raw_string_ostream", + "//testing/base:file_helpers", "//testing/file_test:file_test_base", + "//testing/file_test:manifest", "@abseil-cpp//absl/flags:flag", + "@abseil-cpp//absl/strings", "@re2", ], ) diff --git a/explorer/file_test.cpp b/explorer/file_test.cpp index 22a28f3ffd3b..210bf4518340 100644 --- a/explorer/file_test.cpp +++ b/explorer/file_test.cpp @@ -3,10 +3,13 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #include "absl/flags/flag.h" +#include "absl/strings/str_split.h" #include "common/raw_string_ostream.h" #include "explorer/main.h" #include "re2/re2.h" +#include "testing/base/file_helpers.h" #include "testing/file_test/file_test_base.h" +#include "testing/file_test/manifest.h" ABSL_FLAG(bool, trace, false, "Set to true to run tests with tracing enabled, even if they don't " @@ -115,8 +118,13 @@ class ExplorerFileTest : public FileTestBase { } // namespace // Explorer uses a non-standard approach to getting the manifest path. -auto GetFileTestManifestPath() -> std::filesystem::path { - return absl::GetFlag(FLAGS_explorer_test_targets_file); +auto GetFileTestManifest() -> llvm::SmallVector { + llvm::SmallVector manifest; + auto content = ReadFile(absl::GetFlag(FLAGS_explorer_test_targets_file)); + for (const auto& line : absl::StrSplit(*content, "\n", absl::SkipEmpty())) { + manifest.push_back(std::string(line)); + } + return manifest; } CARBON_FILE_TEST_FACTORY(ExplorerFileTest) diff --git a/scripts/fix_cc_deps.py b/scripts/fix_cc_deps.py index 81b7c38757d1..fd4d8bba73b6 100755 --- a/scripts/fix_cc_deps.py +++ b/scripts/fix_cc_deps.py @@ -68,7 +68,7 @@ EXTERNAL_REPOS: dict[str, ExternalRepo] = { } IGNORE_SOURCE_FILE_REGEX = re.compile( - "^(third_party/clangd.*|common/version.*\\.cpp)$" + r"^(third_party/clangd.*|common/version.*\.cpp|.*_autogen_manifest\.cpp)$" ) diff --git a/testing/file_test/BUILD b/testing/file_test/BUILD index 63e61ec29b93..6560c8a8129f 100644 --- a/testing/file_test/BUILD +++ b/testing/file_test/BUILD @@ -7,9 +7,6 @@ load("rules.bzl", "file_test") package(default_visibility = ["//visibility:public"]) -# Used to access a `#define` with the manifest file, produced by rules.bzl. -exports_files(["file_test_manifest.cpp"]) - cc_library( name = "autoupdate", testonly = 1, @@ -42,6 +39,7 @@ cc_library( hdrs = ["file_test_base.h"], deps = [ ":autoupdate", + ":manifest", "//common:check", "//common:error", "//common:exe_path", @@ -69,3 +67,19 @@ file_test( "@llvm-project//llvm:Support", ], ) + +# Note this is separate from the implementation; see the .h file. +cc_library( + name = "manifest", + testonly = 1, + hdrs = ["manifest.h"], + deps = ["@llvm-project//llvm:Support"], +) + +# Used through `file_test` in rules.bzl. +cc_library( + name = "manifest_impl", + testonly = 1, + srcs = ["manifest.cpp"], + deps = [":manifest"], +) diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index 6a2c325c3fd3..607353df0950 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -29,7 +29,6 @@ #include "absl/flags/flag.h" #include "absl/flags/parse.h" #include "absl/strings/str_join.h" -#include "absl/strings/str_split.h" #include "common/check.h" #include "common/error.h" #include "common/exe_path.h" @@ -42,7 +41,6 @@ #include "llvm/Support/PrettyStackTrace.h" #include "llvm/Support/Process.h" #include "llvm/Support/ThreadPool.h" -#include "testing/base/file_helpers.h" #include "testing/file_test/autoupdate.h" #include "testing/file_test/run_test.h" #include "testing/file_test/test_file.h" @@ -298,14 +296,9 @@ static auto RegisterTests(FileTestFactory* test_factory, llvm::StringRef exe_path, llvm::SmallVectorImpl& tests) -> ErrorOr { - GetFileTestManifestPath(); - CARBON_ASSIGN_OR_RETURN(auto test_manifest, - ReadFile(GetFileTestManifestPath())); - // Prepare the vector first, so that the location of entries won't change. - for (const auto& test_name : - absl::StrSplit(test_manifest, "\n", absl::SkipEmpty())) { - tests.push_back({.test_name = std::string(test_name)}); + for (auto& test_name : GetFileTestManifest()) { + tests.push_back({.test_name = test_name}); } // Amend entries with factory functions. diff --git a/testing/file_test/file_test_base.h b/testing/file_test/file_test_base.h index 6c7433685eb5..4cd6d064e702 100644 --- a/testing/file_test/file_test_base.h +++ b/testing/file_test/file_test_base.h @@ -19,6 +19,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/VirtualFileSystem.h" #include "testing/file_test/autoupdate.h" +#include "testing/file_test/manifest.h" namespace Carbon::Testing { @@ -133,11 +134,6 @@ struct FileTestFactory { // to implement this function. extern auto GetFileTestFactory() -> FileTestFactory; -// Returns the manifest path, which is provided by rules.bzl and -// file_test_manifest.cpp. This is exposed so that the explorer sharding -// approach can use a different implementation. -auto GetFileTestManifestPath() -> std::filesystem::path; - // Provides a standard GetFileTestFactory implementation. #define CARBON_FILE_TEST_FACTORY(Name) \ auto GetFileTestFactory() -> FileTestFactory { \ diff --git a/testing/file_test/file_test_manifest.cpp b/testing/file_test/file_test_manifest.cpp deleted file mode 100644 index 9467c93e1e89..000000000000 --- a/testing/file_test/file_test_manifest.cpp +++ /dev/null @@ -1,13 +0,0 @@ -// 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 - -#include "testing/file_test/file_test_base.h" - -namespace Carbon::Testing { - -auto GetFileTestManifestPath() -> std::filesystem::path { - return CARBON_FILE_TEST_MANIFEST; -} - -} // namespace Carbon::Testing diff --git a/testing/file_test/manifest.cpp b/testing/file_test/manifest.cpp new file mode 100644 index 000000000000..d71281344dbd --- /dev/null +++ b/testing/file_test/manifest.cpp @@ -0,0 +1,21 @@ +// 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 + +#include "testing/file_test/manifest.h" + +// The test manifest, produced by `manifest_as_cpp`. +// NOLINTNEXTLINE(readability-identifier-naming): Constant in practice. +extern const char* CarbonFileTestManifest[]; + +namespace Carbon::Testing { + +auto GetFileTestManifest() -> llvm::SmallVector { + llvm::SmallVector manifest; + for (int i = 0; CarbonFileTestManifest[i]; ++i) { + manifest.push_back(CarbonFileTestManifest[i]); + } + return manifest; +} + +} // namespace Carbon::Testing diff --git a/testing/file_test/manifest.h b/testing/file_test/manifest.h new file mode 100644 index 000000000000..09651469228c --- /dev/null +++ b/testing/file_test/manifest.h @@ -0,0 +1,21 @@ +// 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 + +#ifndef CARBON_TESTING_FILE_TEST_MANIFEST_H_ +#define CARBON_TESTING_FILE_TEST_MANIFEST_H_ + +#include + +#include "llvm/ADT/SmallVector.h" + +namespace Carbon::Testing { + +// Returns the manifest path, which is provided by rules.bzl and manifest.cpp. +// This is exposed separately so that the explorer sharding approach can use a +// different implementation. +auto GetFileTestManifest() -> llvm::SmallVector; + +} // namespace Carbon::Testing + +#endif // CARBON_TESTING_FILE_TEST_MANIFEST_H_ diff --git a/testing/file_test/rules.bzl b/testing/file_test/rules.bzl index d97f9b107249..d4db39f488e7 100644 --- a/testing/file_test/rules.bzl +++ b/testing/file_test/rules.bzl @@ -10,12 +10,13 @@ a file which can be accessed as a list. This avoids long argument parsing. load("@rules_cc//cc:defs.bzl", "cc_test") load("//bazel/cc_toolchains:defs.bzl", "cc_env") -load("//bazel/manifest:defs.bzl", "manifest") +load("//bazel/manifest:defs.bzl", "manifest", "manifest_as_cpp") def file_test( name, tests, srcs = [], + deps = [], data = [], args = [], prebuilt_binary = None, @@ -30,6 +31,7 @@ def file_test( name: The base name of the tests. tests: The list of test files to use as data, typically a glob. srcs: Passed to cc_test. + deps: Passed to cc_test. data: Passed to cc_test. args: Passed to cc_test. prebuilt_binary: If set, specifies a prebuilt test binary to use instead @@ -38,34 +40,41 @@ def file_test( """ # Ensure tests are always a filegroup for tests_as_input_file_rule. - tests_file = "{0}.tests".format(name) - manifest( - name = tests_file, - srcs = tests, - testonly = 1, - ) - data = [":" + tests_file] + tests + data - if prebuilt_binary: # TODO: The prebuilt_binary support is only used by explorer. We should # remove this once explorer is removed, and think about better factoring # approaches if we need it later for toolchain. + tests_file = "{0}.tests".format(name) + manifest( + name = tests_file, + srcs = tests, + testonly = 1, + ) args = ["--explorer_test_targets_file=$(rootpath :{0})".format(tests_file)] + args + native.sh_test( name = name, srcs = srcs + [prebuilt_binary], - data = data, + deps = deps, + data = [":" + tests_file] + tests + data, args = args, env = cc_env(), **kwargs ) else: + manifest_cpp = "{0}_autogen_manifest.cpp".format(name) + manifest_as_cpp( + name = manifest_cpp, + var_name = "CarbonFileTestManifest", + srcs = tests, + testonly = 1, + ) cc_test( name = name, - srcs = srcs + ["//testing/file_test:file_test_manifest.cpp"], - data = data, + srcs = srcs + [":" + manifest_cpp], + deps = deps + ["//testing/file_test:manifest_impl"], + data = tests + data, args = args, env = cc_env(), - local_defines = ["CARBON_FILE_TEST_MANIFEST='\"$(rootpath :{0})\"'".format(tests_file)], **kwargs )