Remove JobExceptionPolicy and move knoen error handling

Code/Tools/TestImpactFramework/Runtime/Code/Source/Platform/Windows/TestEngine/JobRunner/TestImpactWin32_TestTargetExtension.cpp

@@ -41,4 +41,4 @@ namespace TestImpact
         }
         }
     }
-}
+} // namespace TestImpact

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp

@@ -10,7 +10,7 @@
  *
  */
 
-#include <TestImpactFramework/TestImpactUtils.h>
+#include <TestImpactFramework/TestImpactFileUtils.h>
 
 #include <Artifact/Factory/TestImpactTestEnumerationSuiteFactory.h>
 #include <TestEngine/TestImpactTestEngineException.h>
@@ -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<char> 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<TestEngineException>(enumerationFile)));
@@ -79,8 +50,6 @@ namespace TestImpact
     
     AZStd::pair<ProcessSchedulerResult, AZStd::vector<TestEnumerator::Job>> TestEnumerator::Enumerate(
         const AZStd::vector<JobInfo>& jobInfos,
-        CacheExceptionPolicy cacheExceptionPolicy,
-        JobExceptionPolicy jobExceptionPolicy,
         AZStd::optional<AZStd::chrono::milliseconds> enumerationTimeout,
         AZStd::optional<AZStd::chrono::milliseconds> enumeratorTimeout,
         AZStd::optional<ClientJobCallback> clientCallback)
@@ -88,50 +57,67 @@ namespace TestImpact
         AZStd::vector<Job> cachedJobs;
         AZStd::vector<JobInfo> 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<TestEnumeration> enumeration;
