From 52e61fa3cac8b7bae1ae78019dc3306895669cb7 Mon Sep 17 00:00:00 2001 From: jonawals Date: Thu, 27 May 2021 13:24:48 +0100 Subject: [PATCH] Remove JobExceptionPolicy and move knoen error handling --- .../TestImpactWin32_TestTargetExtension.cpp | 2 +- .../Enumeration/TestImpactTestEnumerator.cpp | 105 ++++++++---------- .../Enumeration/TestImpactTestEnumerator.h | 14 --- .../TestImpactTestJobInfoGenerator.cpp | 12 +- .../JobRunner/TestImpactTestJobRunner.h | 41 ++----- .../Run/TestImpactInstrumentedTestRunner.cpp | 4 +- .../Run/TestImpactInstrumentedTestRunner.h | 1 - .../TestEngine/Run/TestImpactTestRunner.cpp | 4 +- .../TestEngine/Run/TestImpactTestRunner.h | 1 - .../TestEngine/TestImpactTestEngine.cpp | 91 +++++---------- .../Source/TestEngine/TestImpactTestEngine.h | 1 + 11 files changed, 88 insertions(+), 188 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Platform/Windows/TestEngine/JobRunner/TestImpactWin32_TestTargetExtension.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Platform/Windows/TestEngine/JobRunner/TestImpactWin32_TestTargetExtension.cpp index 12c78f91cf..14a1a28ba7 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Platform/Windows/TestEngine/JobRunner/TestImpactWin32_TestTargetExtension.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Platform/Windows/TestEngine/JobRunner/TestImpactWin32_TestTargetExtension.cpp @@ -41,4 +41,4 @@ namespace TestImpact } } } -} +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp index f7b6d4a264..afc40036e3 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp @@ -10,7 +10,7 @@ * */ -#include +#include #include #include @@ -22,35 +22,6 @@ namespace TestImpact { - namespace - { - void WriteCacheFile( - const TestEnumeration& enumeration, const RepoPath& path, Bitwise::CacheExceptionPolicy cacheExceptionPolicy) - { - const AZStd::string cacheJSON = SerializeTestEnumeration(enumeration); - const AZStd::vector cacheBytes(cacheJSON.begin(), cacheJSON.end()); - AZ::IO::SystemFile cacheFile; - - if (!cacheFile.Open( - path.c_str(), - AZ::IO::SystemFile::SF_OPEN_CREATE | AZ::IO::SystemFile::SF_OPEN_CREATE_PATH | AZ::IO::SystemFile::SF_OPEN_WRITE_ONLY)) - { - AZ_TestImpact_Eval( - !IsFlagSet(cacheExceptionPolicy, Bitwise::CacheExceptionPolicy::OnCacheWriteFailure), TestEngineException, - "Couldn't open cache file for writing"); - return; - } - - if (cacheFile.Write(cacheBytes.data(), cacheBytes.size()) == 0) - { - AZ_TestImpact_Eval( - !IsFlagSet(cacheExceptionPolicy, Bitwise::CacheExceptionPolicy::OnCacheWriteFailure), TestEngineException, - "Couldn't write cache file data"); - return; - } - } - } // namespace - TestEnumeration ParseTestEnumerationFile(const RepoPath& enumerationFile) { return TestEnumeration(GTest::TestEnumerationSuitesFactory(ReadFileContents(enumerationFile))); @@ -79,8 +50,6 @@ namespace TestImpact AZStd::pair> TestEnumerator::Enumerate( const AZStd::vector& jobInfos, - CacheExceptionPolicy cacheExceptionPolicy, - JobExceptionPolicy jobExceptionPolicy, AZStd::optional enumerationTimeout, AZStd::optional enumeratorTimeout, AZStd::optional clientCallback) @@ -88,50 +57,67 @@ namespace TestImpact AZStd::vector cachedJobs; AZStd::vector jobQueue; - for (const auto& jobInfo : jobInfos) + for (auto jobInfo = jobInfos.begin(); jobInfo != jobInfos.end(); ++jobInfo) { // If this job has a cache read policy attempt to read the cache - if (jobInfo.GetCache().has_value() && jobInfo.GetCache()->m_policy == JobData::CachePolicy::Read) + if (jobInfo->GetCache().has_value()) { - JobMeta meta; - AZStd::optional enumeration; + if (jobInfo->GetCache()->m_policy == JobData::CachePolicy::Read) + { + JobMeta meta; + AZStd::optional enumeration; - try - { - enumeration = TestEnumeration(DeserializeTestEnumeration(ReadFileContents(jobInfo.GetCache()->m_file))); - } - catch (const TestEngineException& e) - { - AZ_Printf("Enumerate", "Enumeration cache error: %s", e.what()); - } - - // Even though cached jobs don't get executed we still give the client the opportunity to handle the job state - // change in order to make the caching process transparent to the client - if (enumeration.has_value()) - { - if (m_clientJobCallback.has_value()) + try { - (*m_clientJobCallback)(jobInfo, meta); + enumeration = TestEnumeration(DeserializeTestEnumeration(ReadFileContents(jobInfo->GetCache()->m_file))); + } + catch (const TestEngineException& e) + { + AZ_Printf("Enumerate", "Enumeration cache error: %s", e.what()); + DeleteFile(jobInfo->GetCache()->m_file); } - // Cache read successfully, this job will not be placed in the job queue - cachedJobs.emplace_back(Job(jobInfo, AZStd::move(meta), AZStd::move(enumeration))); + // Even though cached jobs don't get executed we still give the client the opportunity to handle the job state + // change in order to make the caching process transparent to the client + if (enumeration.has_value()) + { + // Cache read successfully, this job will not be placed in the job queue + cachedJobs.emplace_back(Job(*jobInfo, AZStd::move(meta), AZStd::move(enumeration))); + + if (m_clientJobCallback.has_value() && (*m_clientJobCallback)(*jobInfo, meta) == ProcessCallbackResult::Abort) + { + // Client chose to abort so we will copy over the existing cache enumerations and fill the rest with blanks + AZStd::vector jobs(cachedJobs); + for (auto emptyJobInfo = ++jobInfo; emptyJobInfo != jobInfos.end(); ++emptyJobInfo) + { + jobs.emplace_back(Job(*emptyJobInfo, {}, AZStd::nullopt)); + } + + return { ProcessSchedulerResult::UserAborted, jobs }; + } + } + else + { + // The cache read failed and exception policy for cache read failures is not to throw so instead place this + // job in the job queue + jobQueue.emplace_back(*jobInfo); + } } else { - // The cache read failed and exception policy for cache read failures is not to throw so instead place this job in the - // job queue - jobQueue.emplace_back(jobInfo); + // This job has no cache read policy so delete the cache and place in job queue + DeleteFile(jobInfo->GetCache()->m_file); + jobQueue.emplace_back(*jobInfo); } } else { // This job has no cache read policy so place in job queue - jobQueue.emplace_back(jobInfo); + jobQueue.emplace_back(*jobInfo); } } - const auto payloadGenerator = [this, cacheExceptionPolicy](const JobDataMap& jobDataMap) + const auto payloadGenerator = [this](const JobDataMap& jobDataMap) { PayloadMap enumerations; for (const auto& [jobId, jobData] : jobDataMap) @@ -146,7 +132,7 @@ namespace TestImpact // Write out the enumeration to a cache file if we have a cache write policy for this job if (jobInfo->GetCache().has_value() && jobInfo->GetCache()->m_policy == JobData::CachePolicy::Write) { - WriteCacheFile(enumeration.value(), jobInfo->GetCache()->m_file, cacheExceptionPolicy); + WriteFileContents(SerializeTestEnumeration(enumeration.value()), jobInfo->GetCache()->m_file); } } catch (const Exception& e) @@ -163,7 +149,6 @@ namespace TestImpact // Generate the enumeration results for the jobs that weren't cached auto [result, jobs] = ExecuteJobs( jobQueue, - jobExceptionPolicy, payloadGenerator, StdOutputRouting::None, StdErrorRouting::None, diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.h index c802b92529..196700998f 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.h @@ -51,16 +51,6 @@ namespace TestImpact AZStd::optional m_cache = AZStd::nullopt; //!< No caching takes place if cache is empty. }; - namespace Bitwise - { - //! Exception policy for test enumeration cache reads/writes. - enum class CacheExceptionPolicy - { - Never = 0, //! Never throw. - OnCacheWriteFailure = 1 //! Throw when a cache write policy is in place but the cache file could not be written. - }; - } // namespace Bitwise - //! Enumerate a batch of test targets to determine the test suites and fixtures they contain, caching the results where applicable. class TestEnumerator : public TestJobRunner @@ -68,8 +58,6 @@ namespace TestImpact using JobRunner = TestJobRunner; public: - using CacheExceptionPolicy = Bitwise::CacheExceptionPolicy; - //! Constructs a test enumerator with the specified parameters common to all enumeration job runs of this enumerator. //! @param maxConcurrentEnumerations The maximum number of enumerations to be in flight at any given time. explicit TestEnumerator(size_t maxConcurrentEnumerations); @@ -84,8 +72,6 @@ namespace TestImpact //! @return The result of the run sequence and the enumeration jobs with their associated test enumeration payloads. AZStd::pair> Enumerate( const AZStd::vector& jobInfos, - CacheExceptionPolicy cacheExceptionPolicy, - JobExceptionPolicy jobExceptionPolicy, AZStd::optional enumerationTimeout, AZStd::optional enumeratorTimeout, AZStd::optional clientCallback); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobInfoGenerator.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobInfoGenerator.cpp index 7e8e65e940..356688a128 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobInfoGenerator.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobInfoGenerator.cpp @@ -38,7 +38,7 @@ namespace TestImpact { return AZStd::string::format( "%s%s %s", - (m_targetBinaryDir / testTarget->GetOutputName()).c_str(), + (m_targetBinaryDir / RepoPath(testTarget->GetOutputName())).c_str(), GetTestTargetExtension(testTarget).c_str(), testTarget->GetCustomArgs().c_str()).c_str(); } @@ -47,7 +47,7 @@ namespace TestImpact return AZStd::string::format( "\"%s\" \"%s%s\" %s", m_testRunnerBinary.c_str(), - (m_targetBinaryDir / testTarget->GetOutputName()).c_str(), + (m_targetBinaryDir / RepoPath(testTarget->GetOutputName())).c_str(), GetTestTargetExtension(testTarget).c_str(), testTarget->GetCustomArgs().c_str()).c_str(); } @@ -55,22 +55,22 @@ namespace TestImpact RepoPath TestJobInfoGenerator::GenerateTargetEnumerationCacheFilePath(const TestTarget* testTarget) const { - return AZStd::string::format("%s.cache", (m_artifactDir / testTarget->GetName()).c_str()); + return AZStd::string::format("%s.cache", (m_cacheDir / RepoPath(testTarget->GetName())).c_str()); } RepoPath TestJobInfoGenerator::GenerateTargetEnumerationArtifactFilePath(const TestTarget* testTarget) const { - return AZStd::string::format("%s.Enumeration.xml", (m_artifactDir / testTarget->GetName()).c_str()); + return AZStd::string::format("%s.Enumeration.xml", (m_artifactDir / RepoPath(testTarget->GetName())).c_str()); } RepoPath TestJobInfoGenerator::GenerateTargetRunArtifactFilePath(const TestTarget* testTarget) const { - return AZStd::string::format("%s.Run.xml", (m_artifactDir / testTarget->GetName()).c_str()); + return AZStd::string::format("%s.Run.xml", (m_artifactDir / RepoPath(testTarget->GetName())).c_str()); } RepoPath TestJobInfoGenerator::GenerateTargetCoverageArtifactFilePath(const TestTarget* testTarget) const { - return AZStd::string::format("%s.Coverage.xml", (m_artifactDir / testTarget->GetName()).c_str()); + return AZStd::string::format("%s.Coverage.xml", (m_artifactDir / RepoPath(testTarget->GetName())).c_str()); } TestEnumerator::JobInfo TestJobInfoGenerator::GenerateTestEnumerationJobInfo( diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobRunner.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobRunner.h index 3078114ef4..0c1c8189e3 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobRunner.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobRunner.h @@ -12,8 +12,6 @@ #pragma once -#include - #include #include @@ -23,17 +21,6 @@ namespace TestImpact { - namespace Bitwise - { - //! Exception policy for test jobs (and derived jobs). - enum class TestJobExceptionPolicy - { - Never = 0, //!< Never throw. - OnFailedToExecute = 1, //!< Throw when a job fails to execute. - OnExecutedWithFailure = 1 << 1 //!< Throw when a job returns with an error code. - }; - } // namespace Bitwise - //! Base class for test related job runners. //! @tparam AdditionalInfo The data structure containing the information additional to the command arguments necessary to execute and //! complete a job. @@ -47,9 +34,8 @@ namespace TestImpact using Command = typename JobInfo::Command; using JobPayload = Payload; using Job = Job; - using ClientJobCallback = AZStd::function; + using ClientJobCallback = AZStd::function; using DerivedJobCallback = JobCallback; - using JobExceptionPolicy = Bitwise::TestJobExceptionPolicy; using JobDataMap = JobDataMap; //! Constructs the job runner with the specified parameters common to all job runs of this runner. @@ -70,7 +56,6 @@ namespace TestImpact //! @returns The result of the run sequence and the jobs that the sequence produced. AZStd::pair> ExecuteJobs( const AZStd::vector& jobInfos, - JobExceptionPolicy jobExceptionPolicy, PayloadMapProducer payloadMapProducer, StdOutputRouting stdOutRouting, StdErrorRouting stdErrRouting, @@ -95,7 +80,6 @@ namespace TestImpact template AZStd::pair::Job>> TestJobRunner::ExecuteJobs( const AZStd::vector& jobInfos, - JobExceptionPolicy jobExceptionPolicy, PayloadMapProducer payloadMapProducer, StdOutputRouting stdOutRouting, StdErrorRouting stdErrRouting, @@ -105,30 +89,21 @@ namespace TestImpact AZStd::optional derivedJobCallback) { // Callback to handle job exception policies and client/derived callbacks - const auto jobCallback = [&clientCallback, &derivedJobCallback, &jobExceptionPolicy](const JobInfo& jobInfo, const JobMeta& meta, StdContent&& std) + const auto jobCallback = [&clientCallback, &derivedJobCallback](const JobInfo& jobInfo, const JobMeta& meta, StdContent&& std) { auto callbackResult = ProcessCallbackResult::Continue; - if (meta.m_result == JobResult::FailedToExecute && IsFlagSet(jobExceptionPolicy, JobExceptionPolicy::OnFailedToExecute)) - { - callbackResult = ProcessCallbackResult::Abort; - } - else if (meta.m_result == JobResult::ExecutedWithFailure && IsFlagSet(jobExceptionPolicy, JobExceptionPolicy::OnExecutedWithFailure)) - { - callbackResult = ProcessCallbackResult::Abort; - } - if (derivedJobCallback.has_value()) { - if (const auto result = (*derivedJobCallback)(jobInfo, meta, AZStd::move(std)); - result == ProcessCallbackResult::Abort) - { - callbackResult = ProcessCallbackResult::Abort; - } + callbackResult = (*derivedJobCallback)(jobInfo, meta, AZStd::move(std)); } if (clientCallback.has_value()) { - (*clientCallback)(jobInfo, meta); + if (const auto result = (*clientCallback)(jobInfo, meta); + result == ProcessCallbackResult::Abort) + { + callbackResult = ProcessCallbackResult::Abort; + } } return callbackResult; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp index 3b109c3a77..aa8ff04657 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp @@ -10,7 +10,7 @@ * */ -#include +#include #include #include @@ -51,7 +51,6 @@ namespace TestImpact AZStd::pair> InstrumentedTestRunner::RunInstrumentedTests( const AZStd::vector& jobInfos, - JobExceptionPolicy jobExceptionPolicy, AZStd::optional runTimeout, AZStd::optional runnerTimeout, AZStd::optional clientCallback) @@ -84,7 +83,6 @@ namespace TestImpact return ExecuteJobs( jobInfos, - jobExceptionPolicy, payloadGenerator, StdOutputRouting::None, StdErrorRouting::None, diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.h index b2d7f84b6d..2f3e6eb98e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.h @@ -54,7 +54,6 @@ namespace TestImpact //! @return The result of the run sequence and the instrumented run jobs with their associated test run and coverage payloads. AZStd::pair> RunInstrumentedTests( const AZStd::vector& jobInfos, - JobExceptionPolicy jobExceptionPolicy, AZStd::optional runTimeout, AZStd::optional runnerTimeout, AZStd::optional clientCallback); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.cpp index 401084c934..7ac13f85b2 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.cpp @@ -10,7 +10,7 @@ * */ -#include +#include #include #include @@ -28,7 +28,6 @@ namespace TestImpact AZStd::pair> TestRunner::RunTests( const AZStd::vector& jobInfos, - JobExceptionPolicy jobExceptionPolicy, AZStd::optional runTimeout, AZStd::optional runnerTimeout, AZStd::optional clientCallback) @@ -58,7 +57,6 @@ namespace TestImpact return ExecuteJobs( jobInfos, - jobExceptionPolicy, payloadGenerator, StdOutputRouting::None, StdErrorRouting::None, diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.h index 1fdbc16d58..eaa7b6de1d 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.h @@ -39,7 +39,6 @@ namespace TestImpact //! @return The result of the run sequence and the run jobs with their associated test run payloads. AZStd::pair> RunTests( const AZStd::vector& jobInfos, - JobExceptionPolicy jobExceptionPolicy, AZStd::optional runTimeout, AZStd::optional runnerTimeout, AZStd::optional clientCallback); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp index 5046b3f8ee..31c0d7c5de 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp @@ -17,34 +17,14 @@ #include #include #include +#include #include +#include + namespace TestImpact { - // Known error codes for test instrumentation, test runner and unit test library - // This could be refactored into a generic solution agnostic of the tool and library specific details - namespace ErrorCodes - { - namespace OpenCppCoverage - { - static constexpr ReturnCode InvalidArgs = -1618178468; - } - - namespace GTest - { - static constexpr ReturnCode Unsuccessful = 1; - } - - namespace AZTestRunner - { - static constexpr ReturnCode InvalidArgs = 101; - static constexpr ReturnCode FailedToFindTargetBinary = 102; - static constexpr ReturnCode SymbolNotFound = 103; - static constexpr ReturnCode ModuleSkipped = 104; - } - } - namespace { // Calculate the sequence result by analysing the state of the test targets that were run. @@ -101,21 +81,10 @@ namespace TestImpact // Attempt to determine why a given test target executed successfully but return with an error code if (meta.m_returnCode.has_value()) { - switch (meta.m_returnCode.value()) + if (const auto result = CheckForAnyKnownErrorCode(meta.m_returnCode.value()); + result != AZStd::nullopt) { - // We will consider test targets that technically execute but their launcher or unit test library return a know error - // code that pertains to incorrect argument usage as test targets that failed to execute - case ErrorCodes::OpenCppCoverage::InvalidArgs: - case ErrorCodes::AZTestRunner::InvalidArgs: - case ErrorCodes::AZTestRunner::FailedToFindTargetBinary: - case ErrorCodes::AZTestRunner::ModuleSkipped: - case ErrorCodes::AZTestRunner::SymbolNotFound: - return Client::TestRunResult::FailedToExecute; - // The trivial case: the test target has failing tests - case ErrorCodes::GTest::Unsuccessful: - return Client::TestRunResult::TestFailures; - default: - break; + return result.value(); } } @@ -186,14 +155,18 @@ namespace TestImpact TestJobRunnerCallbackHandler( const AZStd::vector& testTargets, TestEngineJobMap* engineJobs, + Policy::ExecutionFailure executionFailurePolicy, + Policy::TestFailure testFailurePolicy, AZStd::optional* callback) : m_testTargets(testTargets) , m_engineJobs(engineJobs) + , m_executionFailurePolicy(executionFailurePolicy) + , m_testFailurePolicy(testFailurePolicy) , m_callback(callback) { } - void operator()(const typename JobInfo& jobInfo, const TestImpact::JobMeta& meta) + [[nodiscard]] ProcessCallbackResult operator()(const typename JobInfo& jobInfo, const TestImpact::JobMeta& meta) { const auto id = jobInfo.GetId().m_value; const auto& args = jobInfo.GetCommand().m_args; @@ -208,11 +181,21 @@ namespace TestImpact { (*m_callback).value()(it->second); } + + if ((result == Client::TestRunResult::FailedToExecute && m_executionFailurePolicy == Policy::ExecutionFailure::Abort) || + (result == Client::TestRunResult::TestFailures && m_testFailurePolicy == Policy::TestFailure::Abort)) + { + return ProcessCallbackResult::Abort; + } + + return ProcessCallbackResult::Continue; } private: const AZStd::vector& m_testTargets; TestEngineJobMap* m_engineJobs; + Policy::ExecutionFailure m_executionFailurePolicy; + Policy::TestFailure m_testFailurePolicy; AZStd::optional* m_callback; }; @@ -250,23 +233,6 @@ namespace TestImpact return engineRuns; } - - Bitwise::TestJobExceptionPolicy GetTestJobExceptionPolicy( - Policy::ExecutionFailure executionFailurePolicy, Policy::TestFailure testFailurePolicy) - { - auto jobExecutionPolicy = Bitwise::TestJobExceptionPolicy::Never; - if (executionFailurePolicy == Policy::ExecutionFailure::Abort) - { - jobExecutionPolicy |= Bitwise::TestJobExceptionPolicy::OnFailedToExecute; - } - - if (testFailurePolicy == Policy::TestFailure::Abort) - { - jobExecutionPolicy |= Bitwise::TestJobExceptionPolicy::OnExecutedWithFailure; - } - - return jobExecutionPolicy; - } } TestEngine::TestEngine( @@ -291,24 +257,19 @@ namespace TestImpact AZStd::pair> TestEngine::UpdateEnumerationCache( const AZStd::vector& testTargets, Policy::ExecutionFailure executionFailurePolicy, + Policy::TestFailure testFailurePolicy, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, AZStd::optional callback) { TestEngineJobMap engineJobs; const auto jobInfos = m_testJobInfoGenerator->GenerateTestEnumerationJobInfos(testTargets, TestEnumerator::JobInfo::CachePolicy::Write); - - const auto jobExecutionPolicy = executionFailurePolicy == Policy::ExecutionFailure::Abort - ? (TestEnumerator::JobExceptionPolicy::OnExecutedWithFailure | TestEnumerator::JobExceptionPolicy::OnFailedToExecute) - : TestEnumerator::JobExceptionPolicy::Never; auto [result, runnerJobs] = m_testEnumerator->Enumerate( jobInfos, - TestEnumerator::CacheExceptionPolicy::OnCacheWriteFailure, - jobExecutionPolicy, testTargetTimeout, globalTimeout, - TestJobRunnerCallbackHandler(testTargets, &engineJobs, &callback)); + TestJobRunnerCallbackHandler(testTargets, &engineJobs, executionFailurePolicy, testFailurePolicy, &callback)); auto engineRuns = CompileTestEngineRuns(testTargets, runnerJobs, AZStd::move(engineJobs)); return { CalculateSequenceResult(result, engineRuns, executionFailurePolicy), AZStd::move(engineRuns) }; @@ -327,10 +288,9 @@ namespace TestImpact TestEngineJobMap engineJobs; const auto jobInfos = m_testJobInfoGenerator->GenerateRegularTestRunJobInfos(testTargets); - TestJobRunnerCallbackHandler jobCallback(testTargets, &engineJobs, &callback); + TestJobRunnerCallbackHandler jobCallback(testTargets, &engineJobs, executionFailurePolicy, testFailurePolicy, &callback); auto [result, runnerJobs] = m_testRunner->RunTests( jobInfos, - GetTestJobExceptionPolicy(executionFailurePolicy, testFailurePolicy), testTargetTimeout, globalTimeout, jobCallback); @@ -355,10 +315,9 @@ namespace TestImpact auto [result, runnerJobs] = m_instrumentedTestRunner->RunInstrumentedTests( jobInfos, - GetTestJobExceptionPolicy(executionFailurePolicy, testFailurePolicy), testTargetTimeout, globalTimeout, - TestJobRunnerCallbackHandler(testTargets, &engineJobs, &callback)); + TestJobRunnerCallbackHandler(testTargets, &engineJobs, executionFailurePolicy, testFailurePolicy, &callback)); auto engineRuns = CompileTestEngineRuns(testTargets, runnerJobs, AZStd::move(engineJobs)); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h index c67a10aa4c..83f57f00ef 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h @@ -70,6 +70,7 @@ namespace TestImpact AZStd::pair> UpdateEnumerationCache( const AZStd::vector& testTargets, Policy::ExecutionFailure executionFailurePolicy, + Policy::TestFailure testFailurePolicy, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, AZStd::optional callback);