Remove redundant ArtifactFactory class used in tests only

...and move the related tests to artifact_description.test

11 files changed, 81 insertions, 119 deletions

diff --git a/test/buildtool/common/TARGETS b/test/buildtool/common/TARGETS
index 986dea0c..c2ce0eac 100644
--- a/test/buildtool/common/TARGETS
+++ b/test/buildtool/common/TARGETS
@@ -1,24 +1,11 @@
-{ "artifact_factory":
-  { "type": ["@", "rules", "CC/test", "test"]
-  , "name": ["artifact_factory"]
-  , "srcs": ["artifact_factory.test.cpp"]
-  , "private-deps":
-    [ ["@", "catch2", "", "catch2"]
-    , ["", "catch-main"]
-    , ["@", "json", "", "json"]
-    , ["@", "src", "src/buildtool/common", "artifact_description"]
-    , ["@", "src", "src/buildtool/file_system", "object_type"]
-    ]
-  , "stage": ["test", "buildtool", "common"]
-  }
-, "artifact_description":
+{ "artifact_description":
   { "type": ["@", "rules", "CC/test", "test"]
   , "name": ["artifact_description"]
   , "srcs": ["artifact_description.test.cpp"]
   , "private-deps":
     [ ["@", "catch2", "", "catch2"]
     , ["", "catch-main"]
-    , ["@", "src", "src/buildtool/common", "artifact_factory"]
+    , ["@", "json", "", "json"]
     , ["@", "src", "src/buildtool/common", "artifact_description"]
     , ["@", "src", "src/buildtool/common", "common"]
     , ["@", "src", "src/buildtool/file_system", "object_type"]
@@ -33,7 +20,6 @@
     [ ["@", "catch2", "", "catch2"]
     , ["", "catch-main"]
     , ["@", "json", "", "json"]
-    , ["@", "src", "src/buildtool/common", "artifact_factory"]
     , ["@", "src", "src/buildtool/common", "action_description"]
     , ["@", "src", "src/buildtool/common", "common"]
     ]
@@ -77,7 +63,6 @@
   , "deps":
     [ "action_description"
     , "artifact_description"
-    , "artifact_factory"
     , "repository_config"
     , "artifact_object_info"
     ]
diff --git a/test/buildtool/common/action_description.test.cpp b/test/buildtool/common/action_description.test.cpp
index 9d82ec9d..c79caeaa 100644
--- a/test/buildtool/common/action_description.test.cpp
+++ b/test/buildtool/common/action_description.test.cpp
@@ -19,7 +19,6 @@
 #include "src/buildtool/common/action.hpp"
 #include "src/buildtool/common/action_description.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
-#include "src/buildtool/common/artifact_factory.hpp"
 
 TEST_CASE("From JSON", "[action_description]") {
     using path = std::filesystem::path;
@@ -30,11 +29,12 @@ TEST_CASE("From JSON", "[action_description]") {
         {{"path0", ArtifactDescription::CreateTree(path{"input0"})},
          {"path1", ArtifactDescription::CreateTree(path{"input1"})}}};
     auto const& action = desc.GraphAction();
-    auto json = ArtifactFactory::DescribeAction(desc.OutputFiles(),
-                                                desc.OutputDirs(),
-                                                action.Command(),
-                                                desc.Inputs(),
-                                                action.Env());
+    auto json =
+        ActionDescription{desc.OutputFiles(),
+                          desc.OutputDirs(),
+                          Action{"unused", action.Command(), action.Env()},
+                          desc.Inputs()}
+            .ToJson();
 
     SECTION("Parse full action") {
         auto description = ActionDescription::FromJson("id", json);
diff --git a/test/buildtool/common/artifact_description.test.cpp b/test/buildtool/common/artifact_description.test.cpp
index c8fffc46..76e49e34 100644
--- a/test/buildtool/common/artifact_description.test.cpp
+++ b/test/buildtool/common/artifact_description.test.cpp
@@ -16,9 +16,9 @@
 #include <string>
 
 #include "catch2/catch_test_macros.hpp"
+#include "nlohmann/json.hpp"
 #include "src/buildtool/common/artifact.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
-#include "src/buildtool/common/artifact_factory.hpp"
 #include "src/buildtool/file_system/object_type.hpp"
 
 [[nodiscard]] auto operator==(Artifact const& lhs, Artifact const& rhs)
@@ -66,10 +66,12 @@ TEST_CASE("Action artifact", "[artifact_description]") {
 }
 
 TEST_CASE("From JSON", "[artifact_description]") {
-    auto local = ArtifactFactory::DescribeLocalArtifact("local", "repo");
+    auto local = ArtifactDescription::CreateLocal("local", "repo").ToJson();
     auto known =
-        ArtifactFactory::DescribeKnownArtifact("hash", 0, ObjectType::File);
-    auto action = ArtifactFactory::DescribeActionArtifact("id", "output");
+        ArtifactDescription::CreateKnown(
+            ArtifactDigest{"hash", 0, /*is_tree=*/false}, ObjectType::File)
+            .ToJson();
+    auto action = ArtifactDescription::CreateAction("id", "output").ToJson();
 
     SECTION("Parse artifacts") {
         CHECK(ArtifactDescription::FromJson(local));
@@ -144,3 +146,53 @@ TEST_CASE("From JSON", "[artifact_description]") {
         }
     }
 }
+
+TEST_CASE("Description missing mandatory key/value pair",
+          "[artifact_description]") {
+    nlohmann::json const missing_type = {{"data", {{"path", "some/path"}}}};
+    CHECK(not ArtifactDescription::FromJson(missing_type));
+    nlohmann::json const missing_data = {{"type", "LOCAL"}};
+    CHECK(not ArtifactDescription::FromJson(missing_data));
+}
+
+TEST_CASE("Local artifact description contains incorrect value for \"data\"",
+          "[artifact_description]") {
+    nlohmann::json const local_art_missing_path = {
+        {"type", "LOCAL"}, {"data", nlohmann::json::object()}};
+    CHECK(not ArtifactDescription::FromJson(local_art_missing_path));
+}
+
+TEST_CASE("Known artifact description contains incorrect value for \"data\"",
+          "[artifact_description]") {
+    std::string file_type{};
+    file_type += ToChar(ObjectType::File);
+    SECTION("missing \"id\"") {
+        nlohmann::json const known_art_missing_id = {
+            {"type", "KNOWN"},
+            {"data", {{"size", 15}, {"file_type", file_type}}}};
+        CHECK(not ArtifactDescription::FromJson(known_art_missing_id));
+    }
+    SECTION("missing \"size\"") {
+        nlohmann::json const known_art_missing_size = {
+            {"type", "KNOWN"},
+            {"data", {{"id", "known_input"}, {"file_type", file_type}}}};
+        CHECK(not ArtifactDescription::FromJson(known_art_missing_size));
+    }
+    SECTION("missing \"file_type\"") {
+        nlohmann::json const known_art_missing_file_type = {
+            {"type", "KNOWN"}, {"data", {{"id", "known_input"}, {"size", 15}}}};
+
+        CHECK(not ArtifactDescription::FromJson(known_art_missing_file_type));
+    }
+}
+
+TEST_CASE("Action artifact description contains incorrect value for \"data\"",
+          "[artifact_description]") {
+    nlohmann::json const action_art_missing_id = {
+        {"type", "ACTION"}, {"data", {{"path", "output/path"}}}};
+    CHECK(not ArtifactDescription::FromJson(action_art_missing_id));
+
+    nlohmann::json const action_art_missing_path = {
+        {"type", "ACTION"}, {"data", {{"id", "action_id"}}}};
+    CHECK(not ArtifactDescription::FromJson(action_art_missing_path));
+}
diff --git a/test/buildtool/common/artifact_factory.test.cpp b/test/buildtool/common/artifact_factory.test.cpp
deleted file mode 100644
index 222ffd2b..00000000
--- a/test/buildtool/common/artifact_factory.test.cpp
+++ /dev/null
@@ -1,70 +0,0 @@
-// Copyright 2022 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.
-
-#include <string>
-
-#include "catch2/catch_test_macros.hpp"
-#include "nlohmann/json.hpp"
-#include "src/buildtool/common/artifact_description.hpp"
-#include "src/buildtool/file_system/object_type.hpp"
-
-TEST_CASE("Description missing mandatory key/value pair",
-          "[artifact_factory]") {
-    nlohmann::json const missing_type = {{"data", {{"path", "some/path"}}}};
-    CHECK(not ArtifactDescription::FromJson(missing_type));
-    nlohmann::json const missing_data = {{"type", "LOCAL"}};
-    CHECK(not ArtifactDescription::FromJson(missing_data));
-}
-
-TEST_CASE("Local artifact description contains incorrect value for \"data\"",
-          "[artifact_factory]") {
-    nlohmann::json const local_art_missing_path = {
-        {"type", "LOCAL"}, {"data", nlohmann::json::object()}};
-    CHECK(not ArtifactDescription::FromJson(local_art_missing_path));
-}
-
-TEST_CASE("Known artifact description contains incorrect value for \"data\"",
-          "[artifact_factory]") {
-    std::string file_type{};
-    file_type += ToChar(ObjectType::File);
-    SECTION("missing \"id\"") {
-        nlohmann::json const known_art_missing_id = {
-            {"type", "KNOWN"},
-            {"data", {{"size", 15}, {"file_type", file_type}}}};
-        CHECK(not ArtifactDescription::FromJson(known_art_missing_id));
-    }
-    SECTION("missing \"size\"") {
-        nlohmann::json const known_art_missing_size = {
-            {"type", "KNOWN"},
-            {"data", {{"id", "known_input"}, {"file_type", file_type}}}};
-        CHECK(not ArtifactDescription::FromJson(known_art_missing_size));
-    }
-    SECTION("missing \"file_type\"") {
-        nlohmann::json const known_art_missing_file_type = {
-            {"type", "KNOWN"}, {"data", {{"id", "known_input"}, {"size", 15}}}};
-
-        CHECK(not ArtifactDescription::FromJson(known_art_missing_file_type));
-    }
-}
-
-TEST_CASE("Action artifact description contains incorrect value for \"data\"",
-          "[artifact_factory]") {
-    nlohmann::json const action_art_missing_id = {
-        {"type", "ACTION"}, {"data", {{"path", "output/path"}}}};
-    CHECK(not ArtifactDescription::FromJson(action_art_missing_id));
-
-    nlohmann::json const action_art_missing_path = {
-        {"type", "ACTION"}, {"data", {{"id", "action_id"}}}};
-    CHECK(not ArtifactDescription::FromJson(action_art_missing_path));
-}
diff --git a/test/buildtool/execution_engine/dag/TARGETS b/test/buildtool/execution_engine/dag/TARGETS
index 0f4257ae..7b30cab4 100644
--- a/test/buildtool/execution_engine/dag/TARGETS
+++ b/test/buildtool/execution_engine/dag/TARGETS
@@ -9,7 +9,6 @@
     , ["utils", "container_matchers"]
     , ["@", "src", "src/buildtool/common", "action_description"]
     , ["@", "src", "src/buildtool/common", "artifact_description"]
-    , ["@", "src", "src/buildtool/common", "artifact_factory"]
     , ["@", "src", "src/buildtool/common", "common"]
     , ["@", "src", "src/buildtool/execution_engine/dag", "dag"]
     ]
diff --git a/test/buildtool/execution_engine/dag/dag.test.cpp b/test/buildtool/execution_engine/dag/dag.test.cpp
index ba5a122e..8a88775d 100644
--- a/test/buildtool/execution_engine/dag/dag.test.cpp
+++ b/test/buildtool/execution_engine/dag/dag.test.cpp
@@ -24,7 +24,6 @@
 #include "src/buildtool/common/action.hpp"
 #include "src/buildtool/common/action_description.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
-#include "src/buildtool/common/artifact_factory.hpp"
 #include "src/buildtool/common/identifier.hpp"
 #include "src/buildtool/execution_engine/dag/dag.hpp"
 #include "test/utils/container_matchers.hpp"
diff --git a/test/buildtool/execution_engine/executor/TARGETS b/test/buildtool/execution_engine/executor/TARGETS
index 45f981d4..1bd82e69 100644
--- a/test/buildtool/execution_engine/executor/TARGETS
+++ b/test/buildtool/execution_engine/executor/TARGETS
@@ -5,7 +5,6 @@
   , "deps":
     [ ["@", "src", "src/buildtool/auth", "auth"]
     , ["@", "src", "src/buildtool/common", "artifact_description"]
-    , ["@", "src", "src/buildtool/common", "artifact_factory"]
     , ["@", "src", "src/buildtool/common", "common"]
     , ["@", "src", "src/buildtool/execution_api/common", "common"]
     , ["@", "src", "src/buildtool/execution_api/remote", "config"]
@@ -25,7 +24,6 @@
   , "private-deps":
     [ ["@", "src", "src/buildtool/auth", "auth"]
     , ["@", "src", "src/buildtool/common", "artifact_description"]
-    , ["@", "src", "src/buildtool/common", "artifact_factory"]
     , ["@", "src", "src/buildtool/common", "common"]
     , ["@", "src", "src/buildtool/common", "config"]
     , ["@", "src", "src/buildtool/execution_api/common", "common"]
diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp
index 919ae998..f58f4ef4 100644
--- a/test/buildtool/execution_engine/executor/executor.test.cpp
+++ b/test/buildtool/execution_engine/executor/executor.test.cpp
@@ -25,7 +25,6 @@
 #include "gsl/gsl"
 #include "src/buildtool/auth/authentication.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
-#include "src/buildtool/common/artifact_factory.hpp"
 #include "src/buildtool/common/repository_config.hpp"
 #include "src/buildtool/common/statistics.hpp"
 #include "src/buildtool/execution_api/common/execution_api.hpp"
diff --git a/test/buildtool/execution_engine/executor/executor_api.test.hpp b/test/buildtool/execution_engine/executor/executor_api.test.hpp
index 18e30c70..a57c6f36 100644
--- a/test/buildtool/execution_engine/executor/executor_api.test.hpp
+++ b/test/buildtool/execution_engine/executor/executor_api.test.hpp
@@ -26,7 +26,6 @@
 #include "gsl/gsl"
 #include "src/buildtool/common/artifact.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
-#include "src/buildtool/common/artifact_factory.hpp"
 #include "src/buildtool/common/repository_config.hpp"
 #include "src/buildtool/common/statistics.hpp"
 #include "src/buildtool/execution_api/common/execution_api.hpp"
diff --git a/test/buildtool/execution_engine/traverser/TARGETS b/test/buildtool/execution_engine/traverser/TARGETS
index 8dac8992..02294710 100644
--- a/test/buildtool/execution_engine/traverser/TARGETS
+++ b/test/buildtool/execution_engine/traverser/TARGETS
@@ -8,7 +8,6 @@
     , ["utils", "container_matchers"]
     , ["@", "src", "src/buildtool/common", "common"]
     , ["@", "src", "src/buildtool/common", "artifact_description"]
-    , ["@", "src", "src/buildtool/common", "artifact_factory"]
     , ["@", "src", "src/buildtool/execution_engine/dag", "dag"]
     , ["@", "src", "src/buildtool/execution_engine/traverser", "traverser"]
     ]
diff --git a/test/buildtool/execution_engine/traverser/traverser.test.cpp b/test/buildtool/execution_engine/traverser/traverser.test.cpp
index 2aaa4228..c500a0d2 100644
--- a/test/buildtool/execution_engine/traverser/traverser.test.cpp
+++ b/test/buildtool/execution_engine/traverser/traverser.test.cpp
@@ -22,7 +22,6 @@
 
 #include "catch2/catch_test_macros.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
-#include "src/buildtool/common/artifact_factory.hpp"
 #include "src/buildtool/execution_engine/dag/dag.hpp"
 #include "src/buildtool/execution_engine/traverser/traverser.hpp"
 #include "test/utils/container_matchers.hpp"
@@ -31,6 +30,9 @@ namespace {
 
 auto const kNumJobs = std::max(1U, std::thread::hardware_concurrency());
 
+[[nodiscard]] auto IsArtifactLocal(nlohmann::json const& j) -> bool {
+    return j.at("type") == "LOCAL";
+}
 class TestBuildInfo {
   public:
     [[nodiscard]] auto CorrectlyBuilt() const noexcept
@@ -185,7 +187,7 @@ class TestProject {
                 auto const input_id = artifact->Id();
                 command.push_back(input_id);
                 inputs_desc.emplace(input_id, *artifact);
-                if (ArtifactFactory::IsLocal(input_desc)) {
+                if (IsArtifactLocal(input_desc)) {
                     local_artifacts_.insert(input_id);
                 }
             }
@@ -222,7 +224,7 @@ TEST_CASE("Executable", "[traverser]") {
     CHECK(p.AddOutputInputPair(
         "action",
         {"executable"},
-        {ArtifactFactory::DescribeLocalArtifact("main.cpp", "")}));
+        {ArtifactDescription::CreateLocal("main.cpp", "").ToJson()}));
     DependencyGraph g;
     CHECK(p.FillGraph(&g));
     TestBuildInfo build_info;
@@ -277,13 +279,13 @@ TEST_CASE("Executable depends on library", "[traverser]") {
     CHECK(p.AddOutputInputPair(
         "make_exe",
         {"executable"},
-        {ArtifactFactory::DescribeLocalArtifact("main.cpp", "repo"),
-         ArtifactFactory::DescribeActionArtifact("make_lib", "library")}));
+        {ArtifactDescription::CreateLocal("main.cpp", "repo").ToJson(),
+         ArtifactDescription::CreateAction("make_lib", "library").ToJson()}));
     CHECK(p.AddOutputInputPair(
         "make_lib",
         {"library"},
-        {ArtifactFactory::DescribeLocalArtifact("library.hpp", "repo"),
-         ArtifactFactory::DescribeLocalArtifact("library.cpp", "repo")}));
+        {ArtifactDescription::CreateLocal("library.hpp", "repo").ToJson(),
+         ArtifactDescription::CreateLocal("library.cpp", "repo").ToJson()}));
     DependencyGraph g;
     CHECK(p.FillGraph(&g));
     TestBuildInfo build_info;
@@ -369,8 +371,8 @@ TEST_CASE("Two artifacts depend on another", "[traverser]") {
     CHECK(p.AddOutputInputPair(
         "make_dep",
         {"dep"},
-        {ArtifactFactory::DescribeLocalArtifact("leaf1", "repo"),
-         ArtifactFactory::DescribeLocalArtifact("leaf2", "repo")}));
+        {ArtifactDescription::CreateLocal("leaf1", "repo").ToJson(),
+         ArtifactDescription::CreateLocal("leaf2", "repo").ToJson()}));
     DependencyGraph g;
     CHECK(p.FillGraph(&g));
     TestBuildInfo build_info;
@@ -497,7 +499,7 @@ TEST_CASE("Action with two outputs, one dep", "[traverser]") {
     CHECK(p.AddOutputInputPair(
         "make_outputs",
         {"output1", "output2"},
-        {ArtifactFactory::DescribeLocalArtifact("dep", "repo")}));
+        {ArtifactDescription::CreateLocal("dep", "repo").ToJson()}));
     auto const output1_id =
         ArtifactDescription::CreateAction("make_outputs", "output1").Id();
     auto const output2_id =
@@ -710,21 +712,21 @@ TEST_CASE("lib2 depends on lib1, executable depends on lib1 and lib2") {
     CHECK(p.AddOutputInputPair(
         "make_exe",
         {"executable"},
-        {ArtifactFactory::DescribeLocalArtifact("main.cpp", "repo"),
+        {ArtifactDescription::CreateLocal("main.cpp", "repo").ToJson(),
          lib1_desc,
          lib2_desc}));
 
     CHECK(p.AddOutputInputPair(
         "make_lib1",
         {"lib1"},
-        {ArtifactFactory::DescribeLocalArtifact("lib1.hpp", "repo"),
-         ArtifactFactory::DescribeLocalArtifact("lib1.cpp", "repo")}));
+        {ArtifactDescription::CreateLocal("lib1.hpp", "repo").ToJson(),
+         ArtifactDescription::CreateLocal("lib1.cpp", "repo").ToJson()}));
     CHECK(p.AddOutputInputPair(
         "make_lib2",
         {"lib2"},
         {lib1_desc,
-         ArtifactFactory::DescribeLocalArtifact("lib2.hpp", "repo"),
-         ArtifactFactory::DescribeLocalArtifact("lib2.cpp", "repo")}));
+         ArtifactDescription::CreateLocal("lib2.hpp", "repo").ToJson(),
+         ArtifactDescription::CreateLocal("lib2.cpp", "repo").ToJson()}));
 
     DependencyGraph g;
     CHECK(p.FillGraph(&g));