summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/buildtool/common/artifact_factory.hpp9
-rw-r--r--test/buildtool/execution_engine/dag/TARGETS1
-rw-r--r--test/buildtool/execution_engine/dag/dag.test.cpp25
-rw-r--r--test/buildtool/execution_engine/executor/TARGETS1
-rw-r--r--test/buildtool/execution_engine/executor/executor.test.cpp37
-rw-r--r--test/buildtool/execution_engine/traverser/TARGETS1
-rw-r--r--test/buildtool/execution_engine/traverser/traverser.test.cpp110
7 files changed, 90 insertions, 94 deletions
diff --git a/src/buildtool/common/artifact_factory.hpp b/src/buildtool/common/artifact_factory.hpp
index 0ab0efc3..dccbe221 100644
--- a/src/buildtool/common/artifact_factory.hpp
+++ b/src/buildtool/common/artifact_factory.hpp
@@ -34,15 +34,6 @@
class ArtifactFactory {
public:
- [[nodiscard]] static auto Identifier(nlohmann::json const& description)
- -> ArtifactIdentifier {
- auto desc = ArtifactDescription::FromJson(description);
- if (desc) {
- return desc->Id();
- }
- return {};
- }
-
[[nodiscard]] static auto FromDescription(nlohmann::json const& description)
-> std::optional<Artifact> {
auto desc = ArtifactDescription::FromJson(description);
diff --git a/test/buildtool/execution_engine/dag/TARGETS b/test/buildtool/execution_engine/dag/TARGETS
index ea37af31..0f4257ae 100644
--- a/test/buildtool/execution_engine/dag/TARGETS
+++ b/test/buildtool/execution_engine/dag/TARGETS
@@ -8,6 +8,7 @@
, ["@", "gsl", "", "gsl"]
, ["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 cac4b791..ba5a122e 100644
--- a/test/buildtool/execution_engine/dag/dag.test.cpp
+++ b/test/buildtool/execution_engine/dag/dag.test.cpp
@@ -23,6 +23,7 @@
#include "gsl/gsl"
#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"
@@ -45,8 +46,8 @@ void CheckOutputNodesCorrectlyAdded(
std::vector<std::string> const& output_paths) {
std::vector<ArtifactIdentifier> output_ids;
for (auto const& path : output_paths) {
- auto const output_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact(action_id, path));
+ auto const output_id =
+ ArtifactDescription::CreateAction(action_id, path).Id();
CHECK(g.ArtifactNodeWithId(output_id) != nullptr);
auto const* action = GetActionOfArtifact(g, output_id);
CHECK(action != nullptr);
@@ -158,12 +159,11 @@ TEST_CASE("AddAction({single action, single output, source file})", "[dag]") {
CheckLocalArtifactsCorrectlyAdded(g, {src_id}, {"main.cpp"});
// All artifacts are the source file and the executable
- CHECK_THAT(g.ArtifactIdentifiers(),
- HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(
- {src_id,
- ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact(
- action_id, "executable"))}));
+ CHECK_THAT(
+ g.ArtifactIdentifiers(),
+ HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(
+ {src_id,
+ ArtifactDescription::CreateAction(action_id, "executable").Id()}));
CHECK(IsValidGraph(g));
}
@@ -189,11 +189,10 @@ TEST_CASE("AddAction({single action, single output, no inputs, env_variables})",
CHECK(action_node->Env() == env_vars);
// All artifacts are the output file
- CHECK_THAT(g.ArtifactIdentifiers(),
- HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(
- {ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact(action_id,
- "greeting"))}));
+ CHECK_THAT(
+ g.ArtifactIdentifiers(),
+ HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(
+ {ArtifactDescription::CreateAction(action_id, "greeting").Id()}));
CHECK(IsValidGraph(g));
}
diff --git a/test/buildtool/execution_engine/executor/TARGETS b/test/buildtool/execution_engine/executor/TARGETS
index 2d82e097..45f981d4 100644
--- a/test/buildtool/execution_engine/executor/TARGETS
+++ b/test/buildtool/execution_engine/executor/TARGETS
@@ -24,6 +24,7 @@
, "srcs": ["executor.test.cpp"]
, "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"]
diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp
index 9dc2f667..919ae998 100644
--- a/test/buildtool/execution_engine/executor/executor.test.cpp
+++ b/test/buildtool/execution_engine/executor/executor.test.cpp
@@ -24,6 +24,7 @@
#include "catch2/catch_test_macros.hpp"
#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"
@@ -270,13 +271,13 @@ TEST_CASE("Executor: Process artifact", "[executor]") {
DependencyGraph g;
auto [config, repo_config] = CreateTest(&g, workspace_path);
- auto const local_cpp_desc =
- ArtifactFactory::DescribeLocalArtifact("local.cpp", "");
- auto const local_cpp_id = ArtifactFactory::Identifier(local_cpp_desc);
+ auto const local_cpp_id =
+ ArtifactDescription::CreateLocal("local.cpp", "").Id();
- auto const known_cpp_desc = ArtifactFactory::DescribeKnownArtifact(
- "known.cpp", 0, ObjectType::File);
- auto const known_cpp_id = ArtifactFactory::Identifier(known_cpp_desc);
+ auto const known_cpp_id =
+ ArtifactDescription::CreateKnown(
+ ArtifactDigest{"known.cpp", 0, /*is_tree=*/false}, ObjectType::File)
+ .Id();
SECTION("Processing succeeds for valid config") {
auto api = TestApi::Ptr{new TestApi{config}};
@@ -344,22 +345,20 @@ TEST_CASE("Executor: Process action", "[executor]") {
DependencyGraph g;
auto [config, repo_config] = CreateTest(&g, workspace_path);
- auto const local_cpp_desc =
- ArtifactFactory::DescribeLocalArtifact("local.cpp", "");
- auto const local_cpp_id = ArtifactFactory::Identifier(local_cpp_desc);
+ auto const local_cpp_id =
+ ArtifactDescription::CreateLocal("local.cpp", "").Id();
- auto const known_cpp_desc = ArtifactFactory::DescribeKnownArtifact(
- "known.cpp", 0, ObjectType::File);
- auto const known_cpp_id = ArtifactFactory::Identifier(known_cpp_desc);
+ auto const known_cpp_id =
+ ArtifactDescription::CreateKnown(
+ ArtifactDigest{"known.cpp", 0, /*is_tree=*/false}, ObjectType::File)
+ .Id();
- ActionIdentifier action_id{"test_action"};
- auto const output1_desc =
- ArtifactFactory::DescribeActionArtifact(action_id, "output1.exe");
- auto const output1_id = ArtifactFactory::Identifier(output1_desc);
+ ActionIdentifier const action_id{"test_action"};
+ auto const output1_id =
+ ArtifactDescription::CreateAction(action_id, "output1.exe").Id();
- auto const output2_desc =
- ArtifactFactory::DescribeActionArtifact(action_id, "output2.exe");
- auto const output2_id = ArtifactFactory::Identifier(output2_desc);
+ auto const output2_id =
+ ArtifactDescription::CreateAction(action_id, "output2.exe").Id();
SECTION("Processing succeeds for valid config") {
auto api = TestApi::Ptr{new TestApi{config}};
diff --git a/test/buildtool/execution_engine/traverser/TARGETS b/test/buildtool/execution_engine/traverser/TARGETS
index 77603634..8dac8992 100644
--- a/test/buildtool/execution_engine/traverser/TARGETS
+++ b/test/buildtool/execution_engine/traverser/TARGETS
@@ -7,6 +7,7 @@
, ["", "catch-main"]
, ["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 150f3d8c..2aaa4228 100644
--- a/test/buildtool/execution_engine/traverser/traverser.test.cpp
+++ b/test/buildtool/execution_engine/traverser/traverser.test.cpp
@@ -21,6 +21,7 @@
#include <vector>
#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"
@@ -252,9 +253,8 @@ TEST_CASE("Executable", "[traverser]") {
TestExecutor runner{&build_info};
Traverser traverser(runner, g, kNumJobs, &failed);
- auto const exec_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("action",
- "executable"));
+ auto const exec_id =
+ ArtifactDescription::CreateAction("action", "executable").Id();
auto const traversed = traverser.Traverse({exec_id});
CHECK(traversed);
}
@@ -313,9 +313,9 @@ TEST_CASE("Executable depends on library", "[traverser]") {
{
TestExecutor runner{&build_info};
Traverser traverser(runner, g, kNumJobs, &failed);
- auto const exec_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("make_exe",
- "executable"));
+ auto const exec_id =
+ ArtifactDescription::CreateAction("make_exe", "executable")
+ .Id();
CHECK(traverser.Traverse({exec_id}));
}
CHECK_FALSE(failed);
@@ -332,8 +332,8 @@ TEST_CASE("Executable depends on library", "[traverser]") {
CHECK(build_info.Name() == name);
}
SECTION("Only build library") {
- auto const lib_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("make_lib", "library"));
+ auto const lib_id =
+ ArtifactDescription::CreateAction("make_lib", "library").Id();
{
TestExecutor runner{&build_info};
Traverser traverser(runner, g, kNumJobs, &failed);
@@ -345,10 +345,10 @@ TEST_CASE("Executable depends on library", "[traverser]") {
HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(
{lib_id}));
CHECK(build_info.IncorrectlyBuilt().empty());
- auto const lib_cpp_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeLocalArtifact("library.cpp", "repo"));
- auto const lib_hpp_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeLocalArtifact("library.hpp", "repo"));
+ auto const lib_cpp_id =
+ ArtifactDescription::CreateLocal("library.cpp", "repo").Id();
+ auto const lib_hpp_id =
+ ArtifactDescription::CreateLocal("library.hpp", "repo").Id();
CHECK_THAT(
build_info.ArtifactsUploaded(),
HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(
@@ -360,9 +360,10 @@ TEST_CASE("Executable depends on library", "[traverser]") {
TEST_CASE("Two artifacts depend on another", "[traverser]") {
TestProject p;
- auto const dep_desc =
- ArtifactFactory::DescribeActionArtifact("make_dep", "dep");
- auto const dep_id = ArtifactFactory::Identifier(dep_desc);
+ auto const description =
+ ArtifactDescription::CreateAction("make_dep", "dep");
+ auto const dep_desc = description.ToJson();
+ auto const& dep_id = description.Id();
CHECK(p.AddOutputInputPair("action1", {"toplevel1"}, {dep_desc}));
CHECK(p.AddOutputInputPair("action2", {"toplevel2"}, {dep_desc}));
CHECK(p.AddOutputInputPair(
@@ -396,8 +397,8 @@ TEST_CASE("Two artifacts depend on another", "[traverser]") {
CHECK(build_info.Name() == name);
}
SECTION("Only specified top-level artifact is built") {
- auto const toplevel1_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("action1", "toplevel1"));
+ auto const toplevel1_id =
+ ArtifactDescription::CreateAction("action1", "toplevel1").Id();
{
TestExecutor runner{&build_info};
Traverser traverser(runner, g, kNumJobs, &failed);
@@ -421,10 +422,10 @@ TEST_CASE("Two artifacts depend on another", "[traverser]") {
TEST_CASE("Action with two outputs, no deps", "[traverser]") {
TestProject p;
CHECK(p.AddOutputInputPair("make_outputs", {"output1", "output2"}, {}));
- auto const output1_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("make_outputs", "output1"));
- auto const output2_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("make_outputs", "output2"));
+ auto const output1_id =
+ ArtifactDescription::CreateAction("make_outputs", "output1").Id();
+ auto const output2_id =
+ ArtifactDescription::CreateAction("make_outputs", "output2").Id();
DependencyGraph g;
CHECK(p.FillGraph(&g));
TestBuildInfo build_info;
@@ -497,10 +498,10 @@ TEST_CASE("Action with two outputs, one dep", "[traverser]") {
"make_outputs",
{"output1", "output2"},
{ArtifactFactory::DescribeLocalArtifact("dep", "repo")}));
- auto const output1_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("make_outputs", "output1"));
- auto const output2_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("make_outputs", "output2"));
+ auto const output1_id =
+ ArtifactDescription::CreateAction("make_outputs", "output1").Id();
+ auto const output2_id =
+ ArtifactDescription::CreateAction("make_outputs", "output2").Id();
DependencyGraph g;
CHECK(p.FillGraph(&g));
TestBuildInfo build_info;
@@ -565,8 +566,8 @@ TEST_CASE("Action with two outputs, one dep", "[traverser]") {
CHECK(build_info.Name() == name);
}
SECTION("Traverse(dep, output2)") {
- auto const dep_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeLocalArtifact("dep", "repo"));
+ auto const dep_id =
+ ArtifactDescription::CreateLocal("dep", "repo").Id();
{
TestExecutor runner{&build_info};
Traverser traverser(runner, g, kNumJobs, &failed);
@@ -591,20 +592,23 @@ TEST_CASE("Action with two outputs, actions depend on each of outputs",
"[traverser]") {
TestProject p;
CHECK(p.AddOutputInputPair("make_outputs", {"output1", "output2"}, {}));
- auto const output1_desc =
- ArtifactFactory::DescribeActionArtifact("make_outputs", "output1");
- auto const output1_id = ArtifactFactory::Identifier(output1_desc);
- auto const output2_desc =
- ArtifactFactory::DescribeActionArtifact("make_outputs", "output2");
- auto const output2_id = ArtifactFactory::Identifier(output2_desc);
+ auto const desc_1 =
+ ArtifactDescription::CreateAction("make_outputs", "output1");
+ auto const output1_desc = desc_1.ToJson();
+ auto const& output1_id = desc_1.Id();
+
+ auto const desc_2 =
+ ArtifactDescription::CreateAction("make_outputs", "output2");
+ auto const output2_desc = desc_2.ToJson();
+ auto const& output2_id = desc_2.Id();
CHECK(p.AddOutputInputPair("consumer1", {"exec1"}, {output1_desc}));
- auto const exec1_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("consumer1", "exec1"));
+ auto const exec1_id =
+ ArtifactDescription::CreateAction("consumer1", "exec1").Id();
CHECK(p.AddOutputInputPair("consumer2", {"exec2"}, {output2_desc}));
- auto const exec2_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("consumer2", "exec2"));
+ auto const exec2_id =
+ ArtifactDescription::CreateAction("consumer2", "exec2").Id();
DependencyGraph g;
CHECK(p.FillGraph(&g));
@@ -692,16 +696,16 @@ TEST_CASE("Action with two outputs, actions depend on each of outputs",
TEST_CASE("lib2 depends on lib1, executable depends on lib1 and lib2") {
TestProject p;
- auto const lib1_desc =
- ArtifactFactory::DescribeActionArtifact("make_lib1", "lib1");
- auto const lib1_id = ArtifactFactory::Identifier(lib1_desc);
+ auto const desc_1 = ArtifactDescription::CreateAction("make_lib1", "lib1");
+ auto const lib1_desc = desc_1.ToJson();
+ auto const& lib1_id = desc_1.Id();
- auto const lib2_desc =
- ArtifactFactory::DescribeActionArtifact("make_lib2", "lib2");
- auto const lib2_id = ArtifactFactory::Identifier(lib2_desc);
+ auto const desc_2 = ArtifactDescription::CreateAction("make_lib2", "lib2");
+ auto const lib2_desc = desc_2.ToJson();
+ auto const& lib2_id = desc_2.Id();
- auto const exec_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeActionArtifact("make_exe", "executable"));
+ auto const exec_id =
+ ArtifactDescription::CreateAction("make_exe", "executable").Id();
CHECK(p.AddOutputInputPair(
"make_exe",
@@ -858,14 +862,14 @@ TEST_CASE("lib2 depends on lib1, executable depends on lib1 and lib2") {
HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(
{lib1_id, lib2_id}));
CHECK(build_info.IncorrectlyBuilt().empty());
- auto const lib1_hpp_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeLocalArtifact("lib1.hpp", "repo"));
- auto const lib1_cpp_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeLocalArtifact("lib1.cpp", "repo"));
- auto const lib2_hpp_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeLocalArtifact("lib2.hpp", "repo"));
- auto const lib2_cpp_id = ArtifactFactory::Identifier(
- ArtifactFactory::DescribeLocalArtifact("lib2.cpp", "repo"));
+ auto const lib1_hpp_id =
+ ArtifactDescription::CreateLocal("lib1.hpp", "repo").Id();
+ auto const lib1_cpp_id =
+ ArtifactDescription::CreateLocal("lib1.cpp", "repo").Id();
+ auto const lib2_hpp_id =
+ ArtifactDescription::CreateLocal("lib2.hpp", "repo").Id();
+ auto const lib2_cpp_id =
+ ArtifactDescription::CreateLocal("lib2.cpp", "repo").Id();
CHECK_THAT(
build_info.ArtifactsUploaded(),
HasSameUniqueElementsAs<std::unordered_set<ArtifactIdentifier>>(