birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Address PR comments.

Browse Source
This commit is contained in:
jonawals
2021-04-30 12:58:48 +01:00
parent c790b26851
commit 39ba583ad7
7 changed files with 89 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp
+52 -58
View File
@@ -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<AZStd::monostate, const TestTarget*, const ProductionTarget*> 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<const TestTarget*, const ProductionTarget*> DynamicDependencyMap::GetTargetOrThrow(const AZStd::string& name) const
Target DynamicDependencyMap::GetTargetOrThrow(const AZStd::string& name) const
{
AZStd::variant<const TestTarget*, const ProductionTarget*> buildTarget;
Target buildTarget;
AZStd::visit([&buildTarget, &name](auto&& target)
{
if constexpr (IsProductionTarget<decltype(target)> || IsTestTarget<decltype(target)>)
@@ -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<const TestTarget*> DynamicDependencyMap::GetCoveringTestTargetsForProductionTarget(const ProductionTarget& productionTarget) const
{
AZStd::vector<const TestTarget*> 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<AZStd::string> 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
Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h
+2 -3
View File
@@ -24,7 +24,6 @@
#include <AzCore/std/containers/unordered_map.h>
#include <AzCore/std/containers/unordered_set.h>
#include <AzCore/std/containers/vector.h>
#include <AzCore/std/containers/variant.h>
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<AZStd::monostate, const TestTarget*, const ProductionTarget*> 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<const TestTarget*, const ProductionTarget*> 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;
Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.cpp
+6 -1
View File
@@ -16,6 +16,11 @@
namespace TestImpact
{
SourceCoveringTests::SourceCoveringTests(const AZStd::string& path)
: m_path(path)
{
}
SourceCoveringTests::SourceCoveringTests(const AZStd::string& path, AZStd::vector<AZStd::string>&& coveringTestTargets)
: m_path(path)
, m_coveringTestTargets(AZStd::move(coveringTestTargets))
@@ -55,4 +60,4 @@ namespace TestImpact
{
return m_coverage;
}
}
} // namespace TestImpact
Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceCoveringTestsList.h
+2 -1
View File
@@ -21,6 +21,7 @@ namespace TestImpact
class SourceCoveringTests
{
public:
explicit SourceCoveringTests(const AZStd::string& path);
SourceCoveringTests(const AZStd::string& path, AZStd::vector<AZStd::string>&& coveringTestTargets);
//! Returns the path of this source file.
@@ -40,7 +41,7 @@ namespace TestImpact
class SourceCoveringTestsList
{
public:
SourceCoveringTestsList(AZStd::vector<SourceCoveringTests>&& sourceCoveringTests);
explicit SourceCoveringTestsList(AZStd::vector<SourceCoveringTests>&& sourceCoveringTests);
//! Returns the number of source files in the collection.
size_t GetNumSources() const;
Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.cpp
+12 -7
View File
@@ -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<const ProductionTarget*, const TestTarget*>& ParentTarget::GetTarget() const
const Target& ParentTarget::GetTarget() const
{
return m_target;
}
Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactSourceDependency.h
+5 -6
View File
@@ -12,15 +12,15 @@
#pragma once
#include <Target/TestImpactBuildTarget.h>
#include <AzCore/std/string/string.h>
#include <AzCore/std/containers/unordered_map.h>
#include <AzCore/std/containers/unordered_set.h>
#include <AzCore/std/containers/vector.h>
#include <AzCore/std/containers/variant.h>
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<const ProductionTarget*, const TestTarget*>& 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<const ProductionTarget*, const TestTarget*> 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<const TestTarget*>& 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
Code/Tools/TestImpactFramework/Runtime/Code/Source/Target/TestImpactBuildTarget.h
+10
View File
@@ -14,10 +14,20 @@
#include <Artifact/Static/TestImpactBuildTargetDescriptor.h>
#include <AzCore/std/containers/variant.h>
#include <AzCore/std/string/string.h>
namespace TestImpact
{
class TestTarget;
class ProductionTarget;
//! Holder for specializations of BuildTarget.
using Target = AZStd::variant<const TestTarget*, const ProductionTarget*>;
//! Optional holder for specializations of BuildTarget.
using OptionalTarget = AZStd::variant<AZStd::monostate, const TestTarget*, const ProductionTarget*>;
//! Type id for querying specialized derived target types from base pointer/reference.
enum class TargetType : bool
{