From 06c65be40acb231d60b5febbe0bdee6e44511e14 Mon Sep 17 00:00:00 2001 From: jonawals Date: Thu, 8 Jul 2021 21:59:05 +0100 Subject: [PATCH] Add handling for integrity failures when applying and resolving change lists (#1993) * ApplyAndResoveChangeList integration failure policy Signed-off-by: John * Default integrity failure policy for TIAF sequences Signed-off-by: John * Fix superfluous quotation marks Signed-off-by: John * Remove superfluous args parse Signed-off-by: John * Fix broken storage of last commit hash Signed-off-by: John --- .../TestImpactDynamicDependencyMap.cpp | 33 +++++++++++++------ .../TestImpactDynamicDependencyMap.h | 5 ++- .../Runtime/Code/Source/TestImpactRuntime.cpp | 2 +- scripts/build/TestImpactAnalysis/tiaf.py | 24 ++++++++------ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp index 9959ccf5cd..28f8d9bb4e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp @@ -346,7 +346,8 @@ namespace TestImpact return orphans; } - ChangeDependencyList DynamicDependencyMap::ApplyAndResoveChangeList(const ChangeList& changeList) + ChangeDependencyList DynamicDependencyMap::ApplyAndResoveChangeList( + const ChangeList& changeList, Policy::IntegrityFailure integrityFailurePolicy) { AZStd::vector createDependencies; AZStd::vector updateDependencies; @@ -364,11 +365,15 @@ namespace TestImpact { if (sourceDependency->GetNumCoveringTestTargets()) { - const AZStd::string msg = AZStd::string::format("The newly-created file %s belongs to a build target yet " + const AZStd::string msg = AZStd::string::format("The newly-created file '%s' belongs to a build target yet " "still has coverage data in the source covering test list implying that a delete CRUD operation has been " - "missed, thus the integrity of the source covering test list has been compromised", createdFile.c_str()); + "missed, thus the integrity of the source covering test list has been compromised.", createdFile.c_str()); AZ_Error("File Creation", false, msg.c_str()); - throw DependencyException(msg); + + if (integrityFailurePolicy == Policy::IntegrityFailure::Abort) + { + throw DependencyException(msg); + } } if (sourceDependency->GetNumParentTargets()) @@ -418,18 +423,26 @@ namespace TestImpact { if (sourceDependency->GetNumCoveringTestTargets()) { - const AZStd::string msg = AZStd::string::format("The deleted file %s still belongs to a build target and still " + const AZStd::string msg = AZStd::string::format("The deleted file '%s' still belongs to a build target and still " "has coverage data in the source covering test list, implying that the integrity of both the source to target " - "mappings and the source covering test list has been compromised", deletedFile.c_str()); + "mappings and the source covering test list has been compromised.", deletedFile.c_str()); AZ_Error("File Delete", false, msg.c_str()); - throw DependencyException(msg); + + if (integrityFailurePolicy == Policy::IntegrityFailure::Abort) + { + throw DependencyException(msg); + } } else { - const AZStd::string msg = AZStd::string::format("The deleted file %s still belongs to a build target implying " - "that the integrity of the source to target mappings has been compromised", deletedFile.c_str()); + const AZStd::string msg = AZStd::string::format("The deleted file '%s' still belongs to a build target implying " + "that the integrity of the source to target mappings has been compromised.", deletedFile.c_str()); AZ_Error("File Delete", false, msg.c_str()); - throw DependencyException(msg); + + if (integrityFailurePolicy == Policy::IntegrityFailure::Abort) + { + throw DependencyException(msg); + } } } else diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h index 35f90930f8..d021851c25 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include @@ -91,8 +92,10 @@ namespace TestImpact //! Applies the specified change list to the dependency map and resolves the change list to a change dependency list //! containing the updated source dependencies for each source file in the change list. //! @param changeList The change list to apply and resolve. + //! @param integrityFailurePolicy The policy to use for handling integrity errors in the source dependency map. //! @returns The change list as resolved to the appropriate source dependencies. - [[nodiscard]] ChangeDependencyList ApplyAndResoveChangeList(const ChangeList& changeList); + [[nodiscard]] ChangeDependencyList ApplyAndResoveChangeList( + const ChangeList& changeList, Policy::IntegrityFailure integrityFailurePolicy); //! Removes the specified test target from all source coverage. void RemoveTestTargetFromSourceCoverage(const TestTarget* testTarget); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp index af6f02fa20..e325463324 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -205,7 +205,7 @@ namespace TestImpact AZStd::vector discardedTestTargets; // Select and prioritize the test targets pertinent to this change list - const auto changeDependencyList = m_dynamicDependencyMap->ApplyAndResoveChangeList(changeList); + const auto changeDependencyList = m_dynamicDependencyMap->ApplyAndResoveChangeList(changeList, m_integrationFailurePolicy); const auto selectedTestTargets = m_testSelectorAndPrioritizer->SelectTestTargets(changeDependencyList, testPrioritizationPolicy); // Populate a set with the selected test targets so that we can infer the discarded test target not selected for this change list diff --git a/scripts/build/TestImpactAnalysis/tiaf.py b/scripts/build/TestImpactAnalysis/tiaf.py index 1ba0a3f195..5ae76d8269 100644 --- a/scripts/build/TestImpactAnalysis/tiaf.py +++ b/scripts/build/TestImpactAnalysis/tiaf.py @@ -49,12 +49,13 @@ class TestImpact: # Config self.__parse_config_file(config_file) # Sequence - if self.__use_test_impact_analysis: - if self.__is_seeding_branch and self.__is_seeding_pipeline: - self.__is_seeding = True - else: - self.__is_seeding = False - self.__generate_change_list() + if self.__is_seeding_branch and self.__is_seeding_pipeline: + self.__is_seeding = True + else: + self.__is_seeding = False + print(f"Is seeding: '{self.__is_seeding}'.") + if self.__use_test_impact_analysis and not self.__is_seeding: + self.__generate_change_list() # Parse the configuration file and retrieve the data needed for launching the test impact analysis runtime def __parse_config_file(self, config_file): @@ -192,6 +193,9 @@ class TestImpact: # Sequence type args.append("--sequence=tianowrite") print("Sequence type is set to 'tianowrite'.") + # Integrity failure policy + args.append("--ipolicy=continue") + print("Integration failure policy is set to 'continue'.") # Safe mode if safe_mode: args.append("--safemode=on") @@ -210,12 +214,12 @@ class TestImpact: # Sequence type args.append("--sequence=regular") print("Sequence type is set to 'regular'.") - # Pipeline of truth sequence - if self.__is_pipeline_of_truth: + # Seeding job + if self.__is_seeding: # Test failure policy args.append(f"--fpolicy={seed_sequence_test_failure_policy}") print(f"Test failure policy is set to '{seed_sequence_test_failure_policy}'.") - # Non pipeline of truth sequence + # Non seeding job else: # Test failure policy args.append(f"--fpolicy={test_failure_policy}") @@ -227,7 +231,7 @@ class TestImpact: # If the sequence completed (with or without failures) we will update the historical meta-data if result.returncode == 0 or result.returncode == 7: print("Test impact analysis runtime returned successfully.") - if self.__is_pipeline_of_truth: + if self.__is_seeding: print("Writing historical meta-data...") self.__write_last_run_hash(self.__dst_commit) print("Complete!")