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-06-01 12:46:00 +01:00
parent f21794b193
commit 9fe91ec705
7 changed files with 92 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Tools/TestImpactFramework/Frontend/Console/Executable/Code/Source/TestImpactConsole.cpp
+2 -2
View File
@@ -10,7 +10,7 @@
*
*/
#include <TestImpactFramework/TestImpactConsoleApplication.h>
#include <TestImpactFramework/TestImpactConsoleMain.h>
#include <AzCore/Memory/OSAllocator.h>
#include <AzCore/Memory/SystemAllocator.h>
@@ -22,8 +22,8 @@ int main(int argc, char** argv)
TestImpact::Console::ReturnCode returnCode = TestImpact::Console::Main(argc, argv);
AZ::AllocatorInstance<AZ::OSAllocator>::Destroy();
AZ::AllocatorInstance<AZ::SystemAllocator>::Destroy();
AZ::AllocatorInstance<AZ::OSAllocator>::Destroy();
return static_cast<int>(returnCode);
}
Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Include/TestImpactFramework/TestImpactConsoleApplication.h → Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Include/TestImpactFramework/TestImpactConsoleMain.h
View File
Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleApplication.cpp → Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp
+72 -69
View File
@@ -14,7 +14,7 @@
#include <TestImpactFramework/TestImpactChangeListException.h>
#include <TestImpactFramework/TestImpactConfigurationException.h>
#include <TestImpactFramework/TestImpactRuntimeException.h>
#include <TestImpactFramework/TestImpactConsoleApplication.h>
#include <TestImpactFramework/TestImpactConsoleMain.h>
#include <TestImpactFramework/TestImpactChangeListSerializer.h>
#include <TestImpactFramework/TestImpactChangeList.h>
#include <TestImpactFramework/TestImpactRuntime.h>
@@ -22,7 +22,7 @@
#include <TestImpactFramework/TestImpactClientTestSelection.h>
#include <TestImpactFramework/TestImpactRuntime.h>
#include <TestImpactConsoleTestSequence.h>
#include <TestImpactConsoleTestSequenceEventHandler.h>
#include <TestImpactCommandLineOptions.h>
#include <TestImpactConfigurationFactory.h>
#include <TestImpactCommandLineOptionsException.h>
@@ -79,6 +79,64 @@ namespace TestImpact
}
}
//! Wrapper around impact analysis sequences to handle the case where the safe mode option is active.
ReturnCode WrappedImpactAnalysisTestSequence(
TestSequenceEventHandler& sequenceEventHandler,
const CommandLineOptions& options,
Runtime& runtime,
const AZStd::optional<ChangeList>& changeList)
{
AZ_TestImpact_Eval(
changeList.has_value(),
CommandLineOptionsException,
"Expected a change list for impact analysis but none was provided");
TestSequenceResult result = TestSequenceResult::Failure;
if (options.HasSafeMode())
{
auto [selectedResult, discardedResult] = runtime.SafeImpactAnalysisTestSequence(
changeList.value(),
options.GetSuitesFilter(),
options.GetTestPrioritizationPolicy(),
options.GetTestTargetTimeout(),
options.GetGlobalTimeout(),
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler));
// Handling the possible timeout and failure permutations of the selected and discarded test results is splitting hairs
// so apply the following, admittedly arbitrary, rules to determine what the composite test sequence result should be
if (selectedResult == TestSequenceResult::Success && discardedResult == TestSequenceResult::Success)
{
// Trivial case: both sequences succeeded
result = TestSequenceResult::Success;
}
else if (selectedResult == TestSequenceResult::Failure || discardedResult == TestSequenceResult::Failure)
{
// One sequence failed whilst the other sequence either succeeded or timed out
result = TestSequenceResult::Failure;
}
else
{
// One sequence timed out whilst the other sequence succeeded or both sequences timed out
result = TestSequenceResult::Timeout;
}
}
else
{
result = runtime.ImpactAnalysisTestSequence(
changeList.value(),
options.GetTestPrioritizationPolicy(),
options.GetTestTargetTimeout(),
options.GetGlobalTimeout(),
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler));
}
return GetReturnCodeForTestSequenceResult(result);
};
//! Entry point for the test impact analysis framework console front end application.
ReturnCode Main(int argc, char** argv)
{
@@ -123,65 +181,10 @@ namespace TestImpact
}
else
{
std::cout << "Test impact analysis data for this repository was not found, seed or regular sequence fallbacks will be used.\n";
}
TestSequence sequence(&options.GetSuitesFilter());
// Wrapper around impact analysis sequences to handle the case where the safe mode option is active
const auto impactAnalysisTestSequence = [&sequence, &options, &runtime, &changeList]()
{
AZ_TestImpact_Eval(
changeList.has_value(),
CommandLineOptionsException,
"Expected a change list for impact analysis but none was provided");
TestSequenceResult result = TestSequenceResult::Failure;
if (options.HasSafeMode())
{
auto [selectedResult, discardedResult] = runtime.SafeImpactAnalysisTestSequence(
changeList.value(),
options.GetSuitesFilter(),
options.GetTestPrioritizationPolicy(),
options.GetTestTargetTimeout(),
options.GetGlobalTimeout(),
AZStd::ref(sequence),
AZStd::ref(sequence),
AZStd::ref(sequence));
// Handling the possible timeout and failure permutations of the selected and discarded test results is splitting hairs
// so apply the following, admittedly arbitrary, rules to determine what the composite test sequence result should be
if (selectedResult == TestSequenceResult::Success && discardedResult == TestSequenceResult::Success)
{
// Trivial case: both sequences succeeded
result = TestSequenceResult::Success;
}
else if (selectedResult == TestSequenceResult::Failure || discardedResult == TestSequenceResult::Failure)
{
// One sequence failed whilst the other sequence either succeeded or timed out
result = TestSequenceResult::Failure;
}
else
{
// One sequence timed out whilst the other sequence succeeded or both sequences timed out
result = TestSequenceResult::Timeout;
}
}
else
{
result = runtime.ImpactAnalysisTestSequence(
changeList.value(),
options.GetTestPrioritizationPolicy(),
options.GetTestTargetTimeout(),
options.GetGlobalTimeout(),
AZStd::ref(sequence),
AZStd::ref(sequence),
AZStd::ref(sequence));
}
return GetReturnCodeForTestSequenceResult(result);
};
TestSequenceEventHandler sequenceEventHandler(&options.GetSuitesFilter());
switch (const auto type = options.GetTestSequenceType())
{
@@ -191,9 +194,9 @@ namespace TestImpact
options.GetSuitesFilter(),
options.GetTestTargetTimeout(),
options.GetGlobalTimeout(),
AZStd::ref(sequence),
AZStd::ref(sequence),
AZStd::ref(sequence));
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler));
return GetReturnCodeForTestSequenceResult(result);
}
@@ -202,30 +205,30 @@ namespace TestImpact
const auto result = runtime.SeededTestSequence(
options.GetTestTargetTimeout(),
options.GetGlobalTimeout(),
AZStd::ref(sequence),
AZStd::ref(sequence),
AZStd::ref(sequence));
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler));
return GetReturnCodeForTestSequenceResult(result);
}
case TestSequenceType::ImpactAnalysis:
{
return impactAnalysisTestSequence();
return WrappedImpactAnalysisTestSequence(sequenceEventHandler, options, runtime, changeList);
}
case TestSequenceType::ImpactAnalysisOrSeed:
{
if (runtime.HasImpactAnalysisData())
{
return impactAnalysisTestSequence();
return WrappedImpactAnalysisTestSequence(sequenceEventHandler, options, runtime, changeList);
}
else
{
const auto result = runtime.SeededTestSequence(
options.GetTestTargetTimeout(),
options.GetGlobalTimeout(),
AZStd::ref(sequence),
AZStd::ref(sequence),
AZStd::ref(sequence));
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler),
AZStd::ref(sequenceEventHandler));
return GetReturnCodeForTestSequenceResult(result);
}
Code/Tools/TestImpactFramework/Runtime/Code/Source/Platform/Windows/platform_windows_files.cmake
+1
View File
@@ -17,4 +17,5 @@ set(FILES
Process/TestImpactWin32_Pipe.cpp
Process/TestImpactWin32_Pipe.h
TestEngine/JobRunner/TestImpactWin32_TestTargetExtension.cpp
TestEngine/TestImpactWin32_TestEngineJobFailure.cpp
)
Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRepoPath.cpp
+11 -11
View File
@@ -32,70 +32,70 @@ namespace TestImpact
return *this;
}
inline RepoPath operator/(const RepoPath& lhs, const AZ::IO::PathView& rhs)
RepoPath operator/(const RepoPath& lhs, const AZ::IO::PathView& rhs)
{
RepoPath result(lhs);
result.m_path /= RepoPath(rhs).m_path;
return result;
}
inline RepoPath operator/(const RepoPath& lhs, AZStd::string_view rhs)
RepoPath operator/(const RepoPath& lhs, AZStd::string_view rhs)
{
RepoPath result(lhs);
result.m_path /= RepoPath(rhs).m_path;
return result;
}
inline RepoPath operator/(const RepoPath& lhs, const RepoPath::value_type* rhs)
RepoPath operator/(const RepoPath& lhs, const RepoPath::value_type* rhs)
{
RepoPath result(lhs);
result.m_path /= RepoPath(rhs).m_path;
return result;
}
inline RepoPath operator/(const RepoPath& lhs, const RepoPath& rhs)
RepoPath operator/(const RepoPath& lhs, const RepoPath& rhs)
{
RepoPath result(lhs);
result.m_path /= rhs.m_path;
return result;
}
inline RepoPath& RepoPath::operator/=(const AZ::IO::PathView& rhs)
RepoPath& RepoPath::operator/=(const AZ::IO::PathView& rhs)
{
m_path /= RepoPath(rhs).m_path;
return *this;
}
inline RepoPath& RepoPath::operator/=(AZStd::string_view rhs)
RepoPath& RepoPath::operator/=(AZStd::string_view rhs)
{
m_path /= RepoPath(rhs).m_path;
return *this;
}
inline RepoPath& RepoPath::operator/=(const RepoPath::value_type* rhs)
RepoPath& RepoPath::operator/=(const RepoPath::value_type* rhs)
{
m_path /= RepoPath(rhs).m_path;
return *this;
}
inline RepoPath& RepoPath::operator/=(const RepoPath& rhs)
RepoPath& RepoPath::operator/=(const RepoPath& rhs)
{
m_path /= rhs.m_path;
return *this;
}
inline bool operator==(const RepoPath& lhs, const RepoPath& rhs) noexcept
bool operator==(const RepoPath& lhs, const RepoPath& rhs) noexcept
{
return lhs.m_path.Compare(rhs.m_path) == 0;
}
inline bool operator!=(const RepoPath& lhs, const RepoPath& rhs) noexcept
bool operator!=(const RepoPath& lhs, const RepoPath& rhs) noexcept
{
return lhs.m_path.Compare(rhs.m_path) != 0;
}
inline bool operator<([[maybe_unused]] const RepoPath& lhs, [[maybe_unused]] const RepoPath& rhs) noexcept
bool operator<([[maybe_unused]] const RepoPath& lhs, [[maybe_unused]] const RepoPath& rhs) noexcept
{
return lhs.m_path.String() < rhs.m_path.String();
}
Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp
+5 -5
View File
@@ -309,7 +309,7 @@ namespace TestImpact
if (testSequenceEndCallback.has_value())
{
(*testSequenceEndCallback)(CreateSequenceFailureReport(testJobs), timer.Elapsed());
(*testSequenceEndCallback)(GenerateSequenceFailureReport(testJobs), timer.Elapsed());
}
return result;
@@ -353,7 +353,7 @@ namespace TestImpact
if (testSequenceEndCallback.has_value())
{
(*testSequenceEndCallback)(CreateSequenceFailureReport(testJobs), timer.Elapsed());
(*testSequenceEndCallback)(GenerateSequenceFailureReport(testJobs), timer.Elapsed());
}
return result;
@@ -419,8 +419,8 @@ namespace TestImpact
if (testSequenceEndCallback.has_value())
{
(*testSequenceEndCallback)(
CreateSequenceFailureReport(selectedTestJobs),
CreateSequenceFailureReport(discardedTestJobs),
GenerateSequenceFailureReport(selectedTestJobs),
GenerateSequenceFailureReport(discardedTestJobs),
timer.Elapsed());
}
@@ -471,7 +471,7 @@ namespace TestImpact
if (testSequenceEndCallback.has_value())
{
(*testSequenceEndCallback)(CreateSequenceFailureReport(testJobs), timer.Elapsed());
(*testSequenceEndCallback)(GenerateSequenceFailureReport(testJobs), timer.Elapsed());
}
return result;
Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h
+1 -1
View File
@@ -114,7 +114,7 @@ namespace TestImpact
}
case Client::TestRunResult::TestFailures:
{
testRunFailures.push_back(ExtractTestRunFailure(testJob));
testRunFailures.push_back(GenerateTestRunFailure(testJob));
break;
}
default: