From 49c2382ea5ea45966ef15140fce6ad89672e956c Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Thu, 12 Sep 2024 17:45:14 +0200 Subject: execution_response: Allow failures to be reported while populating As populating the containers from remote response only takes place once, no assumptions should be made that this cannot fail (for example if wrong or invalid entries were produced). Instead, return error messages on failure to callers that can log accordingly. --- test/buildtool/execution_api/common/api_test.hpp | 174 +++++++++++++---------- 1 file changed, 96 insertions(+), 78 deletions(-) (limited to 'test/buildtool/execution_api/common/api_test.hpp') diff --git a/test/buildtool/execution_api/common/api_test.hpp b/test/buildtool/execution_api/common/api_test.hpp index 4b84d66a..d00eec7b 100644 --- a/test/buildtool/execution_api/common/api_test.hpp +++ b/test/buildtool/execution_api/common/api_test.hpp @@ -164,26 +164,28 @@ using ExecProps = std::map; action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); if (is_hermetic) { CHECK(not response->IsCached()); SECTION("Rerun execution to verify caching") { // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(response->IsCached()); } } @@ -193,24 +195,25 @@ using ExecProps = std::map; action->SetCacheFlag(IExecutionAction::CacheFlag::DoNotCacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); SECTION("Rerun execution to verify caching") { // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); } } @@ -254,26 +257,28 @@ using ExecProps = std::map; action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); if (is_hermetic) { CHECK(not response->IsCached()); SECTION("Rerun execution to verify caching") { // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(response->IsCached()); } } @@ -283,24 +288,26 @@ using ExecProps = std::map; action->SetCacheFlag(IExecutionAction::CacheFlag::DoNotCacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); SECTION("Rerun execution to verify caching") { // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); } } @@ -335,27 +342,29 @@ using ExecProps = std::map; action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result CHECK(response->ExitCode() == 1); - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); SECTION("Rerun execution to verify that non-zero actions are rerun") { // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result CHECK(response->ExitCode() == 1); - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); } } @@ -364,14 +373,15 @@ using ExecProps = std::map; action->SetCacheFlag(IExecutionAction::CacheFlag::DoNotCacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result CHECK(response->ExitCode() == 1); - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); SECTION("Rerun execution to verify non-zero actions are not cached") { @@ -381,9 +391,10 @@ using ExecProps = std::map; // verify result CHECK(response->ExitCode() == 1); - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(artifacts.at(output_path).digest == test_digest); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(artifacts.value()->at(output_path).digest == test_digest); CHECK(not response->IsCached()); } } @@ -427,7 +438,7 @@ using ExecProps = std::map; action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result @@ -437,10 +448,11 @@ using ExecProps = std::map; CHECK_FALSE(response->IsCached()); } - auto artifacts = response->Artifacts(); - REQUIRE_FALSE(artifacts.empty()); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE_FALSE(artifacts.value()->empty()); - auto info = artifacts.begin()->second; + auto info = artifacts.value()->begin()->second; SECTION("retrieve via same API object") { auto out_path = GetTestDir(test_name) / "out1"; @@ -500,7 +512,7 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result @@ -510,10 +522,11 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, CHECK_FALSE(response->IsCached()); } - auto artifacts = response->Artifacts(); - REQUIRE_FALSE(artifacts.empty()); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE_FALSE(artifacts.value()->empty()); - auto info = artifacts.begin()->second; + auto info = artifacts.value()->begin()->second; SECTION("retrieve via same API object") { auto out_path = GetTestDir(test_name) / "out1"; @@ -569,7 +582,7 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result @@ -579,16 +592,17 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, CHECK_FALSE(response->IsCached()); } - auto artifacts = response->Artifacts(); - REQUIRE_FALSE(artifacts.empty()); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE_FALSE(artifacts.value()->empty()); std::vector paths{}; std::vector infos{}; SECTION("retrieve via same API object") { auto out_path = GetTestDir(test_name) / "out1"; - std::for_each(artifacts.begin(), - artifacts.end(), + std::for_each(artifacts.value()->begin(), + artifacts.value()->end(), [&out_path, &paths, &infos](auto const& entry) { paths.emplace_back(out_path / entry.first); infos.emplace_back(entry.second); @@ -602,8 +616,8 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, SECTION("retrieve from new API object but same endpoint") { auto second_api = api_factory(); auto out_path = GetTestDir(test_name) / "out2"; - std::for_each(artifacts.begin(), - artifacts.end(), + std::for_each(artifacts.value()->begin(), + artifacts.value()->end(), [&out_path, &paths, &infos](auto const& entry) { paths.emplace_back(out_path / entry.first); infos.emplace_back(entry.second); @@ -638,26 +652,28 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(IsTreeObject(artifacts.at(output_path).type)); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(IsTreeObject(artifacts.value()->at(output_path).type)); if (is_hermetic) { CHECK(not response->IsCached()); SECTION("Rerun execution to verify caching") { // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(IsTreeObject(artifacts.at(output_path).type)); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(IsTreeObject(artifacts.value()->at(output_path).type)); CHECK(response->IsCached()); } } @@ -667,24 +683,26 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, action->SetCacheFlag(IExecutionAction::CacheFlag::DoNotCacheOutput); // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(IsTreeObject(artifacts.at(output_path).type)); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(IsTreeObject(artifacts.value()->at(output_path).type)); CHECK(not response->IsCached()); SECTION("Rerun execution to verify caching") { // run execution - auto response = action->Execute(); + auto const response = action->Execute(); REQUIRE(response); // verify result - auto artifacts = response->Artifacts(); - REQUIRE(artifacts.contains(output_path)); - CHECK(IsTreeObject(artifacts.at(output_path).type)); + auto const artifacts = response->Artifacts(); + REQUIRE(artifacts.has_value()); + REQUIRE(artifacts.value()->contains(output_path)); + CHECK(IsTreeObject(artifacts.value()->at(output_path).type)); CHECK(not response->IsCached()); } } -- cgit v1.2.3