From b9dc7639b4182cbdace284ed4226c0c179ffec52 Mon Sep 17 00:00:00 2001 From: jonawals Date: Wed, 28 Apr 2021 17:15:48 +0100 Subject: [PATCH 1/6] Add dynamic dependency map. --- .../Frontend/Console/Code/CMakeLists.txt | 2 +- .../Artifact/Dynamic/TestImpactCoverage.h | 9 +- .../TestImpactChangeDependencyList.cpp | 41 ++ .../TestImpactChangeDependencyList.h | 41 ++ .../TestImpactDependencyException.h | 26 ++ .../TestImpactDynamicDependencyMap.cpp | 414 ++++++++++++++++++ .../TestImpactDynamicDependencyMap.h | 120 +++++ .../TestImpactSourceCoveringTestsList.cpp | 58 +++ .../TestImpactSourceCoveringTestsList.h | 53 +++ .../Test/Run/TestImpactTestCoverage.cpp | 4 +- .../Source/Test/Run/TestImpactTestCoverage.h | 4 +- .../Tests/Test/TestImpactTestCoverageTest.cpp | 12 +- .../testimpactframework_runtime_files.cmake | 68 ++- ...timpactframework_runtime_tests_files.cmake | 24 +- 14 files changed, 853 insertions(+), 23 deletions(-) create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.cpp create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.h create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDependencyException.h create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Code/CMakeLists.txt b/Code/Tools/TestImpactFramework/Frontend/Console/Code/CMakeLists.txt index 7a043a30ca..da2c707cb8 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Code/CMakeLists.txt +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Code/CMakeLists.txt @@ -20,4 +20,4 @@ ly_add_target( BUILD_DEPENDENCIES PRIVATE AZ::TestImpact.Runtime.Static -) +) \ No newline at end of file diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Dynamic/TestImpactCoverage.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Dynamic/TestImpactCoverage.h index 3c5b5353a7..54b78d682e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Dynamic/TestImpactCoverage.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Dynamic/TestImpactCoverage.h @@ -12,9 +12,8 @@ #pragma once -#include +#include #include -#include namespace TestImpact { @@ -28,14 +27,14 @@ namespace TestImpact //! Coverage information about a particular source file. struct SourceCoverage { - AZ::IO::Path m_path; //!< Source file path. - AZStd::optional> m_coverage; //!< Source file line coverage (empty if source level coverage only). + AZStd::string m_path; //!< Source file path. + AZStd::vector m_coverage; //!< Source file line coverage (empty if source level coverage only). }; //! Coverage information about a particular module (executable, shared library). struct ModuleCoverage { - AZ::IO::Path m_path; //!< Module path. + AZStd::string m_path; //!< Module path. AZStd::vector m_sources; //!< Sources of this module that are covered. }; } // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.cpp new file mode 100644 index 0000000000..53e71d3a30 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.cpp @@ -0,0 +1,41 @@ +/* + * 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 + +namespace TestImpact +{ + ChangeDependencyList::ChangeDependencyList( + AZStd::vector&& createSourceDependencies, + AZStd::vector&& updateSourceDependencies, + AZStd::vector&& deleteSourceDependencies) + : m_createSourceDependencies(AZStd::move(createSourceDependencies)) + , m_updateSourceDependencies(AZStd::move(updateSourceDependencies)) + , m_deleteSourceDependencies(AZStd::move(deleteSourceDependencies)) + { + } + + const AZStd::vector& ChangeDependencyList::GetCreateSourceDependencies() const + { + return m_createSourceDependencies; + } + + const AZStd::vector& ChangeDependencyList::GetUpdateSourceDependencies() const + { + return m_updateSourceDependencies; + } + + const AZStd::vector& ChangeDependencyList::GetDeleteSourceDependencies() const + { + return m_deleteSourceDependencies; + } +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.h new file mode 100644 index 0000000000..bf8b0f558d --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactChangeDependencyList.h @@ -0,0 +1,41 @@ +/* + * 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 + +namespace TestImpact +{ + //! Representation of a change list where all CRUD sources have been resolved to source dependencies from the dynamic dependency map. + class ChangeDependencyList + { + public: + ChangeDependencyList( + AZStd::vector&& createSourceDependencies, + AZStd::vector&& updateSourceDependencies, + AZStd::vector&& deleteSourceDependencies); + + //! Gets the sources dependencies of the created source files from the change list. + const AZStd::vector& GetCreateSourceDependencies() const; + + //! Gets the sources dependencies of the updated source files from the change list. + const AZStd::vector& GetUpdateSourceDependencies() const; + + //! Gets the sources dependencies of the deleted source files from the change list. + const AZStd::vector& GetDeleteSourceDependencies() const; + private: + AZStd::vector m_createSourceDependencies; + AZStd::vector m_updateSourceDependencies; + AZStd::vector m_deleteSourceDependencies; + }; +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDependencyException.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDependencyException.h new file mode 100644 index 0000000000..e256fa3ccc --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDependencyException.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 + +namespace TestImpact +{ + //! Exception for dependency related operations. + class DependencyException + : public Exception + { + public: + using Exception::Exception; + }; +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp new file mode 100644 index 0000000000..f3edbe2507 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp @@ -0,0 +1,414 @@ +/* + * 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 + +namespace TestImpact +{ + DynamicDependencyMap::DynamicDependencyMap( + AZStd::vector&& productionTargetDescriptors, + AZStd::vector&& testTargetDescriptors) + : m_productionTargets(AZStd::move(productionTargetDescriptors)) + , m_testTargets(AZStd::move(testTargetDescriptors)) + { + const auto mapBuildTargetSources = [this](const auto* target) + { + for (const auto& source : target->GetSources().m_staticSources) + { + if (auto mapping = m_sourceDependencyMap.find(source); mapping != m_sourceDependencyMap.end()) + { + // This is an existing entry in the dependency map so update the parent build targets with this target + mapping->second.m_parentTargets.insert(target); + } + else + { + // This is a new entry on the dependency map so create an entry with this parent target and no covering targets + m_sourceDependencyMap.emplace(source, DependencyData{ {target}, {} }); + } + } + + // Populate the autogen input to output mapping with any autogen sources + for (const auto& autogen : target->GetSources().m_autogenSources) + { + for (const auto& output : autogen.m_outputs) + { + m_autogenInputToOutputMap[autogen.m_input].push_back(output); + } + } + }; + + for (const auto& target : m_productionTargets.GetTargets()) + { + mapBuildTargetSources(&target); + } + + for (const auto& target : m_testTargets.GetTargets()) + { + mapBuildTargetSources(&target); + } + } + + size_t DynamicDependencyMap::GetNumTargets() const + { + return m_productionTargets.GetNumTargets() + m_testTargets.GetNumTargets(); + } + + size_t DynamicDependencyMap::GetNumSources() const + { + return m_sourceDependencyMap.size(); + } + + const BuildTarget* DynamicDependencyMap::GetBuildTarget(const AZStd::string& name) const + { + const BuildTarget* buildTarget = nullptr; + AZStd::visit([&buildTarget](auto&& target) + { + if constexpr (IsProductionTarget || IsTestTarget) + { + buildTarget = target; + } + + }, GetTarget(name)); + + return buildTarget; + } + + const BuildTarget* DynamicDependencyMap::GetBuildTargetOrThrow(const AZStd::string& name) const + { + const BuildTarget* buildTarget = nullptr; + AZStd::visit([&buildTarget](auto&& target) + { + if constexpr (IsProductionTarget || IsTestTarget) + { + buildTarget = target; + } + }, GetTargetOrThrow(name)); + + return buildTarget; + } + + AZStd::variant DynamicDependencyMap::GetTarget(const AZStd::string& name) const + { + if (auto testTarget = m_testTargets.GetTarget(name); testTarget != nullptr) + { + return testTarget; + } + else if (auto productionTarget = m_productionTargets.GetTarget(name); productionTarget != nullptr) + { + return productionTarget; + } + + return AZStd::monostate{}; + } + + AZStd::variant DynamicDependencyMap::GetTargetOrThrow(const AZStd::string& name) const + { + AZStd::variant buildTarget; + AZStd::visit([&buildTarget, &name](auto&& target) + { + if constexpr (IsProductionTarget || IsTestTarget) + { + buildTarget = target; + } + else + { + throw(TargetException(AZStd::string::format("Couldn't find target %s", name.c_str()).c_str())); + } + }, GetTarget(name)); + + return buildTarget; + } + + void DynamicDependencyMap::ReplaceSourceCoverage(const SourceCoveringTestsList& sourceCoverageDelta) + { + for (const auto& sourceCoverage : sourceCoverageDelta.GetCoverage()) + { + // Autogen input files are not compiled sources and thus supplying coverage data for them makes no sense + AZ_TestImpact_Eval( + m_autogenInputToOutputMap.find(sourceCoverage.GetPath()) == m_autogenInputToOutputMap.end(), + DependencyException, AZStd::string::format("Couldn't replace source coverage for %s, source file is an autogen input file", + sourceCoverage.GetPath().c_str()).c_str()); + + auto [it, inserted] = m_sourceDependencyMap.insert(sourceCoverage.GetPath()); + auto& [key, sourceDependency] = *it; + + // Clear any existing coverage for the delta + sourceDependency.m_coveringTestTargets.clear(); + + // Update the dependency with any new coverage data + for (const auto& unresolvedTestTarget : sourceCoverage.GetCoveringTestTargets()) + { + const TestTarget* testTarget = m_testTargets.GetTarget(unresolvedTestTarget); + if (testTarget) + { + // Source to covering test target mapping + sourceDependency.m_coveringTestTargets.insert(testTarget); + + // Build target to covering test target mapping + for (const auto& parentTarget : sourceDependency.m_parentTargets) + { + m_buildTargetCoverage[parentTarget.GetBuildTarget()].insert(testTarget); + } + } + else + { + AZ_Warning("ReplaceSourceCoverage", false, AZStd::string::format("Test target %s exists in the coverage data " + "but has since been removed from the build system", unresolvedTestTarget.c_str()).c_str()); + } + } + + // If the new coverage data results in a parentless and coverageless entry, consider it a dead entry and remove accordingly + if (sourceDependency.m_coveringTestTargets.empty() && sourceDependency.m_parentTargets.empty()) + { + m_sourceDependencyMap.erase(it); + } + } + } + + void DynamicDependencyMap::ClearSourceCoverage(const AZStd::vector& paths) + { + for (const auto& path : paths) + { + if (const auto outputSources = m_autogenInputToOutputMap.find(path); outputSources != m_autogenInputToOutputMap.end()) + { + // Clearing the coverage data of an autogen input source instead clears the coverage data of its output sources + for (const auto& outputSource : outputSources->second) + { + ReplaceSourceCoverage(SourceCoveringTestsList({ SourceCoveringTests(outputSource, { }) })); + } + } + else + { + ReplaceSourceCoverage(SourceCoveringTestsList({ SourceCoveringTests(path, { }) })); + } + } + } + + const ProductionTargetList& DynamicDependencyMap::GetProductionTargetList() const + { + return m_productionTargets; + } + + const TestTargetList& DynamicDependencyMap::GetTestTargetList() const + { + return m_testTargets; + } + + AZStd::vector DynamicDependencyMap::GetCoveringTestTargetsForProductionTarget(const ProductionTarget& productionTarget) const + { + AZStd::vector coveringTestTargets; + if (const auto coverage = m_buildTargetCoverage.find(&productionTarget); coverage != m_buildTargetCoverage.end()) + { + coveringTestTargets.reserve(coverage->second.size()); + AZStd::copy(coverage->second.begin(), coverage->second.end(), AZStd::back_inserter(coveringTestTargets)); + } + + return coveringTestTargets; + } + + AZStd::optional DynamicDependencyMap::GetSourceDependency(const AZStd::string& path) const + { + AZStd::unordered_set parentTargets; + AZStd::unordered_set coveringTestTargets; + + const auto getSourceDependency = [&parentTargets, &coveringTestTargets, this](const AZStd::string& path) + { + const auto sourceDependency = m_sourceDependencyMap.find(path); + if (sourceDependency != m_sourceDependencyMap.end()) + { + for (const auto& parentTarget : sourceDependency->second.m_parentTargets) + { + parentTargets.insert(parentTarget); + } + + for (const auto& testTarget : sourceDependency->second.m_coveringTestTargets) + { + coveringTestTargets.insert(testTarget); + } + } + }; + + if (const auto outputSources = m_autogenInputToOutputMap.find(path); outputSources != m_autogenInputToOutputMap.end()) + { + // Consolidate the parentage and coverage of each of the autogen input file's generated output files + for (const auto& outputSource : outputSources->second) + { + getSourceDependency(outputSource); + } + } + else + { + getSourceDependency(path); + } + + if (!parentTargets.empty() || !coveringTestTargets.empty()) + { + return SourceDependency(path, DependencyData{ AZStd::move(parentTargets), AZStd::move(coveringTestTargets) }); + } + + return AZStd::nullopt; + } + + SourceDependency DynamicDependencyMap::GetSourceDependencyOrThrow(const AZStd::string& path) const + { + auto sourceDependency = GetSourceDependency(path); + AZ_TestImpact_Eval(sourceDependency.has_value(), DependencyException, AZStd::string::format("Couldn't find source %s", path.c_str()).c_str()); + return sourceDependency.value(); + } + + SourceCoveringTestsList DynamicDependencyMap::ExportSourceCoverage() const + { + AZStd::vector coverage; + for (const auto& [path, dependency] : m_sourceDependencyMap) + { + AZStd::vector souceCoveringTests; + for (const auto& testTarget : dependency.m_coveringTestTargets) + { + souceCoveringTests.push_back(testTarget->GetName()); + } + + coverage.push_back(SourceCoveringTests(path, AZStd::move(souceCoveringTests))); + } + + return coverage; + } + + AZStd::vector DynamicDependencyMap::GetOrphanSourceFiles() const + { + AZStd::vector orphans; + for (const auto& [source, dependency] : m_sourceDependencyMap) + { + if (dependency.m_parentTargets.empty()) + { + orphans.push_back(source); + } + } + + return orphans; + } + + ChangeDependencyList DynamicDependencyMap::ApplyAndResoveChangeList(const ChangeList& changeList) + { + AZStd::vector createDependencies; + AZStd::vector updateDependencies; + AZStd::vector deleteDependencies; + + // Keep track of the coverage to delete as a post step rather than deleting it in situ so that erroneous change lists + // do not corrupt the dynamic dependency map + AZStd::vector coverageToDelete; + + // Create operations + for (const auto& createdFile : changeList.m_createdFiles) + { + auto sourceDependency = GetSourceDependency(createdFile); + if (sourceDependency.has_value()) + { + if (sourceDependency->GetNumParentTargets()) + { + if (sourceDependency->GetNumCoveringTestTargets()) + { + 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()); + AZ_Error("File Creation", false, msg.c_str()); + throw DependencyException(msg); + } + else + { + createDependencies.emplace_back(AZStd::move(*sourceDependency)); + } + } + else + { + if (sourceDependency->GetNumCoveringTestTargets()) + { + const AZStd::string msg = AZStd::string::format("The newly-created file %s does not belong 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()); + AZ_Error("File Creation", false, msg.c_str()); + throw DependencyException(msg); + } + } + } + } + + // Update operations + for (const auto& updatedFile : changeList.m_updatedFiles) + { + auto sourceDependency = GetSourceDependency(updatedFile); + if (sourceDependency.has_value()) + { + if (sourceDependency->GetNumParentTargets()) + { + updateDependencies.emplace_back(AZStd::move(*sourceDependency)); + } + else + { + if (sourceDependency->GetNumCoveringTestTargets()) + { + AZ_Warning( + "File Update", false, AZStd::string::format("Source file %s is potentially an orphan (used by build targets " + "without explicitly being added to the build system, e.g. an include directive pulling in a header from the " + "repository). Running the covering tests for this file with instrumentation will confirm whether or nor this " + "is the case", updatedFile.c_str()).c_str()); + + updateDependencies.emplace_back(AZStd::move(*sourceDependency)); + coverageToDelete.push_back(updatedFile); + } + } + } + } + + // Delete operations + for (const auto& deletedFile : changeList.m_deletedFiles) + { + auto sourceDependency = GetSourceDependency(deletedFile); + if (sourceDependency.has_value()) + { + if (sourceDependency->GetNumParentTargets()) + { + if (sourceDependency->GetNumCoveringTestTargets()) + { + 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()); + AZ_Error("File Delete", false, msg.c_str()); + 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()); + AZ_Error("File Delete", false, msg.c_str()); + throw DependencyException(msg); + } + } + else + { + if (sourceDependency->GetNumCoveringTestTargets()) + { + deleteDependencies.emplace_back(AZStd::move(*sourceDependency)); + coverageToDelete.push_back(deletedFile); + } + } + } + } + + if (!coverageToDelete.empty()) + { + ClearSourceCoverage(coverageToDelete); + } + + return ChangeDependencyList(AZStd::move(createDependencies), AZStd::move(updateDependencies), AZStd::move(deleteDependencies)); + } +} diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h new file mode 100644 index 0000000000..0057fdc264 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h @@ -0,0 +1,120 @@ +/* + * 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 +#include +#include +#include + +#include +#include +#include +#include + +namespace TestImpact +{ + //! Representation of the repository source tree and its relation to the build targets and coverage data. + class DynamicDependencyMap + { + public: + //! Constructs the dependency map with entries for each build target's source files with empty test coverage data. + DynamicDependencyMap( + AZStd::vector&& productionTargetDescriptors, + AZStd::vector&& testTargetDescriptors); + + //! Gets the total number of production and test targets in the repository. + size_t GetNumTargets() const; + + //! Gets the total number of unique source files in the repository. + //! @note This includes autogen output sources. + size_t GetNumSources() const; + + //! Attempts to get the specified build target. + //! @param name The name of the build target to get. + //! @returns If found, the pointer to the specified build target, otherwise nullptr. + const BuildTarget* GetBuildTarget(const AZStd::string& name) const; + + //! Attempts to get the specified build target or throw TargetException. + //! @param name The name of the build target to get. + const BuildTarget* GetBuildTargetOrThrow(const AZStd::string& name) const; + + //! Attempts to get the specified target's specialized type. + //! @param name The name of the target to get. + //! @returns If found, the pointer to the specialized target, otherwise AZStd::monostate. + AZStd::variant GetTarget(const AZStd::string& name) const; + + //! Attempts to get the specified target's specialized type or throw TargetException. + //! @param name The name of the target to get. + AZStd::variant GetTargetOrThrow(const AZStd::string& name) const; + + //! Get the list of production targets in the repository. + const ProductionTargetList& GetProductionTargetList() const; + + //! Get the list of test targets in the repository. + const TestTargetList& GetTestTargetList() const; + + //! Gets the test targets covering the specified production target. + //! @param productionTarget The production target to retrieve the covering tests for. + AZStd::vector GetCoveringTestTargetsForProductionTarget(const ProductionTarget& productionTarget) const; + + //! Gets the source dependency for the specified source file. + //! @note Autogen input source dependencies are the consolidated source dependencies of all of their generated output sources. + //! @returns If found, the source dependency information for the specified source file, otherwise empty. + AZStd::optional GetSourceDependency(const AZStd::string& path) const; + + //! Gets the source dependency for the specified source file or throw DependencyException. + SourceDependency GetSourceDependencyOrThrow(const AZStd::string& path) const; + + //! Replaces the source coverage of the specified sources with the specified source coverage. + //! @note The covering targets for the parent test target(s) will not be pruned if those covering targets are removed. + //! @param sourceCoverageDelta The source coverage delta to replace in the dependency map. + void ReplaceSourceCoverage(const SourceCoveringTestsList& sourceCoverageDelta); + + //! Exports the coverage of all sources in the dependency map. + SourceCoveringTestsList ExportSourceCoverage() const; + + //! Gets the list of orphaned source files in the dependency map that have coverage data but belong to no parent build targets. + AZStd::vector GetOrphanSourceFiles() const; + + //! Applies the specified change list to the dynamic 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. + //! @returns The change list as resolved to the appropriate source dependencies. + [[nodiscard]] ChangeDependencyList ApplyAndResoveChangeList(const ChangeList& changeList); + + private: + //! Clears the source coverage of the specified sources. + //! @note The covering targets for the parent test target(s) will not be pruned if those covering targets are removed. + void ClearSourceCoverage(const AZStd::vector& paths); + + //! The sorted list of unique production targets in the repository. + ProductionTargetList m_productionTargets; + + //! The sorted list of unique test targets in the repository. + TestTargetList m_testTargets; + + //! The dependency map of sources to their parent build targets and covering test targets. + AZStd::unordered_map m_sourceDependencyMap; + + //! The map of build targets and their covering test targets. + AZStd::unordered_map> m_buildTargetCoverage; + + //! Mapping of autogen input sources to their generated output sources. + AZStd::unordered_map> m_autogenInputToOutputMap; + }; +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp new file mode 100644 index 0000000000..57a11e9f69 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp @@ -0,0 +1,58 @@ +/* + * 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 + +namespace TestImpact +{ + SourceCoveringTests::SourceCoveringTests(const AZStd::string& path, AZStd::vector&& coveringTestTargets) + : m_path(path) + , m_coveringTestTargets(AZStd::move(coveringTestTargets)) + { + } + + const AZStd::string& SourceCoveringTests::GetPath() const + { + return m_path; + } + + size_t SourceCoveringTests::GetNumCoveringTestTargets() const + { + return m_coveringTestTargets.size(); + } + + const AZStd::vector& SourceCoveringTests::GetCoveringTestTargets() const + { + return m_coveringTestTargets; + } + + SourceCoveringTestsList::SourceCoveringTestsList(AZStd::vector&& sourceCoveringTests) + : m_coverage(AZStd::move(sourceCoveringTests)) + { + AZStd::sort(m_coverage.begin(), m_coverage.end(), [](const SourceCoveringTests& lhs, const SourceCoveringTests& rhs) + { + return lhs.GetPath() < rhs.GetPath(); + }); + } + + size_t SourceCoveringTestsList::GetNumSources() const + { + return m_coverage.size(); + } + + const AZStd::vector& SourceCoveringTestsList::GetCoverage() const + { + return m_coverage; + } +} diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h new file mode 100644 index 0000000000..68b09d9e5a --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h @@ -0,0 +1,53 @@ +/* + * 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 +{ + //! Represents the unresolved test target coverage for a given source file. + class SourceCoveringTests + { + public: + SourceCoveringTests(const AZStd::string& path, AZStd::vector&& coveringTestTargets); + + //! Returns the path of this source file. + const AZStd::string& GetPath() const; + + //! Returns the number of unresolved test targets covering this source file. + size_t GetNumCoveringTestTargets() const; + + //! Returns the unresolved test targets covering this source file. + const AZStd::vector& GetCoveringTestTargets() const; + private: + AZStd::string m_path; //!< The path of this source file. + AZStd::vector m_coveringTestTargets; //!< The unresolved test targets that cover this source file. + }; + + //! Sorted collection of source file test coverage. + class SourceCoveringTestsList + { + public: + SourceCoveringTestsList(AZStd::vector&& sourceCoveringTests); + + //! Returns the number of source files in the collection. + size_t GetNumSources() const; + + //! Returns the source file coverages. + const AZStd::vector& GetCoverage() const; + private: + AZStd::vector m_coverage; //!< The collection of source file coverages. + }; +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.cpp index 3a6f5d0b79..0d50dbbf9a 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.cpp @@ -25,7 +25,7 @@ namespace TestImpact for (const auto& sourceCovered : moduleCovered.m_sources) { m_sourcesCovered.emplace_back(sourceCovered.m_path); - if (sourceCovered.m_coverage.has_value()) + if (!sourceCovered.m_coverage.empty()) { m_coverageLevel = CoverageLevel::Line; } @@ -51,7 +51,7 @@ namespace TestImpact return m_modules.size(); } - const AZStd::vector& TestCoverage::GetSourcesCovered() const + const AZStd::vector& TestCoverage::GetSourcesCovered() const { return m_sourcesCovered; } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.h index ab92b1bd80..7253a29df0 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Test/Run/TestImpactTestCoverage.h @@ -38,7 +38,7 @@ namespace TestImpact size_t GetNumModulesCovered() const; //! Returns the sorted set of unique sources covered (empty if no coverage). - const AZStd::vector& GetSourcesCovered() const; + const AZStd::vector& GetSourcesCovered() const; //! Returns the modules covered (empty if no coverage). const AZStd::vector& GetModuleCoverages() const; @@ -48,7 +48,7 @@ namespace TestImpact private: AZStd::vector m_modules; - AZStd::vector m_sourcesCovered; + AZStd::vector m_sourcesCovered; AZStd::optional m_coverageLevel; }; } // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Tests/Test/TestImpactTestCoverageTest.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Tests/Test/TestImpactTestCoverageTest.cpp index 538097d11e..c1d7ba3e4b 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Tests/Test/TestImpactTestCoverageTest.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Tests/Test/TestImpactTestCoverageTest.cpp @@ -19,12 +19,12 @@ namespace UnitTest { namespace { - AZ::IO::Path GenerateSourcePath(AZ::u32 index) + AZStd::string GenerateSourcePath(AZ::u32 index) { return AZStd::string::format("SourceFile%u", index); } - AZ::IO::Path GenerateModulePath(AZ::u32 index) + AZStd::string GenerateModulePath(AZ::u32 index) { return AZStd::string::format("Module%u", index); } @@ -47,7 +47,7 @@ namespace UnitTest sourceCoverage.m_path = GenerateSourcePath(index); if (coverageLevel == TestImpact::CoverageLevel::Line) { - sourceCoverage.m_coverage.emplace(GenerateLineCoverages(index + 1)); + sourceCoverage.m_coverage = GenerateLineCoverages(index + 1); } return sourceCoverage; @@ -152,9 +152,9 @@ namespace UnitTest if (m_coverageLevel == TestImpact::CoverageLevel::Line) { // Expect there to actually be line coverage data if this coverage was procedurally generated with line data - EXPECT_TRUE(sourceCoverage.m_coverage.has_value()); + EXPECT_FALSE(sourceCoverage.m_coverage.empty()); - const AZStd::vector& lineCoverages = sourceCoverage.m_coverage.value(); + const AZStd::vector& lineCoverages = sourceCoverage.m_coverage; // Expect the source's number of lines to match that of the corresponding procedurally generated source EXPECT_EQ(lineCoverages.size(), sourceIndex + 1); @@ -171,7 +171,7 @@ namespace UnitTest else { // Do not expect there to actually be line coverage data if this coverage was not procedurally generated with line data - EXPECT_FALSE(sourceCoverage.m_coverage.has_value()); + EXPECT_TRUE(sourceCoverage.m_coverage.empty()); } } } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake index fd65a16e39..f4128f7002 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake +++ b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake @@ -10,11 +10,37 @@ # set(FILES + Include/TestImpactFramework/TestImpactBitwise.h + Include/TestImpactFramework/TestImpactCallback.h Include/TestImpactFramework/TestImpactException.h Include/TestImpactFramework/TestImpactFrameworkPath.h - Include/TestImpactFramework/TestImpactCallback.h - Source/TestImpactException.cpp - Source/TestImpactFrameworkPath.cpp + Source/Artifact/TestImpactArtifactException.h + Source/Artifact/Factory/TestImpactBuildTargetDescriptorFactory.cpp + Source/Artifact/Factory/TestImpactBuildTargetDescriptorFactory.h + Source/Artifact/Factory/TestImpactChangeListFactory.cpp + Source/Artifact/Factory/TestImpactChangeListFactory.h + Source/Artifact/Factory/TestImpactTestEnumerationSuiteFactory.cpp + Source/Artifact/Factory/TestImpactTestEnumerationSuiteFactory.h + Source/Artifact/Factory/TestImpactTestRunSuiteFactory.cpp + Source/Artifact/Factory/TestImpactTestRunSuiteFactory.h + Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.cpp + Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.h + Source/Artifact/Factory/TestImpactModuleCoverageFactory.cpp + Source/Artifact/Factory/TestImpactModuleCoverageFactory.h + Source/Artifact/Static/TestImpactBuildTargetDescriptor.cpp + Source/Artifact/Static/TestImpactBuildTargetDescriptor.h + Source/Artifact/Static/TestImpactTargetDescriptorCompiler.cpp + Source/Artifact/Static/TestImpactTargetDescriptorCompiler.h + Source/Artifact/Static/TestImpactProductionTargetDescriptor.cpp + Source/Artifact/Static/TestImpactProductionTargetDescriptor.h + Source/Artifact/Static/TestImpactTestTargetMeta.h + Source/Artifact/Static/TestImpactTestTargetDescriptor.cpp + Source/Artifact/Static/TestImpactTestTargetDescriptor.h + Source/Artifact/Dynamic/TestImpactChangelist.h + Source/Artifact/Dynamic/TestImpactTestEnumerationSuite.h + Source/Artifact/Dynamic/TestImpactTestRunSuite.h + Source/Artifact/Dynamic/TestImpactTestSuite.h + Source/Artifact/Dynamic/TestImpactCoverage.h Source/Process/TestImpactProcess.cpp Source/Process/TestImpactProcess.h Source/Process/TestImpactProcessException.h @@ -26,4 +52,40 @@ set(FILES Source/Process/JobRunner/TestImpactProcessJobRunner.h Source/Process/Scheduler/TestImpactProcessScheduler.cpp Source/Process/Scheduler/TestImpactProcessScheduler.h + + Source/Target/TestImpactBuildTarget.cpp + Source/Target/TestImpactBuildTarget.h + Source/Target/TestImpactBuildTargetList.h + Source/Target/TestImpactProductionTarget.cpp + Source/Target/TestImpactProductionTarget.h + Source/Target/TestImpactProductionTargetList.h + Source/Target/TestImpactTargetException.h + Source/Target/TestImpactTestTarget.cpp + Source/Target/TestImpactTestTarget.h + Source/Target/TestImpactTestTargetList.h + Source/Test/Enumeration/TestImpactTestEnumeration.h + Source/Test/Enumeration/TestImpactTestEnumerationException.h + Source/Test/Enumeration/TestImpactTestEnumerationSerializer.cpp + Source/Test/Enumeration/TestImpactTestEnumerationSerializer.h + Source/Test/Enumeration/TestImpactTestEnumerator.cpp + Source/Test/Enumeration/TestImpactTestEnumerator.h + Source/Test/Run/TestImpactTestRunSerializer.cpp + Source/Test/Run/TestImpactTestRunSerializer.h + Source/Test/Run/TestImpactTestRunner.cpp + Source/Test/Run/TestImpactTestRunner.h + Source/Test/Run/TestImpactInstrumentedTestRunner.cpp + Source/Test/Run/TestImpactInstrumentedTestRunner.h + Source/Test/Run/TestImpactTestRun.cpp + Source/Test/Run/TestImpactTestRun.h + Source/Test/Run/TestImpactTestRunJobData.cpp + Source/Test/Run/TestImpactTestRunJobData.h + Source/Test/Run/TestImpactTestCoverage.cpp + Source/Test/Run/TestImpactTestCoverage.h + Source/Test/Run/TestImpactTestRunException.h + Source/Test/Job/TestImpactTestJobRunner.h + Source/Test/Job/TestImpactTestJobException.h + Source/Test/Job/TestImpactTestJobCommon.h + Source/Test/TestImpactTestSuiteContainer.h + Source/TestImpactException.cpp + Source/TestImpactFrameworkPath.cpp ) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake index 61059dc543..13f788c37b 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake +++ b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake @@ -10,13 +10,29 @@ # set(FILES + Tests/Artifact/TestImpactTargetDescriptorCompilerTest.cpp + Tests/Artifact/TestImpactBuildTargetDescriptorFactoryTest.cpp + Tests/Artifact/TestImpactModuleCoverageFactoryTest.cpp + Tests/Artifact/TestImpactChangeListFactoryTest.cpp + Tests/Artifact/TestImpactTestEnumerationSuiteFactoryTest.cpp + Tests/Artifact/TestImpactTestRunSuiteFactoryTest.cpp + Tests/Artifact/TestImpactTestTargetMetaMapFactoryTest.cpp + Tests/Process/TestImpactProcessSchedulerTest.cpp + Tests/Process/TestImpactProcessTest.cpp + Tests/Target/TestImpactBuildTargetTest.cpp Tests/TestImpactExceptionTest.cpp Tests/TestImpactFrameworkPathTest.cpp - Tests/TestImpactProcessSchedulerTest.cpp - Tests/TestImpactProcessTest.cpp - Tests/TestImpactProcessTestShared.cpp - Tests/TestImpactProcessTestShared.h + Tests/Test/TestImpactTestEnumeratorTest.cpp + Tests/Test/TestImpactTestEumerationSerializerTest.cpp + Tests/Test/TestImpactTestRunSerializerTest.cpp + Tests/Test/TestImpactTestRunnerTest.cpp + Tests/Test/TestImpactInstrumentedTestRunnerTest.cpp + Tests/Test/TestImpactTestCoverageTest.cpp + Tests/TestImpactTestJobRunnerCommon.h Tests/TestImpactTestMain.cpp + Tests/TestImpactTestUtils.cpp + Tests/TestImpactTestUtils.h + ) From 692dd21b892dfc647053734230e26bda3bbfa9b4 Mon Sep 17 00:00:00 2001 From: jonawals Date: Wed, 28 Apr 2021 17:26:28 +0100 Subject: [PATCH 2/6] Add missing SourceDependency. --- .../Dependency/TestImpactSourceDependency.cpp | 82 ++++++++++++++++ .../Dependency/TestImpactSourceDependency.h | 96 +++++++++++++++++++ .../testimpactframework_runtime_files.cmake | 10 +- 3 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp create mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp new file mode 100644 index 0000000000..49d82e341a --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp @@ -0,0 +1,82 @@ +/* + * 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 + +namespace TestImpact +{ + ParentTarget::ParentTarget(const TestTarget* target) + : m_buildTarget(target) + , m_target(target) + { + } + + ParentTarget::ParentTarget(const ProductionTarget* target) + : m_buildTarget(target) + , m_target(target) + { + } + + bool ParentTarget::operator==(const ParentTarget& other) const + { + return m_buildTarget == other.m_buildTarget; + } + + const BuildTarget* ParentTarget::GetBuildTarget() const + { + return m_buildTarget; + } + + const AZStd::variant& ParentTarget::GetTarget() const + { + return m_target; + } + + SourceDependency::SourceDependency( + const AZStd::string& path, + DependencyData&& dependencyData) + : m_path(path) + , m_dependencyData(AZStd::move(dependencyData)) + { + } + + const AZStd::string& SourceDependency::GetPath() const + { + return m_path; + } + + size_t SourceDependency::GetNumParentTargets() const + { + return m_dependencyData.m_parentTargets.size(); + } + + size_t SourceDependency::GetNumCoveringTestTargets() const + { + return m_dependencyData.m_coveringTestTargets.size(); + } + + const AZStd::unordered_set& SourceDependency::GetParentTargets() const + { + return m_dependencyData.m_parentTargets; + } + + const AZStd::unordered_set& SourceDependency::GetCoveringTestTargets() const + { + return m_dependencyData.m_coveringTestTargets; + } + +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h new file mode 100644 index 0000000000..e6cd89df9d --- /dev/null +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h @@ -0,0 +1,96 @@ +/* + * 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 BuildTarget; + class ProductionTarget; + class TestTarget; + + //! Representation of a source dependency's parent target. + class ParentTarget + { + public: + //! Constructor overload for test target types. + ParentTarget(const TestTarget* target); + + //! Constructor overload for production target types. + ParentTarget(const ProductionTarget* target); + + //! Returns the base build target pointer for this parent. + const BuildTarget* GetBuildTarget() const; + + //! Returns the specialized target pointer for this parent. + const AZStd::variant& GetTarget() const; + + bool operator==(const ParentTarget& other) const; + private: + const BuildTarget* m_buildTarget; //! The base built target pointer for this parent. + AZStd::variant m_target; //! The specialized target pointer for this parent. + }; +} + +namespace AZStd +{ + //! Hash function for ParentTarget types for use in maps and sets + template<> struct hash + { + size_t operator()(const TestImpact::ParentTarget& parentTarget) const noexcept + { + return reinterpret_cast(parentTarget.GetBuildTarget()); + } + }; +} + +namespace TestImpact +{ + struct DependencyData + { + AZStd::unordered_set m_parentTargets; + AZStd::unordered_set m_coveringTestTargets; + }; + + //! Test target coverage and build target dependency information for a given source file in the dynamic dependency map. + class SourceDependency + { + public: + SourceDependency( + const AZStd::string& path, + DependencyData&& dependencyData); + + //! Returns the path of this source file. + const AZStd::string& GetPath() const; + + //! Returns the number of parent build targets this source belongs to. + size_t GetNumParentTargets() const; + + //! Returns the number of test targets covering this source file. + size_t GetNumCoveringTestTargets() const; + + //! Returns the parent targets that this source file belongs to. + const AZStd::unordered_set& GetParentTargets() const; + + //! Returns the test targets covering this source file. + const AZStd::unordered_set& GetCoveringTestTargets() const; + private: + AZStd::string m_path; //!< The path of this source file. + DependencyData m_dependencyData; //!< + }; +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake index f4128f7002..19f3ec23f7 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake +++ b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake @@ -52,7 +52,15 @@ set(FILES Source/Process/JobRunner/TestImpactProcessJobRunner.h Source/Process/Scheduler/TestImpactProcessScheduler.cpp Source/Process/Scheduler/TestImpactProcessScheduler.h - + Source/Dependency/TestImpactDynamicDependencyMap.cpp + Source/Dependency/TestImpactDynamicDependencyMap.h + Source/Dependency/TestImpactChangeDependencyList.cpp + Source/Dependency/TestImpactChangeDependencyList.h + Source/Dependency/TestImpactDependencyException.h + Source/Dependency/TestImpactSourceDependency.h + Source/Dependency/TestImpactSourceDependency.cpp + Source/Dependency/TestImpactSourceCoveringTestsList.h + Source/Dependency/TestImpactSourceCoveringTestsList.cpp Source/Target/TestImpactBuildTarget.cpp Source/Target/TestImpactBuildTarget.h Source/Target/TestImpactBuildTargetList.h From 2b21b70635ef0d78b3af55adcf3c3018d491b840 Mon Sep 17 00:00:00 2001 From: jonawals Date: Thu, 29 Apr 2021 08:28:42 +0100 Subject: [PATCH 3/6] 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 4/6] 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 5/6] 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); From 39ba583ad7ae1761e13d356bd50b02ae29e3f6b4 Mon Sep 17 00:00:00 2001 From: jonawals Date: Fri, 30 Apr 2021 12:58:48 +0100 Subject: [PATCH 6/6] Address PR comments. --- .../TestImpactDynamicDependencyMap.cpp | 110 +++++++++--------- .../TestImpactDynamicDependencyMap.h | 5 +- .../TestImpactSourceCoveringTestsList.cpp | 7 +- .../TestImpactSourceCoveringTestsList.h | 3 +- .../Dependency/TestImpactSourceDependency.cpp | 19 +-- .../Dependency/TestImpactSourceDependency.h | 11 +- .../Source/Target/TestImpactBuildTarget.h | 10 ++ 7 files changed, 89 insertions(+), 76 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp index f3edbe2507..f99300ad4e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp @@ -25,7 +25,8 @@ namespace TestImpact { for (const auto& source : target->GetSources().m_staticSources) { - if (auto mapping = m_sourceDependencyMap.find(source); mapping != m_sourceDependencyMap.end()) + if (auto mapping = m_sourceDependencyMap.find(source); + mapping != m_sourceDependencyMap.end()) { // This is an existing entry in the dependency map so update the parent build targets with this target mapping->second.m_parentTargets.insert(target); @@ -97,13 +98,15 @@ namespace TestImpact return buildTarget; } - AZStd::variant DynamicDependencyMap::GetTarget(const AZStd::string& name) const + OptionalTarget DynamicDependencyMap::GetTarget(const AZStd::string& name) const { - if (auto testTarget = m_testTargets.GetTarget(name); testTarget != nullptr) + if (const auto testTarget = m_testTargets.GetTarget(name); + testTarget != nullptr) { return testTarget; } - else if (auto productionTarget = m_productionTargets.GetTarget(name); productionTarget != nullptr) + else if (auto productionTarget = m_productionTargets.GetTarget(name); + productionTarget != nullptr) { return productionTarget; } @@ -111,9 +114,9 @@ namespace TestImpact return AZStd::monostate{}; } - AZStd::variant DynamicDependencyMap::GetTargetOrThrow(const AZStd::string& name) const + Target DynamicDependencyMap::GetTargetOrThrow(const AZStd::string& name) const { - AZStd::variant buildTarget; + Target buildTarget; AZStd::visit([&buildTarget, &name](auto&& target) { if constexpr (IsProductionTarget || IsTestTarget) @@ -148,8 +151,8 @@ namespace TestImpact // Update the dependency with any new coverage data for (const auto& unresolvedTestTarget : sourceCoverage.GetCoveringTestTargets()) { - const TestTarget* testTarget = m_testTargets.GetTarget(unresolvedTestTarget); - if (testTarget) + if (const TestTarget* testTarget = m_testTargets.GetTarget(unresolvedTestTarget); + testTarget) { // Source to covering test target mapping sourceDependency.m_coveringTestTargets.insert(testTarget); @@ -179,12 +182,13 @@ namespace TestImpact { for (const auto& path : paths) { - if (const auto outputSources = m_autogenInputToOutputMap.find(path); outputSources != m_autogenInputToOutputMap.end()) + if (const auto outputSources = m_autogenInputToOutputMap.find(path); + outputSources != m_autogenInputToOutputMap.end()) { // Clearing the coverage data of an autogen input source instead clears the coverage data of its output sources for (const auto& outputSource : outputSources->second) { - ReplaceSourceCoverage(SourceCoveringTestsList({ SourceCoveringTests(outputSource, { }) })); + ReplaceSourceCoverage(SourceCoveringTestsList({ SourceCoveringTests(outputSource) })); } } else @@ -207,7 +211,8 @@ namespace TestImpact AZStd::vector DynamicDependencyMap::GetCoveringTestTargetsForProductionTarget(const ProductionTarget& productionTarget) const { AZStd::vector coveringTestTargets; - if (const auto coverage = m_buildTargetCoverage.find(&productionTarget); coverage != m_buildTargetCoverage.end()) + if (const auto coverage = m_buildTargetCoverage.find(&productionTarget); + coverage != m_buildTargetCoverage.end()) { coveringTestTargets.reserve(coverage->second.size()); AZStd::copy(coverage->second.begin(), coverage->second.end(), AZStd::back_inserter(coveringTestTargets)); @@ -280,7 +285,7 @@ namespace TestImpact coverage.push_back(SourceCoveringTests(path, AZStd::move(souceCoveringTests))); } - return coverage; + return SourceCoveringTestsList(AZStd::move(coverage)); } AZStd::vector DynamicDependencyMap::GetOrphanSourceFiles() const @@ -313,31 +318,18 @@ namespace TestImpact auto sourceDependency = GetSourceDependency(createdFile); if (sourceDependency.has_value()) { + if (sourceDependency->GetNumCoveringTestTargets()) + { + 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()); + AZ_Error("File Creation", false, msg.c_str()); + throw DependencyException(msg); + } + if (sourceDependency->GetNumParentTargets()) { - if (sourceDependency->GetNumCoveringTestTargets()) - { - 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()); - AZ_Error("File Creation", false, msg.c_str()); - throw DependencyException(msg); - } - else - { - createDependencies.emplace_back(AZStd::move(*sourceDependency)); - } - } - else - { - if (sourceDependency->GetNumCoveringTestTargets()) - { - const AZStd::string msg = AZStd::string::format("The newly-created file %s does not belong 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()); - AZ_Error("File Creation", false, msg.c_str()); - throw DependencyException(msg); - } + createDependencies.emplace_back(AZStd::move(*sourceDependency)); } } } @@ -373,33 +365,35 @@ namespace TestImpact for (const auto& deletedFile : changeList.m_deletedFiles) { auto sourceDependency = GetSourceDependency(deletedFile); - if (sourceDependency.has_value()) + if (!sourceDependency.has_value()) { - if (sourceDependency->GetNumParentTargets()) + continue; + } + + if (sourceDependency->GetNumParentTargets()) + { + if (sourceDependency->GetNumCoveringTestTargets()) { - if (sourceDependency->GetNumCoveringTestTargets()) - { - 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()); - AZ_Error("File Delete", false, msg.c_str()); - 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()); - AZ_Error("File Delete", false, msg.c_str()); - throw DependencyException(msg); - } + 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()); + AZ_Error("File Delete", false, msg.c_str()); + throw DependencyException(msg); } else { - if (sourceDependency->GetNumCoveringTestTargets()) - { - deleteDependencies.emplace_back(AZStd::move(*sourceDependency)); - coverageToDelete.push_back(deletedFile); - } + 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); + } + } + else + { + if (sourceDependency->GetNumCoveringTestTargets()) + { + deleteDependencies.emplace_back(AZStd::move(*sourceDependency)); + coverageToDelete.push_back(deletedFile); } } } @@ -411,4 +405,4 @@ namespace TestImpact return ChangeDependencyList(AZStd::move(createDependencies), AZStd::move(updateDependencies), AZStd::move(deleteDependencies)); } -} +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h index 0057fdc264..527a6555c6 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h @@ -24,7 +24,6 @@ #include #include #include -#include namespace TestImpact { @@ -56,11 +55,11 @@ namespace TestImpact //! Attempts to get the specified target's specialized type. //! @param name The name of the target to get. //! @returns If found, the pointer to the specialized target, otherwise AZStd::monostate. - AZStd::variant GetTarget(const AZStd::string& name) const; + OptionalTarget GetTarget(const AZStd::string& name) const; //! Attempts to get the specified target's specialized type or throw TargetException. //! @param name The name of the target to get. - AZStd::variant GetTargetOrThrow(const AZStd::string& name) const; + Target GetTargetOrThrow(const AZStd::string& name) const; //! Get the list of production targets in the repository. const ProductionTargetList& GetProductionTargetList() const; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp index 57a11e9f69..792eb1f0b2 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp @@ -16,6 +16,11 @@ namespace TestImpact { + SourceCoveringTests::SourceCoveringTests(const AZStd::string& path) + : m_path(path) + { + } + SourceCoveringTests::SourceCoveringTests(const AZStd::string& path, AZStd::vector&& coveringTestTargets) : m_path(path) , m_coveringTestTargets(AZStd::move(coveringTestTargets)) @@ -55,4 +60,4 @@ namespace TestImpact { return m_coverage; } -} +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h index 68b09d9e5a..52f3a7c3f6 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h @@ -21,6 +21,7 @@ namespace TestImpact class SourceCoveringTests { public: + explicit SourceCoveringTests(const AZStd::string& path); SourceCoveringTests(const AZStd::string& path, AZStd::vector&& coveringTestTargets); //! Returns the path of this source file. @@ -40,7 +41,7 @@ namespace TestImpact class SourceCoveringTestsList { public: - SourceCoveringTestsList(AZStd::vector&& sourceCoveringTests); + explicit SourceCoveringTestsList(AZStd::vector&& sourceCoveringTests); //! Returns the number of source files in the collection. size_t GetNumSources() const; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp index 49d82e341a..39193d4dd9 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp @@ -20,28 +20,33 @@ namespace TestImpact { ParentTarget::ParentTarget(const TestTarget* target) - : m_buildTarget(target) - , m_target(target) + : m_target(target) { } ParentTarget::ParentTarget(const ProductionTarget* target) - : m_buildTarget(target) - , m_target(target) + : m_target(target) { } bool ParentTarget::operator==(const ParentTarget& other) const { - return m_buildTarget == other.m_buildTarget; + return GetBuildTarget() == other.GetBuildTarget(); } const BuildTarget* ParentTarget::GetBuildTarget() const { - return m_buildTarget; + const BuildTarget* buildTarget; + AZStd::visit([&buildTarget](auto&& target) + { + buildTarget = target; + + }, m_target); + + return buildTarget; } - const AZStd::variant& ParentTarget::GetTarget() const + const Target& ParentTarget::GetTarget() const { return m_target; } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h index e6cd89df9d..a3b6783d13 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h @@ -12,15 +12,15 @@ #pragma once +#include + #include #include #include #include -#include namespace TestImpact { - class BuildTarget; class ProductionTarget; class TestTarget; @@ -38,12 +38,11 @@ namespace TestImpact const BuildTarget* GetBuildTarget() const; //! Returns the specialized target pointer for this parent. - const AZStd::variant& GetTarget() const; + const Target& GetTarget() const; bool operator==(const ParentTarget& other) const; private: - const BuildTarget* m_buildTarget; //! The base built target pointer for this parent. - AZStd::variant m_target; //! The specialized target pointer for this parent. + Target m_target; //! The specialized target pointer for this parent. }; } @@ -91,6 +90,6 @@ namespace TestImpact const AZStd::unordered_set& GetCoveringTestTargets() const; private: AZStd::string m_path; //!< The path of this source file. - DependencyData m_dependencyData; //!< + DependencyData m_dependencyData; //!< The dependency data for this source file. }; } // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Target/TestImpactBuildTarget.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Target/TestImpactBuildTarget.h index d4346c15ef..0cd6369472 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Target/TestImpactBuildTarget.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Target/TestImpactBuildTarget.h @@ -14,10 +14,20 @@ #include +#include #include namespace TestImpact { + class TestTarget; + class ProductionTarget; + + //! Holder for specializations of BuildTarget. + using Target = AZStd::variant; + + //! Optional holder for specializations of BuildTarget. + using OptionalTarget = AZStd::variant; + //! Type id for querying specialized derived target types from base pointer/reference. enum class TargetType : bool {