Fix instrumented non-gtests treated as failures

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

@@ -28,17 +28,6 @@ namespace TestImpact
         return m_coverageArtifact;
     }
 
-    InstrumentedTestRunner::JobPayload ParseTestRunAndCoverageFiles(
-        const RepoPath& runFile,
-        const RepoPath& coverageFile,
-        AZStd::chrono::milliseconds duration)
-    {
-        TestRun run(GTest::TestRunSuitesFactory(ReadFileContents<TestEngineException>(runFile)), duration);
-        AZStd::vector<ModuleCoverage> moduleCoverages = Cobertura::ModuleCoveragesFactory(ReadFileContents<TestEngineException>(coverageFile));
-        TestCoverage coverage(AZStd::move(moduleCoverages));
-        return {AZStd::move(run), AZStd::move(coverage)};
-    }
-
     InstrumentedTestRunner::InstrumentedTestRunner(size_t maxConcurrentRuns)
         : JobRunner(maxConcurrentRuns)
     {
@@ -58,16 +47,35 @@ namespace TestImpact
                 const auto& [meta, jobInfo] = jobData;
                 if (meta.m_result == JobResult::ExecutedWithSuccess || meta.m_result == JobResult::ExecutedWithFailure)
                 {
+                    const auto printException = [](const Exception& e)
+                    {
+                        AZ_Printf("RunInstrumentedTests", AZStd::string::format("%s\n.", e.what()).c_str());
+                    };
+
+                    AZStd::optional<TestRun> run;
                     try
                     {
-                        runs[jobId] = ParseTestRunAndCoverageFiles(
-                            jobInfo->GetRunArtifactPath(),
-                            jobInfo->GetCoverageArtifactPath(),
+                        run = TestRun(
+                            GTest::TestRunSuitesFactory(ReadFileContents<TestEngineException>(jobInfo->GetRunArtifactPath())),
                             meta.m_duration.value());
                     }
                     catch (const Exception& e)
                     {
-                        AZ_Printf("RunInstrumentedTests", AZStd::string::format("%s\n", e.what()).c_str());
+                        // No run result is not necessarily a failure (e.g. test targets not using gtest)
+                        printException(e);
+                    }
+
+                    try
+                    {
+                        AZStd::vector<ModuleCoverage> moduleCoverages =
+                            Cobertura::ModuleCoveragesFactory(ReadFileContents<TestEngineException>(jobInfo->GetCoverageArtifactPath()));
+                        TestCoverage coverage(AZStd::move(moduleCoverages));
+                        runs[jobId] = { run, AZStd::move(coverage) };
+                    }
+                    catch (const Exception& e)
+                    {
+                        printException(e);
+                        // No coverage, however, is a failure
                         runs[jobId] = AZStd::nullopt;
                     }
                 }

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

@@ -30,9 +30,9 @@ namespace TestImpact
 
     //! Runs a batch of test targets to determine the test coverage and passes/failures.
     class InstrumentedTestRunner
-        : public TestJobRunner<InstrumentedTestRunJobData, AZStd::pair<TestRun, TestCoverage>>
+        : public TestJobRunner<InstrumentedTestRunJobData, AZStd::pair<AZStd::optional<TestRun>, TestCoverage>>
     {
-        using JobRunner = TestJobRunner<InstrumentedTestRunJobData, AZStd::pair<TestRun, TestCoverage>>;
+        using JobRunner = TestJobRunner<InstrumentedTestRunJobData, AZStd::pair<AZStd::optional<TestRun>, TestCoverage>>;
 
     public:
         //! Constructs an instrumented test runner with the specified parameters common to all job runs of this runner.

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

@@ -13,9 +13,9 @@ namespace TestImpact
 {
     namespace
     {
-        AZStd::optional<TestRun> ReleaseTestRun(AZStd::optional<AZStd::pair<TestRun, TestCoverage>>& testRunAndCoverage)
+        AZStd::optional<TestRun> ReleaseTestRun(AZStd::optional<AZStd::pair<AZStd::optional<TestRun>, TestCoverage>>& testRunAndCoverage)
         {
-            if (testRunAndCoverage.has_value())
+            if (testRunAndCoverage.has_value() && testRunAndCoverage.has_value())
             {
                 return AZStd::move(testRunAndCoverage.value().first);
             }
@@ -23,7 +23,8 @@ namespace TestImpact
             return AZStd::nullopt;
         }
 
-        AZStd::optional<TestCoverage> ReleaseTestCoverage(AZStd::optional<AZStd::pair<TestRun, TestCoverage>>& testRunAndCoverage)
+        AZStd::optional<TestCoverage> ReleaseTestCoverage(
+            AZStd::optional<AZStd::pair<AZStd::optional<TestRun>, TestCoverage>>& testRunAndCoverage)
         {
             if (testRunAndCoverage.has_value())
             {
@@ -34,7 +35,8 @@ namespace TestImpact
         }
     }
 
-    TestEngineInstrumentedRun::TestEngineInstrumentedRun(TestEngineJob&& testJob, AZStd::optional<AZStd::pair<TestRun, TestCoverage>>&& testRunAndCoverage)
+    TestEngineInstrumentedRun::TestEngineInstrumentedRun(
+        TestEngineJob&& testJob, AZStd::optional<AZStd::pair<AZStd::optional<TestRun>, TestCoverage>>&& testRunAndCoverage)
         : TestEngineRegularRun(AZStd::move(testJob), ReleaseTestRun(testRunAndCoverage))
         , m_testCoverage(ReleaseTestCoverage(testRunAndCoverage))
     {

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

@@ -17,7 +17,7 @@ namespace TestImpact
         : public TestEngineRegularRun
     {
     public:
-        TestEngineInstrumentedRun(TestEngineJob&& testJob, AZStd::optional<AZStd::pair<TestRun, TestCoverage>>&& testRunAndCoverage);
+        TestEngineInstrumentedRun(TestEngineJob&& testJob, AZStd::optional<AZStd::pair<AZStd::optional<TestRun>, TestCoverage>>&& testRunAndCoverage);
 
         //! Returns the test coverage payload for this job (if any).
         const AZStd::optional<TestCoverage>& GetTestCoverge() const;

Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp

@@ -22,6 +22,8 @@ namespace TestImpact
 {
     namespace
     {
+        static const char* const LogCallSite = "TestImpact";
+
         //! Simple helper class for tracking basic timing information.
         class Timer
         {
@@ -149,7 +151,8 @@ namespace TestImpact
         }
         catch ([[maybe_unused]]const Exception& e)
         {
-            AZ_Printf("TestImpactRuntime",
+            AZ_Printf(
+                LogCallSite,
                 AZStd::string::format(
                     "No test impact analysis data found for suite '%s' at %s\n", GetSuiteTypeName(m_suiteFilter).c_str(), m_sparTIAFile.c_str()).c_str());
         }
@@ -283,8 +286,8 @@ namespace TestImpact
                         job.GetTestCoverge().has_value(),
                         RuntimeException,
                         AZStd::string::format(
-                            "Test target '%s' completed its test run successfully but produced no coverage data",
-                            job.GetTestTarget()->GetName().c_str()));
+                            "Test target '%s' completed its test run successfully but produced no coverage data. Command string: '%s'",
+                            job.GetTestTarget()->GetName().c_str(), job.GetCommandString().c_str()));
                 }
 
                 if (!job.GetTestCoverge().has_value())
@@ -313,7 +316,7 @@ namespace TestImpact
             }
             else
             {
-                AZ_Warning("TestImpact", false, "Ignoring source, source it outside of repo: '%s'", sourcePath.c_str());
+                AZ_Warning(LogCallSite, false, "Ignoring source, source it outside of repo: '%s'", sourcePath.c_str());
             }
         }
 
@@ -322,17 +325,31 @@ namespace TestImpact
 
     void Runtime::UpdateAndSerializeDynamicDependencyMap(const AZStd::vector<TestEngineInstrumentedRun>& jobs)
     {
-        const auto sourceCoverageTestsList = CreateSourceCoveringTestFromTestCoverages(jobs);
-        if (!sourceCoverageTestsList.GetNumSources())
+        try
         {
-            return;
-        }
+            const auto sourceCoverageTestsList = CreateSourceCoveringTestFromTestCoverages(jobs);
+            if (!sourceCoverageTestsList.GetNumSources())
+            {
+                return;
+            }
 
-        m_dynamicDependencyMap->ReplaceSourceCoverage(sourceCoverageTestsList);
-        const auto sparTIA = m_dynamicDependencyMap->ExportSourceCoverage();
-        const auto sparTIAData = SerializeSourceCoveringTestsList(sparTIA);
-        WriteFileContents<RuntimeException>(sparTIAData, m_sparTIAFile);
-        m_hasImpactAnalysisData = true;
+            m_dynamicDependencyMap->ReplaceSourceCoverage(sourceCoverageTestsList);
+            const auto sparTIA = m_dynamicDependencyMap->ExportSourceCoverage();
+            const auto sparTIAData = SerializeSourceCoveringTestsList(sparTIA);
+            WriteFileContents<RuntimeException>(sparTIAData, m_sparTIAFile);
+            m_hasImpactAnalysisData = true;
+        }
+        catch(const RuntimeException& e)
+        {
+            if (m_integrationFailurePolicy == Policy::IntegrityFailure::Abort)
+            {
+                throw e;
+            }
+            else
+            {
+                AZ_Error("TestImpact", false, e.what());
+            }
+        }
     }
 
     TestSequenceResult Runtime::RegularTestSequence(