Create an individual instance of RemoteServeConfig in tests

...instead of intialization of the singleton.

7 files changed, 114 insertions, 24 deletions

diff --git a/test/buildtool/build_engine/target_map/TARGETS b/test/buildtool/build_engine/target_map/TARGETS
index 98a41c9a..ac369d22 100644
--- a/test/buildtool/build_engine/target_map/TARGETS
+++ b/test/buildtool/build_engine/target_map/TARGETS
@@ -38,6 +38,7 @@
     , ["@", "src", "src/buildtool/serve_api/remote", "config"]
     , ["@", "src", "src/buildtool/serve_api/remote", "serve_api"]
     , ["utils", "local_hermeticity"]
+    , ["utils", "test_serve_config"]
     ]
   , "stage": ["test", "buildtool", "build_engine", "target_map"]
   }
diff --git a/test/buildtool/build_engine/target_map/target_map.test.cpp b/test/buildtool/build_engine/target_map/target_map.test.cpp
index b7b1e1e3..67f21c0b 100644
--- a/test/buildtool/build_engine/target_map/target_map.test.cpp
+++ b/test/buildtool/build_engine/target_map/target_map.test.cpp
@@ -36,6 +36,7 @@
 #include "src/buildtool/serve_api/remote/serve_api.hpp"
 #include "src/buildtool/storage/storage.hpp"
 #include "test/utils/hermeticity/local.hpp"
+#include "test/utils/serve_service/test_serve_config.hpp"
 
 namespace {
 
@@ -94,7 +95,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "simple targets", "[target_map]") {
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
@@ -533,7 +537,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture,
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
@@ -617,7 +624,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture,
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
@@ -711,7 +721,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "built-in rules", "[target_map]") {
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
@@ -915,7 +928,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "target reference", "[target_map]") {
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
@@ -1052,7 +1068,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "trees", "[target_map]") {
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
@@ -1155,7 +1174,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture,
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
@@ -1315,7 +1337,10 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "wrong arguments", "[target_map]") {
     Statistics stats{};
     Progress exports_progress{};
 
-    auto serve = ServeApi::Create(RemoteServeConfig::Instance());
+    auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(serve_config);
+
+    auto serve = ServeApi::Create(*serve_config);
     AnalyseContext ctx{.repo_config = &repo_config,
                        .target_cache = Storage::Instance().TargetCache(),
                        .statistics = &stats,
diff --git a/test/buildtool/serve_api/TARGETS b/test/buildtool/serve_api/TARGETS
index 5d8af3c0..190f1445 100644
--- a/test/buildtool/serve_api/TARGETS
+++ b/test/buildtool/serve_api/TARGETS
@@ -7,6 +7,8 @@
     [ ["@", "catch2", "", "catch2"]
     , ["utils", "catch-main-serve"]
     , ["@", "src", "src/buildtool/serve_api/remote", "source_tree_client"]
+    , ["utils", "test_serve_config"]
+    , ["@", "src", "src/buildtool/serve_api/remote", "config"]
     ]
   , "stage": ["test", "buildtool", "serve_api"]
   }
diff --git a/test/buildtool/serve_api/source_tree_client.test.cpp b/test/buildtool/serve_api/source_tree_client.test.cpp
index 3de6e864..604471cb 100644
--- a/test/buildtool/serve_api/source_tree_client.test.cpp
+++ b/test/buildtool/serve_api/source_tree_client.test.cpp
@@ -18,6 +18,7 @@
 #include "catch2/catch_test_macros.hpp"
 #include "src/buildtool/serve_api/remote/config.hpp"
 #include "src/buildtool/serve_api/remote/source_tree_client.hpp"
+#include "test/utils/serve_service/test_serve_config.hpp"
 
 auto const kRootCommit =
     std::string{"e4fc610c60716286b98cf51ad0c8f0d50f3aebb5"};
@@ -30,7 +31,9 @@ auto const kRootSymId = std::string{"18770dacfe14c15d88450c21c16668e13ab0e7f9"};
 auto const kBazSymId = std::string{"1868f82682c290f0b1db3cacd092727eef1fa57f"};
 
 TEST_CASE("Serve service client: tree-of-commit request", "[serve_api]") {
-    auto const address = RemoteServeConfig::Instance().RemoteAddress();
+    auto config = TestServeConfig::ReadServeConfigFromEnvironment();
+    REQUIRE(config);
+    auto const address = config->RemoteAddress();
 
     // Create TLC client
     SourceTreeClient st_client(*address);
diff --git a/test/utils/TARGETS b/test/utils/TARGETS
index 5bb8490a..c98da595 100644
--- a/test/utils/TARGETS
+++ b/test/utils/TARGETS
@@ -79,6 +79,18 @@
     ]
   , "stage": ["test", "utils"]
   }
