From 2b21b70635ef0d78b3af55adcf3c3018d491b840 Mon Sep 17 00:00:00 2001 From: jonawals Date: Thu, 29 Apr 2021 08:28:42 +0100 Subject: [PATCH 1/3] Add test selector and prioritizor. --- .../TestImpactModuleCoverageFactory.cpp | 13 +- .../Static/TestImpactDependencyGraphData.h | 26 ++ .../TestImpactTestSelectorAndPrioritizer.cpp | 224 ++++++++++++++++++ .../TestImpactTestSelectorAndPrioritizer.h | 77 ++++++ .../testimpactframework_runtime_files.cmake | 3 + 5 files changed, 333 insertions(+), 10 deletions(-) create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactModuleCoverageFactory.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactModuleCoverageFactory.cpp index 54ba65c762..889f91c600 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactModuleCoverageFactory.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactModuleCoverageFactory.cpp @@ -97,7 +97,7 @@ namespace TestImpact { // Module ModuleCoverage moduleCoverage; - moduleCoverage.m_path = AZ::IO::Path(package_node->first_attribute(Keys[NameKey])->value()); + moduleCoverage.m_path = package_node->first_attribute(Keys[NameKey])->value(); const auto classes_node = package_node->first_node(Keys[ClassesKey]); if (classes_node) @@ -107,13 +107,11 @@ namespace TestImpact { // Source SourceCoverage sourceCoverage; - sourceCoverage.m_path = AZ::IO::Path(pathRoot + class_node->first_attribute(Keys[FileNameKey])->value()); + sourceCoverage.m_path = pathRoot + class_node->first_attribute(Keys[FileNameKey])->value(); const auto lines_node = class_node->first_node(Keys[LinesKey]); if (lines_node) { - AZStd::vector lineCoverage; - // Lines for (auto line_node = lines_node->first_node(); line_node; line_node = line_node->next_sibling()) { @@ -121,12 +119,7 @@ namespace TestImpact const size_t number = AZStd::stol(AZStd::string(line_node->first_attribute(Keys[NumberKey])->value())); const size_t hits = AZStd::stol(AZStd::string(line_node->first_attribute(Keys[HitsKey])->value())); - lineCoverage.emplace_back(LineCoverage{number, hits}); - } - - if (!lineCoverage.empty()) - { - sourceCoverage.m_coverage.emplace(AZStd::move(lineCoverage)); + sourceCoverage.m_coverage.emplace_back(LineCoverage{number, hits}); } } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h new file mode 100644 index 0000000000..ee8ca722b4 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h @@ -0,0 +1,26 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#pragma once + +#include +#include + +namespace TestImpact +{ + struct DependencyGraphData + { + AZStd::string m_root; + AZStd::vector m_vertexes; + AZStd::vector> m_edges; + }; +} diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp new file mode 100644 index 0000000000..f8213738e5 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp @@ -0,0 +1,224 @@ +/* +* All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +* its licensors. +* +* For complete copyright and license terms please see the LICENSE at the root of this +* distribution (the "License"). All use of this software is governed by the License, +* or, if provided, by the license below or the license accompanying this file. Do not +* remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* +*/ + +#include +#include +#include +#include + +namespace TestImpact +{ + TestSelectorAndPrioritizer::TestSelectorAndPrioritizer(const DynamicDependencyMap* dynamicDependencyMap, DependencyGraphDataMap&& dependencyGraphDataMap) + : m_dynamicDependencyMap(dynamicDependencyMap) + , m_dependencyGraphDataMap(AZStd::move(dependencyGraphDataMap)) + { + } + + AZStd::vector TestSelectorAndPrioritizer::SelectTestTargets(const ChangeDependencyList& changeDependencyList, TestSelectionStrategy testSelectionStrategy) + { + const auto selectedTestTargetAndDependerMap = SelectTestTargets(changeDependencyList); + const auto prioritizedSelectedTests = PrioritizeSelectedTestTargets(selectedTestTargetAndDependerMap, testSelectionStrategy); + return prioritizedSelectedTests; + } + + TestSelectorAndPrioritizer::SelectedTestTargetAndDependerMap TestSelectorAndPrioritizer::SelectTestTargets( + const ChangeDependencyList& changeDependencyList) + { + SelectedTestTargetAndDependerMap selectedTestTargetMap; + + // Create operations + for (const auto& sourceDependency : changeDependencyList.GetCreateSourceDependencies()) + { + for (const auto& parentTarget : sourceDependency.GetParentTargets()) + { + AZStd::visit([&selectedTestTargetMap, this](auto&& target) + { + if constexpr (IsProductionTarget) + { + // Parent Targets: Yes + // Coverage Data : No + // Source Type : Production + // + // Scenario + // 1. The file has been newly created + // 2. This file exists in one or more source to production target mapping artifacts + // 3. There exists no coverage data for this file in the source covering test list + // + // Action + // 1. Select all test targets covering the parent production targets + const auto coverage = m_dynamicDependencyMap->GetCoveringTestTargetsForProductionTarget(*target); + for (const auto* testTarget : coverage) + { + selectedTestTargetMap[testTarget].insert(target); + } + } + else + { + // Parent Targets: Yes + // Coverage Data : No + // Source Type : Test + // + // Scenario + // 1. The file has been newly created + // 2. This file exists in one or more source to test target mapping artifacts + // 3. There exists no coverage data for this file in the source covering test list + // + // Action + // 1. Select all parent test targets + selectedTestTargetMap.insert(target); + } + }, parentTarget.GetTarget()); + } + } + + // Update operations + for (const auto& sourceDependency : changeDependencyList.GetUpdateSourceDependencies()) + { + if (sourceDependency.GetNumParentTargets()) + { + if (sourceDependency.GetNumCoveringTestTargets()) + { + for (const auto& parentTarget : sourceDependency.GetParentTargets()) + { + AZStd::visit([&selectedTestTargetMap, &sourceDependency, this](auto&& target) + { + if constexpr (IsProductionTarget) + { + // Parent Targets: Yes + // Coverage Data : Yes + // Source Type : Production + // + // Scenario + // 1. The existing file has been modified + // 2. This file exists in one or more source to production target mapping artifacts + // 3. There exists coverage data for this file in the source covering test list + // + // Action + // 1. Select all test targets covering this file + for (const auto* testTarget : sourceDependency.GetCoveringTestTargets()) + { + selectedTestTargetMap[testTarget].insert(target); + } + } + else + { + // Parent Targets: Yes + // Coverage Data : Yes + // Source Type : Test + // + // Scenario + // 1. The existing file has been modified + // 2. This file exists in one or more source to test target mapping artifacts + // 3. There exists coverage data for this file in the source covering test list + // + // Action + // 1. Select the parent test targets for this file + selectedTestTargetMap.insert(target); + } + }, parentTarget.GetTarget()); + } + } + else + { + for (const auto& parentTarget : sourceDependency.GetParentTargets()) + { + AZStd::visit([&selectedTestTargetMap, &sourceDependency, this](auto&& target) + { + if constexpr (IsTestTarget) + { + // Parent Targets: Yes + // Coverage Data : No + // Source Type : Test + // + // Scenario + // 1. The existing file has been modified + // 2. This file exists in one or more source to test target mapping artifacts + // 3. There exists no coverage data for this file in the source covering test list + // + // Action + // 1. Select the parent test targets for this file + selectedTestTargetMap.insert(target); + } + }, parentTarget.GetTarget()); + } + } + } + else + { + // Parent Targets: No + // Coverage Data : Yes + // Source Type : Indeterminate + // + // Scenario + // 1. The existing file has been modified + // 2. Either: + // a) This file previously existed in one or more source to target mapping artifacts + // b) This file no longer exists in any source to target mapping artifacts + // c) The coverage data for this file was has yet to be deleted from the source covering test list + // 3. Or: + // a) The file is being used by build targets but has erroneously not been explicitly added to the build + // system (e.g. include directive pulling in a header from the repository that has not been added to + // any build targets due to an oversight) + // + // Action + // 1. Log potential orphaned source file warning + // 2. Select all test targets covering this file + // 3. Delete the existing coverage data from the source covering test list + + for (const auto* testTarget : sourceDependency.GetCoveringTestTargets()) + { + selectedTestTargetMap.insert(testTarget); + } + } + } + + // Delete operations + for (const auto& sourceDependency : changeDependencyList.GetDeleteSourceDependencies()) + { + // Parent Targets: No + // Coverage Data : Yes + // Source Type : Indeterminate + // + // Scenario + // 1. The existing file has been deleted + // 2. This file previously existed in one or more source to target mapping artifacts + // 2. This file does not exist in any source to target mapping artifacts + // 4. The coverage data for this file was has yet to be deleted from the source covering test list + // + // Action + // 1. Select all test targets covering this file + // 2. Delete the existing coverage data from the source covering test list + for (const auto* testTarget : sourceDependency.GetCoveringTestTargets()) + { + selectedTestTargetMap.insert(testTarget); + } + } + + return selectedTestTargetMap; + } + + AZStd::vector TestSelectorAndPrioritizer::PrioritizeSelectedTestTargets( + const SelectedTestTargetAndDependerMap& selectedTestTargetAndDependerMap, + [[maybe_unused]]TestSelectionStrategy testSelectionStrategy) + { + AZStd::vector selectedTestTargets; + + // Prioritization disabled for now + // https://jira.agscollab.com/browse/SPEC-6563 + for (const auto& [testTarget, dependerTargets] : selectedTestTargetAndDependerMap) + { + selectedTestTargets.push_back(testTarget); + } + + return selectedTestTargets; + } +} diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h new file mode 100644 index 0000000000..261e469ee2 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h @@ -0,0 +1,77 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#pragma once + +#include +#include + +#include +#include +#include + +namespace TestImpact +{ + class DynamicDependencyMap; + class BuildTarget; + class TestTarget; + + //! Strategy for selecting tests given a set of source changes. + enum class TestSelectionStrategy : bool + { + SelectOnly, //!< Select tests only, do not attempt prioritization of those selected tests. + SelectAndPriotitize //!< Select tests and prioritize according to dependency graph locality of coverer and coveree. + }; + + //! Map of build targets and their dependency graph data. + //! For test targets, the dependency graph data is that of the build targets which the test target depends on. + //! For production targets, the dependency graph is that of the build targets that depend on it (dependers). + //! @note No dependency graph data is not an error, it simple means that the target cannot be prioritized. + using DependencyGraphDataMap = AZStd::unordered_map; + + //! Selects the test targets that cover a given set of changes based on the CRUD rules and optionally prioritizes the test + //! selection according to their locality of their covering production targets in the their dependency graphs. + //! @note the CRUD rules for how tests are selected can be found in the MicroRepo header file. + class TestSelectorAndPrioritizer + { + public: + //! Constructs the test selector and prioritizer for the given dynamic dependency map. + //! @param dynamicDependencyMap The dynamic dependency map representing the repository source tree. + //! @param dependencyGraphDataMap The map of build targets and their dependency graph data for use in test prioritization. + TestSelectorAndPrioritizer(const DynamicDependencyMap* dynamicDependencyMap, DependencyGraphDataMap&& dependencyGraphDataMap); + + //! Select the covering test targets for the given set of source changes and optionally prioritizes said test selection. + //! @param changeDependencyList The resolved list of source dependencies for the CRUD source changes. + //! @param testSelectionStrategy The test selection and prioritization strategy to apply to the given CRUD source changes. + AZStd::vector SelectTestTargets(const ChangeDependencyList& changeDependencyList, TestSelectionStrategy testSelectionStrategy); + + private: + //! Map of selected test targets and the production targets they cover for the given set of source changes. + using SelectedTestTargetAndDependerMap = AZStd::unordered_map>; + + //! Selects the test targets covering the set of source changes in the change dependency list. + //! @param changeDependencyList The change dependency list containing the CRUD source changes to select tests for. + //! @returns The selected tests and their covering production targets for the given set of source changes. + SelectedTestTargetAndDependerMap SelectTestTargets(const ChangeDependencyList& changeDependencyList); + + //! Prioritizes the selected tests according to the specified test selection strategy, + //! @note If no dependency graph data exists for a given test target then that test target still be selected albeit not prioritized. + //! @param selectedTestTargetAndDependerMap The selected tests to prioritize. + //! @param testSelectionStrategy The test selection strategy to prioritize the selected tests. + //! @returns The selected tests either in either arbitrary order or in prioritized with highest priority first. + AZStd::vector PrioritizeSelectedTestTargets( + const SelectedTestTargetAndDependerMap& selectedTestTargetAndDependerMap, TestSelectionStrategy testSelectionStrategy); + + const DynamicDependencyMap* m_dynamicDependencyMap; + DependencyGraphDataMap m_dependencyGraphDataMap; + }; +} diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake index 19f3ec23f7..da3693433e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake +++ b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake @@ -36,6 +36,7 @@ set(FILES Source/Artifact/Static/TestImpactTestTargetMeta.h Source/Artifact/Static/TestImpactTestTargetDescriptor.cpp Source/Artifact/Static/TestImpactTestTargetDescriptor.h + Source/Artifact/Static/TestImpactDependencyGraphData.h Source/Artifact/Dynamic/TestImpactChangelist.h Source/Artifact/Dynamic/TestImpactTestEnumerationSuite.h Source/Artifact/Dynamic/TestImpactTestRunSuite.h @@ -59,6 +60,8 @@ set(FILES Source/Dependency/TestImpactDependencyException.h Source/Dependency/TestImpactSourceDependency.h Source/Dependency/TestImpactSourceDependency.cpp + Source/Dependency/TestImpactTestSelectorAndPrioritizer.h + Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp Source/Dependency/TestImpactSourceCoveringTestsList.h Source/Dependency/TestImpactSourceCoveringTestsList.cpp Source/Target/TestImpactBuildTarget.cpp From eb5dd7ee47ee3ce0d6c4ab3fdc86fd923fff503e Mon Sep 17 00:00:00 2001 From: jonawals Date: Thu, 29 Apr 2021 14:51:51 +0100 Subject: [PATCH 2/3] Address PR comments. --- .../Source/Artifact/Static/TestImpactDependencyGraphData.h | 2 +- .../Dependency/TestImpactTestSelectorAndPrioritizer.cpp | 4 ++-- .../Source/Dependency/TestImpactTestSelectorAndPrioritizer.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h index ee8ca722b4..8dae41c579 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h @@ -23,4 +23,4 @@ namespace TestImpact AZStd::vector m_vertexes; AZStd::vector> m_edges; }; -} +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp index f8213738e5..2962e91381 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp @@ -213,7 +213,7 @@ namespace TestImpact AZStd::vector selectedTestTargets; // Prioritization disabled for now - // https://jira.agscollab.com/browse/SPEC-6563 + // SPEC-6563 for (const auto& [testTarget, dependerTargets] : selectedTestTargetAndDependerMap) { selectedTestTargets.push_back(testTarget); @@ -221,4 +221,4 @@ namespace TestImpact return selectedTestTargets; } -} +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h index 261e469ee2..c742176073 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.h @@ -74,4 +74,4 @@ namespace TestImpact const DynamicDependencyMap* m_dynamicDependencyMap; DependencyGraphDataMap m_dependencyGraphDataMap; }; -} +} // namespace TestImpact From 20243549e6a36bd59e5b12f829b691cf88eca8ce Mon Sep 17 00:00:00 2001 From: jonawals Date: Fri, 30 Apr 2021 12:51:26 +0100 Subject: [PATCH 3/3] Address PR comments. --- .../Source/Artifact/Static/TestImpactDependencyGraphData.h | 7 ++++--- .../Dependency/TestImpactTestSelectorAndPrioritizer.cpp | 6 ++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h index 8dae41c579..3c9f455254 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Static/TestImpactDependencyGraphData.h @@ -17,10 +17,11 @@ namespace TestImpact { + //! Raw representation of the dependency graph for a given build target. struct DependencyGraphData { - AZStd::string m_root; - AZStd::vector m_vertexes; - AZStd::vector> m_edges; + AZStd::string m_root; //!< The build target this dependency graph is for. + AZStd::vector m_vertices; //!< The depender/depending built targets in this graph. + AZStd::vector> m_edges; //!< The dependency connectivity of the build targets in this graph. }; } // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp index 2962e91381..549ef0c108 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp @@ -17,13 +17,15 @@ namespace TestImpact { - TestSelectorAndPrioritizer::TestSelectorAndPrioritizer(const DynamicDependencyMap* dynamicDependencyMap, DependencyGraphDataMap&& dependencyGraphDataMap) + TestSelectorAndPrioritizer::TestSelectorAndPrioritizer( + const DynamicDependencyMap* dynamicDependencyMap, DependencyGraphDataMap&& dependencyGraphDataMap) : m_dynamicDependencyMap(dynamicDependencyMap) , m_dependencyGraphDataMap(AZStd::move(dependencyGraphDataMap)) { } - AZStd::vector TestSelectorAndPrioritizer::SelectTestTargets(const ChangeDependencyList& changeDependencyList, TestSelectionStrategy testSelectionStrategy) + AZStd::vector TestSelectorAndPrioritizer::SelectTestTargets( + const ChangeDependencyList& changeDependencyList, TestSelectionStrategy testSelectionStrategy) { const auto selectedTestTargetAndDependerMap = SelectTestTargets(changeDependencyList); const auto prioritizedSelectedTests = PrioritizeSelectedTestTargets(selectedTestTargetAndDependerMap, testSelectionStrategy);