+                if (jobInfo->GetCache()->m_policy == JobData::CachePolicy::Read)
+                {
+                    JobMeta meta;
+                    AZStd::optional<TestEnumeration> enumeration;
 
-                try
-                {
-                    enumeration = TestEnumeration(DeserializeTestEnumeration(ReadFileContents<TestEngineException>(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<TestEngineException>(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<Job> 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<Job> 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<TestEngineException>(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,

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.h

@@ -51,16 +51,6 @@ namespace TestImpact
         AZStd::optional<Cache> 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<TestEnumerationJobData, TestEnumeration>
@@ -68,8 +58,6 @@ namespace TestImpact
         using JobRunner = TestJobRunner<TestEnumerationJobData, TestEnumeration>;
 
     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<ProcessSchedulerResult, AZStd::vector<Job>> Enumerate(
             const AZStd::vector<JobInfo>& jobInfos,
-            CacheExceptionPolicy cacheExceptionPolicy,
-            JobExceptionPolicy jobExceptionPolicy,
             AZStd::optional<AZStd::chrono::milliseconds> enumerationTimeout,
             AZStd::optional<AZStd::chrono::milliseconds> enumeratorTimeout,
             AZStd::optional<ClientJobCallback> clientCallback);

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(

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/JobRunner/TestImpactTestJobRunner.h

@@ -12,8 +12,6 @@
 
 #pragma once
 
-#include <TestImpactBitwise.h>
-
 #include <Process/JobRunner/TestImpactProcessJob.h>
 #include <Process/JobRunner/TestImpactProcessJobRunner.h>
 
@@ -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<JobInfo, Payload>;
-        using ClientJobCallback = AZStd::function<void(const JobInfo& jobInfo, const JobMeta& meta)>;
+        using ClientJobCallback = AZStd::function<ProcessCallbackResult(const JobInfo& jobInfo, const JobMeta& meta)>;
         using DerivedJobCallback = JobCallback<Job>;
-        using JobExceptionPolicy = Bitwise::TestJobExceptionPolicy;
         using JobDataMap = JobDataMap<Job>;
 
         //! 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<ProcessSchedulerResult, AZStd::vector<Job>> ExecuteJobs(
             const AZStd::vector<JobInfo>& jobInfos,
-            JobExceptionPolicy jobExceptionPolicy,
             PayloadMapProducer<Job> payloadMapProducer,
             StdOutputRouting stdOutRouting,
             StdErrorRouting stdErrRouting,
@@ -95,7 +80,6 @@ namespace TestImpact
     template<typename Data, typename Payload>
     AZStd::pair<ProcessSchedulerResult, AZStd::vector<typename TestJobRunner<Data, Payload>::Job>> TestJobRunner<Data, Payload>::ExecuteJobs(
         const AZStd::vector<JobInfo>& jobInfos,
-        JobExceptionPolicy jobExceptionPolicy,
         PayloadMapProducer<Job> payloadMapProducer,
         StdOutputRouting stdOutRouting,
         StdErrorRouting stdErrRouting,
@@ -105,30 +89,21 @@ namespace TestImpact
         AZStd::optional<DerivedJobCallback> 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;

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp

@@ -10,7 +10,7 @@
  *
  */
 
-#include <TestImpactFramework/TestImpactUtils.h>
+#include <TestImpactFramework/TestImpactFileUtils.h>
 
 #include <Artifact/Factory/TestImpactModuleCoverageFactory.h>
 #include <Artifact/Factory/TestImpactTestRunSuiteFactory.h>
@@ -51,7 +51,6 @@ namespace TestImpact
 
     AZStd::pair<ProcessSchedulerResult, AZStd::vector<InstrumentedTestRunner::Job>> InstrumentedTestRunner::RunInstrumentedTests(
         const AZStd::vector<JobInfo>& jobInfos,
-        JobExceptionPolicy jobExceptionPolicy,
         AZStd::optional<AZStd::chrono::milliseconds> runTimeout,
         AZStd::optional<AZStd::chrono::milliseconds> runnerTimeout,
         AZStd::optional<ClientJobCallback> clientCallback)
@@ -84,7 +83,6 @@ namespace TestImpact
 
         return ExecuteJobs(
             jobInfos,
-            jobExceptionPolicy,
             payloadGenerator,
             StdOutputRouting::None,
             StdErrorRouting::None,

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<ProcessSchedulerResult, AZStd::vector<Job>> RunInstrumentedTests(
             const AZStd::vector<JobInfo>& jobInfos,
-            JobExceptionPolicy jobExceptionPolicy,
             AZStd::optional<AZStd::chrono::milliseconds> runTimeout,
             AZStd::optional<AZStd::chrono::milliseconds> runnerTimeout,
             AZStd::optional<ClientJobCallback> clientCallback);

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.cpp

@@ -10,7 +10,7 @@
  *
  */
 
-#include <TestImpactFramework/TestImpactUtils.h>
+#include <TestImpactFramework/TestImpactFileUtils.h>
 
 #include <Artifact/Factory/TestImpactTestRunSuiteFactory.h>
 #include <TestEngine/TestImpactTestEngineException.h>
@@ -28,7 +28,6 @@ namespace TestImpact
 
     AZStd::pair<ProcessSchedulerResult, AZStd::vector<TestRunner::Job>> TestRunner::RunTests(
         const AZStd::vector<JobInfo>& jobInfos,
-        JobExceptionPolicy jobExceptionPolicy,
         AZStd::optional<AZStd::chrono::milliseconds> runTimeout,
         AZStd::optional<AZStd::chrono::milliseconds> runnerTimeout,
         AZStd::optional<ClientJobCallback> clientCallback)
@@ -58,7 +57,6 @@ namespace TestImpact
 
         return ExecuteJobs(
             jobInfos,
-            jobExceptionPolicy,
             payloadGenerator,
             StdOutputRouting::None,
             StdErrorRouting::None,

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<ProcessSchedulerResult, AZStd::vector<Job>> RunTests(
             const AZStd::vector<JobInfo>& jobInfos,
-            JobExceptionPolicy jobExceptionPolicy,
             AZStd::optional<AZStd::chrono::milliseconds> runTimeout,
             AZStd::optional<AZStd::chrono::milliseconds> runnerTimeout,
             AZStd::optional<ClientJobCallback> clientCallback);

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp

@@ -17,34 +17,14 @@
 #include <TestEngine/Run/TestImpactInstrumentedTestRunner.h>
 #include <TestEngine/Run/TestImpactTestRunner.h>
 #include <TestEngine/JobRunner/TestImpactTestJobInfoGenerator.h>
+#include <TestEngine/TestImpactTestEngineJobFailure.h>
 
 #include <AzCore/std/containers/unordered_map.h>
 
+#include <iostream>
+
 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<const TestTarget*>& testTargets,
                 TestEngineJobMap<IdType>* engineJobs,
+                Policy::ExecutionFailure executionFailurePolicy,
+                Policy::TestFailure testFailurePolicy,
                 AZStd::optional<TestEngineJobCompleteCallback>* 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<const TestTarget*>& m_testTargets;
             TestEngineJobMap<typename IdType>* m_engineJobs;
+            Policy::ExecutionFailure m_executionFailurePolicy;
+            Policy::TestFailure m_testFailurePolicy;
             AZStd::optional<TestEngineJobCompleteCallback>* 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<TestSequenceResult, AZStd::vector<TestEngineEnumeration>> TestEngine::UpdateEnumerationCache(
         const AZStd::vector<const TestTarget*>& testTargets,
         Policy::ExecutionFailure executionFailurePolicy,
+        Policy::TestFailure testFailurePolicy,
         AZStd::optional<AZStd::chrono::milliseconds> testTargetTimeout,
         AZStd::optional<AZStd::chrono::milliseconds> globalTimeout,
         AZStd::optional<TestEngineJobCompleteCallback> callback)
     {
         TestEngineJobMap<TestEnumerator::JobInfo::IdType> 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<TestEnumerator>(testTargets, &engineJobs, &callback));
+            TestJobRunnerCallbackHandler<TestEnumerator>(testTargets, &engineJobs, executionFailurePolicy, testFailurePolicy, &callback));
 
         auto engineRuns = CompileTestEngineRuns<TestEnumerator>(testTargets, runnerJobs, AZStd::move(engineJobs));
         return { CalculateSequenceResult(result, engineRuns, executionFailurePolicy), AZStd::move(engineRuns) };
@@ -327,10 +288,9 @@ namespace TestImpact
         TestEngineJobMap<TestRunner::JobInfo::IdType> engineJobs;
         const auto jobInfos = m_testJobInfoGenerator->GenerateRegularTestRunJobInfos(testTargets);
 
-        TestJobRunnerCallbackHandler<TestRunner> jobCallback(testTargets, &engineJobs, &callback);
+        TestJobRunnerCallbackHandler<TestRunner> 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<InstrumentedTestRunner>(testTargets, &engineJobs, &callback));
+            TestJobRunnerCallbackHandler<InstrumentedTestRunner>(testTargets, &engineJobs, executionFailurePolicy, testFailurePolicy, &callback));
 
         auto engineRuns = CompileTestEngineRuns<InstrumentedTestRunner>(testTargets, runnerJobs, AZStd::move(engineJobs));
 

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h

@@ -70,6 +70,7 @@ namespace TestImpact
         AZStd::pair<TestSequenceResult, AZStd::vector<TestEngineEnumeration>> UpdateEnumerationCache(
             const AZStd::vector<const TestTarget*>& testTargets,
             Policy::ExecutionFailure executionFailurePolicy,
+            Policy::TestFailure testFailurePolicy,
             AZStd::optional<AZStd::chrono::milliseconds> testTargetTimeout,
             AZStd::optional<AZStd::chrono::milliseconds> globalTimeout,
             AZStd::optional<TestEngineJobCompleteCallback> callback);