+, "test_serve_config":
+  { "type": ["@", "rules", "CC", "library"]
+  , "name": ["catch-main-remote-execution"]
+  , "hdrs": ["serve_service/test_serve_config.hpp"]
+  , "deps":
+    [ ["@", "src", "src/buildtool/serve_api/remote", "config"]
+    , ["@", "src", "src/buildtool/logging", "log_level"]
+    , ["@", "src", "src/buildtool/logging", "logging"]
+    , "test_env"
+    ]
+  , "stage": ["test", "utils"]
+  }
 , "catch-main-serve":
   { "type": ["@", "rules", "CC", "library"]
   , "name": ["catch-main-serve"]
@@ -95,6 +107,7 @@
     , "shell_quoting"
     , "log_config"
     , "test_env"
+    , "test_serve_config"
     ]
   , "stage": ["test", "utils"]
   }
diff --git a/test/utils/serve_service/main-serve.cpp b/test/utils/serve_service/main-serve.cpp
index 32b0885e..6aec500c 100644
--- a/test/utils/serve_service/main-serve.cpp
+++ b/test/utils/serve_service/main-serve.cpp
@@ -31,6 +31,7 @@
 #include "src/buildtool/serve_api/remote/config.hpp"
 #include "src/buildtool/storage/storage.hpp"
 #include "test/utils/logging/log_config.hpp"
+#include "test/utils/serve_service/test_serve_config.hpp"
 #include "test/utils/shell_quoting.hpp"
 #include "test/utils/test_env.hpp"
 
@@ -78,35 +79,31 @@ void wait_for_grpc_to_shutdown() {
 [[nodiscard]] auto ConfigureServeService() -> bool {
     // just serve shares here compatibility and authentication args with
     // remote execution, so no need to do those again
-    auto address = ReadRemoteServeAddressFromEnv();
-    if (address and
-        not RemoteServeConfig::Instance().SetRemoteAddress(*address)) {
-        Logger::Log(LogLevel::Error, "parsing address '{}' failed.", *address);
-        std::exit(EXIT_FAILURE);
-    }
 
-    auto repos = ReadRemoteServeReposFromEnv();
-    if (not repos.empty() and
-        not RemoteServeConfig::Instance().SetKnownRepositories(repos)) {
-        Logger::Log(LogLevel::Error, "setting serve repos failed.");
-        std::exit(EXIT_FAILURE);
+    // Ensure the config can be read from the environment
+    auto config = TestServeConfig::ReadServeConfigFromEnvironment();
+    if (not config or not config->RemoteAddress()) {
+        return false;
     }
 
     // now actually populate the serve repositories, one bare and one non-bare
-    if (repos.size() != 2) {
+    if (config->KnownRepositories().size() != 2) {
         Logger::Log(LogLevel::Error,
                     "Expected 2 serve repositories in test env.");
         std::exit(EXIT_FAILURE);
     }
-    if (not CreateServeTestRepo(repos[0], kBundlePath, /*is_bare=*/true) or
+
+    auto bare_repo = config->KnownRepositories()[0];
+    auto nonbare_repo = config->KnownRepositories()[1];
+    if (not CreateServeTestRepo(bare_repo, kBundlePath, /*is_bare=*/true) or
         not CreateServeTestRepo(
-            repos[1], kBundlePathSymlinks, /*is_bare=*/false)) {
+            nonbare_repo, kBundlePathSymlinks, /*is_bare=*/false)) {
         Logger::Log(LogLevel::Error,
                     "Failed to setup serve service repositories.");
         std::exit(EXIT_FAILURE);
     }
 
-    return static_cast<bool>(RemoteServeConfig::Instance().RemoteAddress());
+    return true;
 }
 
 }  // namespace
diff --git a/test/utils/serve_service/test_serve_config.hpp b/test/utils/serve_service/test_serve_config.hpp
new file mode 100644
index 00000000..f225c557
--- /dev/null
+++ b/test/utils/serve_service/test_serve_config.hpp
@@ -0,0 +1,49 @@
+// Copyright 2024 Huawei Cloud Computing Technology Co., Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef INCLUDED_SRC_TEST_UTILS_SERVE_SERVICE_TEST_SERVE_CONFIG_HPP
+#define INCLUDED_SRC_TEST_UTILS_SERVE_SERVICE_TEST_SERVE_CONFIG_HPP
+
+#include <optional>
+#include <string>
+#include <variant>
+
+#include "src/buildtool/logging/logger.hpp"
+#include "src/buildtool/serve_api/remote/config.hpp"
+#include "test/utils/test_env.hpp"
+
+class TestServeConfig final {
+  public:
+    [[nodiscard]] static auto ReadServeConfigFromEnvironment() noexcept
+        -> std::optional<RemoteServeConfig> {
+        RemoteServeConfig::Builder builder;
+        auto result = builder.SetRemoteAddress(ReadRemoteServeAddressFromEnv())
+                          .SetKnownRepositories(ReadRemoteServeReposFromEnv())
+                          .Build();
+
+        if (auto* config = std::get_if<RemoteServeConfig>(&result)) {
+            return std::move(*config);
+        }
+
+        if (auto* error = std::get_if<std::string>(&result)) {
+            Logger::Log(LogLevel::Error, *error);
+            return std::nullopt;
+        }
+
+        Logger::Log(LogLevel::Error, "Unknown error occured.");
+        return std::nullopt;
+    }
+};
+
+#endif  // INCLUDED_SRC_TEST_UTILS_SERVE_SERVICE_TEST_SERVE_CONFIG_HPP