Jelajahi Sumber

Refactor testing exe path and benchmark main handling. (#4216)

Consolidates both main libraries into `//testing/base`, and factors out
the exe path handling for benchmarks and unit tests into a common
library to remove duplication. Refactors how that logic is managed to be
cleaner and avoid a confusing bool that came up in code review.

Updates all the tests and benchmarks that use these. I still need to
update other benchmarks to use the same main, but I wanted to keep this
PR somewhat minimal.

This also fixes a bug noticed in passing that the compilation benchmark
didn't have the required dependency on the benchmark library itself,
just the benchmark main library.
Chandler Carruth 1 tahun lalu
induk
melakukan
72cb9d0d06

+ 1 - 15
common/BUILD

@@ -35,20 +35,6 @@ cc_library(
     ],
 )
 
-cc_library(
-    name = "benchmark_main",
-    srcs = ["benchmark_main.cpp"],
-    hdrs = ["benchmark_main.h"],
-    deps = [
-        ":check",
-        ":exe_path",
-        ":init_llvm",
-        "@abseil-cpp//absl/flags:parse",
-        "@google_benchmark//:benchmark",
-        "@llvm-project//llvm:Support",
-    ],
-)
-
 cc_library(
     name = "command_line",
     srcs = ["command_line.cpp"],
@@ -196,9 +182,9 @@ cc_binary(
     testonly = 1,
     srcs = ["hashing_benchmark.cpp"],
     deps = [
-        ":benchmark_main",
         ":check",
         ":hashing",
+        "//testing/base:benchmark_main",
         "@abseil-cpp//absl/hash",
         "@abseil-cpp//absl/random",
         "@google_benchmark//:benchmark",

+ 39 - 9
testing/base/BUILD

@@ -19,23 +19,28 @@ cc_library(
     name = "gtest_main",
     testonly = 1,
     srcs = ["gtest_main.cpp"],
-    hdrs = ["gtest_main.h"],
     deps = [
-        "//common:check",
-        "//common:exe_path",
+        ":global_exe_path",
         "//common:init_llvm",
         "@googletest//:gtest",
         "@llvm-project//llvm:Support",
     ],
 )
 
-cc_test(
-    name = "gtest_main_test",
-    size = "small",
-    srcs = ["gtest_main_test.cpp"],
+# This does extra initialization on top of Google benchmark's main in order to
+# provide stack traces and setup LLVM.
+#
+# This replaces `@google_benchmark//:benchmark_main`;
+# `@google_benchmark//:benchmark` should still be used directly.
+cc_library(
+    name = "benchmark_main",
+    testonly = 1,
+    srcs = ["benchmark_main.cpp"],
     deps = [
-        ":gtest_main",
-        "@googletest//:gtest",
+        ":global_exe_path",
+        "//common:init_llvm",
+        "@abseil-cpp//absl/flags:parse",
+        "@google_benchmark//:benchmark",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -60,6 +65,7 @@ cc_test(
     size = "small",
     srcs = ["source_gen_test.cpp"],
     deps = [
+        ":global_exe_path",
         ":gtest_main",
         ":source_gen_lib",
         "//common:set",
@@ -104,3 +110,27 @@ cc_test(
         "@googletest//:gtest",
     ],
 )
+
+cc_library(
+    name = "global_exe_path",
+    testonly = 1,
+    srcs = ["global_exe_path.cpp"],
+    hdrs = ["global_exe_path.h"],
+    deps = [
+        "//common:check",
+        "//common:exe_path",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_test(
+    name = "global_exe_path_test",
+    size = "small",
+    srcs = ["global_exe_path_test.cpp"],
+    deps = [
+        ":global_exe_path",
+        ":gtest_main",
+        "@googletest//:gtest",
+        "@llvm-project//llvm:Support",
+    ],
+)

+ 2 - 25
common/benchmark_main.cpp → testing/base/benchmark_main.cpp

@@ -2,40 +2,17 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#include "common/benchmark_main.h"
-
 #include <benchmark/benchmark.h>
 
-#include <string>
-
 #include "absl/flags/parse.h"
-#include "common/check.h"
-#include "common/exe_path.h"
 #include "common/init_llvm.h"
-#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/StringRef.h"
-
-static bool after_main = false;
-static llvm::StringRef exe_path;
-
-namespace Carbon::Testing {
-
-auto GetBenchmarkExePath() -> llvm::StringRef {
-  CARBON_CHECK(after_main)
-      << "Must not query the executable path until after `main` is entered!";
-  return exe_path;
-}
-
-}  // namespace Carbon::Testing
+#include "testing/base/global_exe_path.h"
 
-// TODO: Refactor this to share code with `gtest_main.cpp`.
 auto main(int orig_argc, char** orig_argv) -> int {
   // Do LLVM's initialization first, this will also transform UTF-16 to UTF-8.
   Carbon::InitLLVM init_llvm(orig_argc, orig_argv);
 
-  std::string exe_path_storage = Carbon::FindExecutablePath(orig_argv[0]);
-  exe_path = exe_path_storage;
-  after_main = true;
+  Carbon::Testing::SetExePath(orig_argv[0]);
 
   // Inject a flag to override the defaults for benchmarks. This can still be
   // disabled by user arguments.

+ 27 - 0
testing/base/global_exe_path.cpp

@@ -0,0 +1,27 @@
+// 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/base/global_exe_path.h"
+
+#include <string>
+
+#include "common/check.h"
+#include "common/exe_path.h"
+
+static constinit std::optional<std::string> exe_path = {};
+
+namespace Carbon::Testing {
+
+auto GetExePath() -> llvm::StringRef {
+  CARBON_CHECK(exe_path)
+      << "Must not query the executable path until after it has been set!";
+  return *exe_path;
+}
+
+auto SetExePath(const char* argv_zero) -> void {
+  CARBON_CHECK(!exe_path) << "Must not call `SetExePath` more than once!";
+  exe_path.emplace(Carbon::FindExecutablePath(argv_zero));
+}
+
+}  // namespace Carbon::Testing

+ 10 - 4
testing/base/gtest_main.h → testing/base/global_exe_path.h

@@ -2,8 +2,8 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#ifndef CARBON_TESTING_BASE_GTEST_MAIN_H_
-#define CARBON_TESTING_BASE_GTEST_MAIN_H_
+#ifndef CARBON_TESTING_BASE_GLOBAL_EXE_PATH_H_
+#define CARBON_TESTING_BASE_GLOBAL_EXE_PATH_H_
 
 #include "llvm/ADT/StringRef.h"
 
@@ -13,8 +13,14 @@
 namespace Carbon::Testing {
 
 // The executable path of the test binary.
-auto GetTestExePath() -> llvm::StringRef;
+auto GetExePath() -> llvm::StringRef;
+
+// Sets the executable path of a test binary from its `argv[0]`.
+//
+// This function must only be called once for an execution, and before any
+// callers to `GetExePath`. Typically, it is called from within `main`.
+auto SetExePath(const char* argv_zero) -> void;
 
 }  // namespace Carbon::Testing
 
-#endif  // CARBON_TESTING_BASE_GTEST_MAIN_H_
+#endif  // CARBON_TESTING_BASE_GLOBAL_EXE_PATH_H_

+ 2 - 2
testing/base/gtest_main_test.cpp → testing/base/global_exe_path_test.cpp

@@ -2,7 +2,7 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#include "testing/base/gtest_main.h"
+#include "testing/base/global_exe_path.h"
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
@@ -15,7 +15,7 @@ namespace {
 using ::testing::StrNe;
 
 TEST(TestExePathTest, Test) {
-  llvm::StringRef exe_path = GetTestExePath();
+  llvm::StringRef exe_path = GetExePath();
   EXPECT_THAT(exe_path, StrNe(""));
   EXPECT_TRUE(llvm::sys::fs::exists(exe_path));
   EXPECT_TRUE(llvm::sys::fs::can_execute(exe_path));

+ 4 - 23
testing/base/gtest_main.cpp

@@ -2,35 +2,16 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#include "testing/base/gtest_main.h"
-
 #include <gtest/gtest.h>
 
-#include <string>
-
-#include "common/check.h"
-#include "common/exe_path.h"
 #include "common/init_llvm.h"
-
-static bool after_main = false;
-static llvm::StringRef exe_path;
-
-namespace Carbon::Testing {
-
-auto GetTestExePath() -> llvm::StringRef {
-  CARBON_CHECK(after_main)
-      << "Must not query the executable path until after `main` is entered!";
-  return exe_path;
-}
-
-}  // namespace Carbon::Testing
+#include "testing/base/global_exe_path.h"
 
 auto main(int argc, char** argv) -> int {
-  std::string exe_path_storage = Carbon::FindExecutablePath(argv[0]);
-  exe_path = exe_path_storage;
-  after_main = true;
-
+  // Initialize LLVM first, as that will also handle ensuring UTF-8 encoding.
   Carbon::InitLLVM init_llvm(argc, argv);
+
+  Carbon::Testing::SetExePath(argv[0]);
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }

+ 2 - 2
testing/base/source_gen_test.cpp

@@ -8,7 +8,7 @@
 #include <gtest/gtest.h>
 
 #include "common/set.h"
-#include "testing/base/gtest_main.h"
+#include "testing/base/global_exe_path.h"
 #include "toolchain/driver/driver.h"
 
 namespace Carbon::Testing {
@@ -144,7 +144,7 @@ TEST(SourceGenTest, UniqueIdentifiers) {
 auto TestCompile(llvm::StringRef source) -> bool {
   llvm::vfs::InMemoryFileSystem fs;
   InstallPaths installation(
-      InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath()));
+      InstallPaths::MakeForBazelRunfiles(Testing::GetExePath()));
   Driver driver(fs, &installation, llvm::outs(), llvm::errs());
 
   // Load the prelude into our VFS.

+ 5 - 1
toolchain/driver/BUILD

@@ -43,6 +43,7 @@ cc_test(
         "//common:all_llvm_targets",
         "//common:check",
         "//common:ostream",
+        "//testing/base:global_exe_path",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
         "@googletest//:gtest",
@@ -58,8 +59,10 @@ cc_binary(
     srcs = ["compile_benchmark.cpp"],
     deps = [
         ":driver",
-        "//common:benchmark_main",
+        "//testing/base:benchmark_main",
+        "//testing/base:global_exe_path",
         "//testing/base:source_gen_lib",
+        "@google_benchmark//:benchmark",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -110,6 +113,7 @@ cc_test(
     deps = [
         ":driver",
         "//common:all_llvm_targets",
+        "//testing/base:global_exe_path",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
         "//toolchain/diagnostics:diagnostic_emitter",

+ 3 - 3
toolchain/driver/clang_runner_test.cpp

@@ -18,7 +18,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Program.h"
 #include "llvm/TargetParser/Host.h"
-#include "testing/base/gtest_main.h"
+#include "testing/base/global_exe_path.h"
 #include "testing/base/test_raw_ostream.h"
 
 namespace Carbon {
@@ -56,7 +56,7 @@ static auto RunWithCapturedOutput(std::string& out, std::string& err,
 TEST(ClangRunnerTest, Version) {
   TestRawOstream test_os;
   const auto install_paths =
-      InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath());
+      InstallPaths::MakeForBazelRunfiles(Testing::GetExePath());
   std::string target = llvm::sys::getDefaultTargetTriple();
   ClangRunner runner(&install_paths, target, &test_os);
 
@@ -125,7 +125,7 @@ TEST(ClangRunnerTest, LinkCommandEcho) {
   std::filesystem::path bar_file = WriteTestFile("bar.o", "");
 
   const auto install_paths =
-      InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath());
+      InstallPaths::MakeForBazelRunfiles(Testing::GetExePath());
   std::string verbose_out;
   llvm::raw_string_ostream verbose_os(verbose_out);
   std::string target = llvm::sys::getDefaultTargetTriple();

+ 2 - 3
toolchain/driver/compile_benchmark.cpp

@@ -6,7 +6,7 @@
 
 #include <string>
 
-#include "common/benchmark_main.h"
+#include "testing/base/global_exe_path.h"
 #include "testing/base/source_gen.h"
 #include "toolchain/driver/driver.h"
 
@@ -20,8 +20,7 @@ namespace {
 class CompileBenchmark {
  public:
   CompileBenchmark()
-      : installation_(
-            InstallPaths::MakeForBazelRunfiles(GetBenchmarkExePath())),
+      : installation_(InstallPaths::MakeForBazelRunfiles(GetExePath())),
         driver_(fs_, &installation_, llvm::outs(), llvm::errs()) {
     // Load the prelude into our VFS.
     //

+ 2 - 2
toolchain/driver/driver_test.cpp

@@ -14,7 +14,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Support/FormatVariadic.h"
-#include "testing/base/gtest_main.h"
+#include "testing/base/global_exe_path.h"
 #include "testing/base/test_raw_ostream.h"
 #include "toolchain/testing/yaml_test_helpers.h"
 
@@ -43,7 +43,7 @@ class DriverTest : public testing::Test {
  protected:
   DriverTest()
       : installation_(
-            InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath())),
+            InstallPaths::MakeForBazelRunfiles(Testing::GetExePath())),
         driver_(fs_, &installation_, test_output_stream_, test_error_stream_) {
     char* tmpdir_env = getenv("TEST_TMPDIR");
     CARBON_CHECK(tmpdir_env != nullptr);

+ 1 - 0
toolchain/install/BUILD

@@ -144,6 +144,7 @@ cc_test(
         ":install_paths",
         "//common:check",
         "//common:ostream",
+        "//testing/base:global_exe_path",
         "//testing/base:gtest_main",
         "@bazel_tools//tools/cpp/runfiles",
         "@googletest//:gtest",

+ 3 - 4
toolchain/install/install_paths_test.cpp

@@ -12,7 +12,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
-#include "testing/base/gtest_main.h"
+#include "testing/base/global_exe_path.h"
 #include "tools/cpp/runfiles/runfiles.h"
 
 namespace Carbon {
@@ -28,8 +28,7 @@ class InstallPathsTest : public ::testing::Test {
  protected:
   InstallPathsTest() {
     std::string error;
-    test_runfiles_.reset(
-        Runfiles::Create(Testing::GetTestExePath().str(), &error));
+    test_runfiles_.reset(Runfiles::Create(Testing::GetExePath().str(), &error));
     CARBON_CHECK(test_runfiles_ != nullptr) << error;
   }
 
@@ -102,7 +101,7 @@ TEST_F(InstallPathsTest, PrefixRootExplicit) {
 }
 
 TEST_F(InstallPathsTest, TestRunfiles) {
-  auto paths = InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath());
+  auto paths = InstallPaths::MakeForBazelRunfiles(Testing::GetExePath());
   ASSERT_THAT(paths.error(), Eq(std::nullopt)) << *paths.error();
   TestInstallPaths(paths);
 }

+ 3 - 3
toolchain/lex/BUILD

@@ -84,8 +84,8 @@ cc_binary(
     srcs = ["numeric_literal_benchmark.cpp"],
     deps = [
         ":numeric_literal",
-        "//common:benchmark_main",
         "//common:check",
+        "//testing/base:benchmark_main",
         "//toolchain/diagnostics:null_diagnostics",
         "@google_benchmark//:benchmark",
     ],
@@ -140,7 +140,7 @@ cc_binary(
     srcs = ["string_literal_benchmark.cpp"],
     deps = [
         ":string_literal",
-        "//common:benchmark_main",
+        "//testing/base:benchmark_main",
         "//toolchain/diagnostics:null_diagnostics",
         "@google_benchmark//:benchmark",
     ],
@@ -287,8 +287,8 @@ cc_binary(
         ":lex",
         ":token_kind",
         ":tokenized_buffer",
-        "//common:benchmark_main",
         "//common:check",
+        "//testing/base:benchmark_main",
         "//testing/base:source_gen_lib",
         "//toolchain/base:value_store",
         "//toolchain/diagnostics:diagnostic_emitter",

+ 1 - 0
toolchain/sem_ir/BUILD

@@ -196,6 +196,7 @@ cc_test(
     srcs = ["yaml_test.cpp"],
     deps = [
         "//common:ostream",
+        "//testing/base:global_exe_path",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
         "//toolchain/driver",

+ 2 - 2
toolchain/sem_ir/yaml_test.cpp

@@ -8,7 +8,7 @@
 #include "common/ostream.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
-#include "testing/base/gtest_main.h"
+#include "testing/base/global_exe_path.h"
 #include "testing/base/test_raw_ostream.h"
 #include "toolchain/driver/driver.h"
 #include "toolchain/testing/yaml_test_helpers.h"
@@ -36,7 +36,7 @@ TEST(SemIRTest, YAML) {
       "test.carbon", /*ModificationTime=*/0,
       llvm::MemoryBuffer::getMemBuffer("fn F() { var x: () = (); return; }")));
   const auto install_paths =
-      InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath());
+      InstallPaths::MakeForBazelRunfiles(Testing::GetExePath());
   TestRawOstream print_stream;
   Driver d(fs, &install_paths, print_stream, llvm::errs());
   auto run_result =