diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp index 01005718bc..0a4c80afa4 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp @@ -51,7 +51,7 @@ namespace TestImpact Ignore, StdOut, File, - Remove, + Discard, Keep }; @@ -87,7 +87,7 @@ namespace TestImpact "ignore", "stdout", "file", - "remove", + "discard", "keep" }; @@ -147,7 +147,7 @@ namespace TestImpact { const AZStd::vector> states = { - {OptionKeys[Remove], Policy::FailedTestCoverage::Remove}, + {OptionKeys[Discard], Policy::FailedTestCoverage::Discard}, {OptionKeys[Keep], Policy::FailedTestCoverage::Keep} }; @@ -381,71 +381,74 @@ namespace TestImpact AZStd::string help = "usage: tiaf [options]\n" " options:\n" - " -config= Path to the configuration file for the TIAF runtime (default: \n" - " ..json).\n" - " -changelist= Path to the JSON of source file changes to perform test impact \n" - " analysis on.\n" - " -gtimeout= Global timeout value to terminate the entire test sequence should it \n" - " be exceeded.\n" - " -ttimeout= Timeout value to terminate individual test targets should it be \n" - " exceeded.\n" - " -sequence= The type of test sequence to perform, where none runs no tests and\n" - " will report a all tests successful, seed removes any prior coverage \n" - " data and runs all test targets with instrumentation to reseed the \n" - " data from scratch, regular runs all of the test targets without any \n" - " instrumentation to generate coverage data(any prior coverage data is \n" - " left intact), tia uses any prior coverage data to run the instrumented \n" - " subset of selected tests(if no prior coverage data a regular run is \n" - " performed instead) and tiaorseed uses any prior coverage data to run \n" - " the instrumented subset of selected tests(if no prior coverage data a \n" - " seed run is performed instead).\n" - " -safemode= Flag to specify a safe mode sequence where the set of unselected \n" - " tests is run without instrumentation after the set of selected \n" - " instrumented tests is run (this has the effect of ensuring all \n" - " tests are run regardless).\n" - " -shard= Break any test targets with a sharding policy into the number of \n" - " shards according to the maximum concurrency value.\n" - " -cpolicy= Policy for handling the coverage data of failed tests (both tests that \n" - " failed to execute and tests that ran but failed), where remove will \n" - " remove the failed tests from the all coverage data (causing them to be \n" - " drafted into future test runs) and keep will keep any existing coverage \n" - " data and update the coverage data for failed tests that produce coverage.\n" - " -targetout= Capture of individual test run stdout, where stdout will capture \n" - " each individual test target's stdout and output each one to stdout \n" - " and file will capture each individual test target's stdout and output \n" - " each one individually to a file (multiple values are accepted).\n" - " -epolicy= Policy for handling test execution failure (test targets could not be \n" - " launched due to the binary not being built, incorrect paths, etc.), \n" - " where abort will abort the entire test sequence upon the first test\n" - " target execution failure and report a failure(along with the return \n" - " code of the test target that failed to launch), continue will continue \n" - " with the test sequence in the event of test target execution failures\n" - " and treat the test targets that failed to launch as as test failures\n" - " (along with the return codes of the test targets that failed to \n" - " launch), ignore will continue with the test sequence in the event of \n" - " test target execution failures and treat the test targets that failed\n" - " to launch as test passes(along with the return codes of the test \n" - " targets that failed to launch).\n" - " -fpolicy Policy for handling test failures (test targets report failing tests), \n" - " where abort will abort the entire test sequence upon the first test \n" - " failure and report a failure and continue will continue with the test\n" - " sequence in the event of test failures and report the test failures.\n" - " -ipolicy= Policy for handling coverage data integrity failures, where abort will \n" - " abort the test sequenceand report a failure, seed will attempt another \n" - " sequence using the seed sequence type, otherwise will abort and report \n" - " a failure (this option has no effect for regular and seed sequence \n" - " types) and rerun will attempt another sequence using the regular \n" - " sequence type, otherwise will abort and report a failure(this option has \n" - " no effect for regular sequence type).\n" - " -ppolicy= Policy for prioritizing selected test targets, where none will not \n" - " attempt any test target prioritization and locality will attempt to \n" - " prioritize test targets according to the locality of their covering \n" - " production targets in the dependency graph(if no dependency graph data \n" - " available, no prioritization will occur).\n" - " -maxconcurrency= The maximum number of concurrent test targets/shards to be in flight at \n" - " any given moment.\n" - " -ochangelist= Outputs the change list used for test selection.\n" - " -suite= The test suite to select from for this test sequence."; + " -config= Path to the configuration file for the TIAF runtime (default: \n" + " ..json).\n" + " -changelist= Path to the JSON of source file changes to perform test impact \n" + " analysis on.\n" + " -gtimeout= Global timeout value to terminate the entire test sequence should it \n" + " be exceeded.\n" + " -ttimeout= Timeout value to terminate individual test targets should it be \n" + " exceeded.\n" + " -sequence= The type of test sequence to perform, where 'none' runs no tests and\n" + " will report a all tests successful, 'seed' removes any prior coverage \n" + " data and runs all test targets with instrumentation to reseed the \n" + " data from scratch, 'regular' runs all of the test targets without any \n" + " instrumentation to generate coverage data(any prior coverage data is \n" + " left intact), 'tia' uses any prior coverage data to run the instrumented \n" + " subset of selected tests(if no prior coverage data a regular run is \n" + " performed instead), 'tianowrite' uses any prior coverage data to run the \n" + " uninstrumented subset of selected tests (if no prior coverage data a \n" + " regular run is performed instead). The coverage data is not updated with \n" + " the subset of selected tests and 'tiaorseed' uses any prior coverage data \n" + " to run the instrumented subset of selected tests (if no prior coverage \n" + " data a seed run is performed instead).\n" + " -safemode= Flag to specify a safe mode sequence where the set of unselected \n" + " tests is run without instrumentation after the set of selected \n" + " instrumented tests is run (this has the effect of ensuring all \n" + " tests are run regardless).\n" + " -shard= Break any test targets with a sharding policy into the number of \n" + " shards according to the maximum concurrency value.\n" + " -cpolicy= Policy for handling the coverage data of failing tests, where 'discard' \n" + " will discard the coverage data produced by the failing tests, causing \n" + " them to be drafted into future test runs and 'keep' will keep any existing \n" + " coverage data and update the coverage data for failed tests that produce \n" + " coverage.\n" + " -targetout= Capture of individual test run stdout, where 'stdout' will capture \n" + " each individual test target's stdout and output each one to stdout \n" + " and 'file' will capture each individual test target's stdout and output \n" + " each one individually to a file (multiple values are accepted).\n" + " -epolicy= Policy for handling test execution failure (test targets could not be \n" + " launched due to the binary not being built, incorrect paths, etc.), \n" + " where 'abort' will abort the entire test sequence upon the first test\n" + " target execution failure and report a failure(along with the return \n" + " code of the test target that failed to launch), 'continue' will continue \n" + " with the test sequence in the event of test target execution failures\n" + " and treat the test targets that failed to launch as test failures\n" + " (along with the return codes of the test targets that failed to \n" + " launch), 'ignore' will continue with the test sequence in the event of \n" + " test target execution failures and treat the test targets that failed\n" + " to launch as test passes(along with the return codes of the test \n" + " targets that failed to launch).\n" + " -fpolicy Policy for handling test failures (test targets report failing tests), \n" + " where 'abort' will abort the entire test sequence upon the first test \n" + " failure and report a failure and 'continue' will continue with the test\n" + " sequence in the event of test failures and report the test failures.\n" + " -ipolicy= Policy for handling coverage data integrity failures, where 'abort' will \n" + " abort the test sequence and report a failure, 'seed' will attempt another \n" + " sequence using the seed sequence type, otherwise will abort and report \n" + " a failure (this option has no effect for regular and seed sequence \n" + " types) and 'rerun' will attempt another sequence using the regular \n" + " sequence type, otherwise will abort and report a failure(this option has \n" + " no effect for regular sequence type).\n" + " -ppolicy= Policy for prioritizing selected test targets, where 'none' will not \n" + " attempt any test target prioritization and 'locality' will attempt to \n" + " prioritize test targets according to the locality of their covering \n" + " production targets in the dependency graph(if no dependency graph data \n" + " available, no prioritization will occur).\n" + " -maxconcurrency= The maximum number of concurrent test targets/shards to be in flight at \n" + " any given moment.\n" + " -ochangelist= Outputs the change list used for test selection.\n" + " -suite= The test suite to select from for this test sequence."; return help; } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h index 86f2907ab7..485b966a1e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h @@ -81,7 +81,8 @@ namespace TestImpact using SafeTestSequenceCompleteCallback = AZStd::function; + AZStd::chrono::milliseconds selectedDuration, + AZStd::chrono::milliseconds discardedDuration)>; //! Callback for test runs that have completed for any reason. //! @param selectedTests The test that has completed. @@ -201,6 +202,10 @@ namespace TestImpact AZStd::pair, AZStd::vector> SelectTestTargetsByExcludeList( AZStd::vector testTargets) const; + //! Prunes the existing coverage for the specified jobs and creates the consolidated source covering tests list from the + //! test engine instrumented run jobs. + SourceCoveringTestsList CreateSourceCoveringTestFromTestCoverages(const AZStd::vector& jobs); + //! Prepares the dynamic dependency map for a seed update by clearing all existing data and deleting the file that will be serialized. void ClearDynamicDependencyMapAndRemoveExistingFile(); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h index 5f9bff952f..5a5a6196e2 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h @@ -34,7 +34,7 @@ namespace TestImpact //! Policy for handling the coverage data of failed tests targets (both test that failed to execute and tests that ran but failed). enum class FailedTestCoverage { - Remove, //!< Remove the failed test targets from the all coverage data (causing them to be drafted into future test runs). + Discard, //!< Discard the coverage data produced by the failing tests, causing them to be drafted into future test runs. Keep //!< Keep any existing coverage data and update the coverage data for failed test targetss that produce coverage. }; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp index 22bcc97c2a..28b817e32c 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp @@ -133,8 +133,9 @@ namespace TestImpact return buildTarget; } - void DynamicDependencyMap::ReplaceSourceCoverage(const SourceCoveringTestsList& sourceCoverageDelta) + void DynamicDependencyMap::ReplaceSourceCoverageInternal(const SourceCoveringTestsList& sourceCoverageDelta, bool pruneIfNoParentsOrCoverage) { + AZStd::vector killList; for (const auto& sourceCoverage : sourceCoverageDelta.GetCoverage()) { // Autogen input files are not compiled sources and thus supplying coverage data for them makes no sense @@ -146,7 +147,12 @@ namespace TestImpact auto [sourceDependencyIt, inserted] = m_sourceDependencyMap.insert(sourceCoverage.GetPath().String()); auto& [source, sourceDependency] = *sourceDependencyIt; - // Remove the source from the test target covering sources map and clear any existing coverage for the delta + // Before we can replace the coverage for this source dependency, we must: + // 1. Remove the source from the test target covering sources map + // 2. Prune the covered targets for the parent test target(s) of this source dependency + // 3. Clear any existing coverage for the delta + + // 1. for (const auto& testTarget : sourceDependency.m_coveringTestTargets) { if (auto coveringTestTargetIt = m_testTargetSourceCoverage.find(testTarget); @@ -154,7 +160,20 @@ namespace TestImpact { coveringTestTargetIt->second.erase(source); } + + } + + // 2. + // This step is prohibitively expensive as it requires iterating over all of the sources of the build targets covered by + // the parent test targets of this source dependency to ensure that this is in fact the last source being cleared and thus + // it can be determined that the parent test target is no longer covering the given build target + // + // The implications of this are that multiple calls to the test selector and priritizor's SelectTestTargets method will end + // up pulling in more test targets than needed for newly-created production sources until the next time the dynamic dependency + // map reconstructed, however until this use case materializes the implications described will not be addressed + + // 3. sourceDependency.m_coveringTestTargets.clear(); // Update the dependency with any new coverage data @@ -183,13 +202,18 @@ namespace TestImpact } // If the new coverage data results in a parentless and coverageless entry, consider it a dead entry and remove accordingly - if (sourceDependency.m_coveringTestTargets.empty() && sourceDependency.m_parentTargets.empty()) + if (sourceDependency.m_coveringTestTargets.empty() && sourceDependency.m_parentTargets.empty() && pruneIfNoParentsOrCoverage) { m_sourceDependencyMap.erase(sourceDependencyIt); } } } + void DynamicDependencyMap::ReplaceSourceCoverage(const SourceCoveringTestsList& sourceCoverageDelta) + { + ReplaceSourceCoverageInternal(sourceCoverageDelta, true); + } + void DynamicDependencyMap::ClearSourceCoverage(const AZStd::vector& paths) { for (const auto& path : paths) @@ -212,9 +236,14 @@ namespace TestImpact void DynamicDependencyMap::ClearAllSourceCoverage() { - for (const auto& [path, coverage] : m_sourceDependencyMap) + for (auto it = m_sourceDependencyMap.begin(); it != m_sourceDependencyMap.end(); ++it) { - ReplaceSourceCoverage(SourceCoveringTestsList(AZStd::vector{ SourceCoveringTests(RepoPath(path)) })); + const auto& [path, coverage] = *it; + ReplaceSourceCoverageInternal(SourceCoveringTestsList(AZStd::vector{ SourceCoveringTests(RepoPath(path)) }), false); + if (coverage.m_coveringTestTargets.empty() && coverage.m_parentTargets.empty()) + { + it = m_sourceDependencyMap.erase(it); + } } } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h index cd01a1a728..701f69e68d 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h @@ -81,7 +81,6 @@ namespace TestImpact SourceDependency GetSourceDependencyOrThrow(const RepoPath& path) const; //! Replaces the source coverage of the specified sources with the specified source coverage. - //! @note The covering targets for the parent test target(s) will not be pruned if those covering targets are removed. //! @param sourceCoverageDelta The source coverage delta to replace in the dependency map. void ReplaceSourceCoverage(const SourceCoveringTestsList& sourceCoverageDelta); @@ -110,6 +109,13 @@ namespace TestImpact AZStd::vector GetNotCoveringTests() const; private: + //! Internal handler for ReplaceSourceCoverage where the pruning of parentless and coverageless source depenencies after the + //! source coverage has been replaced must be explicitly stated. + //! @note The covered targets for the source dependency's parent test target(s) will not be pruned if those covering targets are removed. + //! @param sourceCoverageDelta The source coverage delta to replace in the dependency map. + //! @param pruneIfNoParentsOrCoverage Flag to specify whether or not newly parentless and coverageless dependencies will be removed. + void ReplaceSourceCoverageInternal(const SourceCoveringTestsList& sourceCoverageDelta, bool pruneIfNoParentsOrCoverage); + //! Clears the source coverage of the specified sources. //! @note The covering targets for the parent test target(s) will not be pruned if those covering targets are removed. void ClearSourceCoverage(const AZStd::vector& paths); @@ -127,6 +133,7 @@ namespace TestImpact AZStd::unordered_map> m_testTargetSourceCoverage; //! The map of build targets and their covering test targets. + //! @note As per the note for ReplaceSourceCoverageInternal, this map is currently not pruned when source coverage is replaced. AZStd::unordered_map> m_buildTargetCoverage; //! Mapping of autogen input sources to their generated output sources. 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 58ffb85cb3..17784dd937 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp @@ -73,7 +73,7 @@ namespace TestImpact } catch (const TestEngineException& e) { - AZ_Printf("Enumerate", "Enumeration cache error: %s", e.what()); + AZ_Printf("Enumerate", AZStd::string::format("Enumeration cache error: %s\n", e.what()).c_str()); DeleteFile(jobInfo->GetCache()->m_file); } 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 aa8ff04657..5a0ef74472 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp @@ -72,7 +72,7 @@ namespace TestImpact } catch (const Exception& e) { - AZ_Printf("RunInstrumentedTests", e.what()); + AZ_Printf("RunInstrumentedTests", AZStd::string::format("%s\n", e.what()).c_str()); runs[jobId] = AZStd::nullopt; } } 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 7ac13f85b2..16717a1fe5 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/Run/TestImpactTestRunner.cpp @@ -46,7 +46,7 @@ namespace TestImpact } catch (const Exception& e) { - AZ_Printf("RunTests", e.what()); + AZ_Printf("RunTests", AZStd::string::format("%s\n", e.what()).c_str()); runs[jobId] = AZStd::nullopt; } } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp index d83976b488..784a46ffa0 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -256,32 +256,84 @@ namespace TestImpact void Runtime::ClearDynamicDependencyMapAndRemoveExistingFile() { - DeleteFile(m_sparTIAFile); m_dynamicDependencyMap->ClearAllSourceCoverage(); + DeleteFile(m_sparTIAFile); + } + + SourceCoveringTestsList Runtime::CreateSourceCoveringTestFromTestCoverages(const AZStd::vector& jobs) + { + AZStd::unordered_map> coverage; + for (const auto& job : jobs) + { + // First we must remove any existing coverage for the test target so as to not end up with source remnants from previous + // coverage that is no longer covered by this revision of the test target + m_dynamicDependencyMap->RemoveTestTargetFromSourceCoverage(job.GetTestTarget()); + + // Next we will update the coverage of test targets that completed (with or without failures), unless the failed test coverage + // policy dictates we should instead discard the coverage of test targets with failing tests + const auto testResult = job.GetTestResult(); + + if (m_failedTestCoveragePolicy == Policy::FailedTestCoverage::Discard && testResult == Client::TestRunResult::TestFailures) + { + // Discard the coverage for this job + continue; + } + + if (testResult == Client::TestRunResult::AllTestsPass || testResult == Client::TestRunResult::TestFailures) + { + if (testResult == Client::TestRunResult::AllTestsPass) + { + // Passing tests should have coverage data, otherwise something is very wrong + AZ_TestImpact_Eval( + 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())); + } + + if (!job.GetTestCoverge().has_value()) + { + // When a test run completes with failing tests but produces no coverage artifact that's typically a sign of the + // test aborting due to an unhandled exception, in which case ignore it and let it be picked up in the failure report + continue; + } + + for (const auto& source : job.GetTestCoverge().value().GetSourcesCovered()) + { + coverage[source.String()].insert(job.GetTestTarget()->GetName()); + } + } + } + + AZStd::vector sourceCoveringTests; + sourceCoveringTests.reserve(coverage.size()); + for (auto&& [source, testTargets] : coverage) + { + if (const auto sourcePath = RepoPath(source); + sourcePath.IsRelativeTo(m_config.m_repo.m_root)) + { + sourceCoveringTests.push_back( + SourceCoveringTests(RepoPath(sourcePath.LexicallyRelative(m_config.m_repo.m_root)), AZStd::move(testTargets))); + } + else + { + AZ_Warning("TestImpact", false, "Ignoring source, source it outside of repo: '%s'", sourcePath.c_str()); + } + } + + return SourceCoveringTestsList(AZStd::move(sourceCoveringTests)); } void Runtime::UpdateAndSerializeDynamicDependencyMap(const AZStd::vector& jobs) { - const auto sourceCoverageTestsList = CreateSourceCoveringTestFromTestCoverages(jobs, m_config.m_repo.m_root); + const auto sourceCoverageTestsList = CreateSourceCoveringTestFromTestCoverages(jobs); if (!sourceCoverageTestsList.GetNumSources()) { return; } m_dynamicDependencyMap->ReplaceSourceCoverage(sourceCoverageTestsList); - - if (m_failedTestCoveragePolicy == Policy::FailedTestCoverage::Remove) - { - for (const auto& job : jobs) - { - if (job.GetTestResult() != Client::TestRunResult::AllTestsPass || - !job.GetTestCoverge().has_value()) - { - m_dynamicDependencyMap->RemoveTestTargetFromSourceCoverage(job.GetTestTarget()); - } - } - } - const auto sparTIA = m_dynamicDependencyMap->ExportSourceCoverage(); const auto sparTIAData = SerializeSourceCoveringTestsList(sparTIA); WriteFileContents(sparTIAData, m_sparTIAFile); @@ -422,7 +474,7 @@ namespace TestImpact AZStd::optional testSequenceEndCallback, AZStd::optional testCompleteCallback) { - Timer timer; + Timer timer; // Draft in the test targets that have no coverage entries in the dynamic dependency map AZStd::vector draftedTestTargets = m_dynamicDependencyMap->GetNotCoveringTests(); @@ -459,6 +511,8 @@ namespace TestImpact testTargetTimeout, globalTimeout, TestRunCompleteCallbackHandler(testCompleteCallback)); + + const auto selectedDuraton = timer.Elapsed(); // Carry the remaining global sequence time over to the discarded test run if (globalTimeout.has_value()) @@ -478,16 +532,18 @@ namespace TestImpact globalTimeout, TestRunCompleteCallbackHandler(testCompleteCallback)); - UpdateAndSerializeDynamicDependencyMap(selectedTestJobs); + const auto discardedDuraton = timer.Elapsed(); if (testSequenceEndCallback.has_value()) { (*testSequenceEndCallback)( GenerateSequenceFailureReport(selectedTestJobs), GenerateSequenceFailureReport(discardedTestJobs), - timer.Elapsed()); + selectedDuraton, + discardedDuraton); } + UpdateAndSerializeDynamicDependencyMap(selectedTestJobs); return { selectedResult, discardedResult }; } @@ -530,14 +586,14 @@ namespace TestImpact globalTimeout, TestRunCompleteCallbackHandler(testCompleteCallback)); - ClearDynamicDependencyMapAndRemoveExistingFile(); - UpdateAndSerializeDynamicDependencyMap(testJobs); - if (testSequenceEndCallback.has_value()) { (*testSequenceEndCallback)(GenerateSequenceFailureReport(testJobs), timer.Elapsed()); } + ClearDynamicDependencyMapAndRemoveExistingFile(); + UpdateAndSerializeDynamicDependencyMap(testJobs); + return result; } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp index 47c0e83233..68482ab721 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp @@ -82,54 +82,4 @@ namespace TestImpact return testNames; } - - SourceCoveringTestsList CreateSourceCoveringTestFromTestCoverages(const AZStd::vector& jobs, const RepoPath& root) - { - AZStd::unordered_map> coverage; - for (const auto& job : jobs) - { - if (const auto testResult = job.GetTestResult(); - testResult == Client::TestRunResult::AllTestsPass || testResult == Client::TestRunResult::TestFailures) - { - if (testResult == Client::TestRunResult::AllTestsPass) - { - // Passing tests should have coverage data, otherwise something is very wrong - AZ_TestImpact_Eval( - 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())); - } - else if (!job.GetTestCoverge().has_value()) - { - // When a test run completes with failing tests but produces no coverage artifact that's typically a sign of the - // test aborting due to an unhandled exception, in which case ignore it and let it be picked up in the failure report - continue; - } - - for (const auto& source : job.GetTestCoverge().value().GetSourcesCovered()) - { - coverage[source.String()].insert(job.GetTestTarget()->GetName()); - } - } - } - - AZStd::vector sourceCoveringTests; - sourceCoveringTests.reserve(coverage.size()); - for (auto&& [source, testTargets] : coverage) - { - if (const auto sourcePath = RepoPath(source); - sourcePath.IsRelativeTo(root)) - { - sourceCoveringTests.push_back(SourceCoveringTests(RepoPath(sourcePath.LexicallyRelative(root)), AZStd::move(testTargets))); - } - else - { - AZ_Warning("TestImpact", false, "Ignoring source, source it outside of repo: %s", sourcePath.c_str()); - } - } - - return SourceCoveringTestsList(AZStd::move(sourceCoveringTests)); - } -} +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h index 943b0cbec4..807911d854 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h @@ -40,12 +40,7 @@ namespace TestImpact const AZStd::vector& excludedTestTargets); //! Extracts the name information from the specified test targets. - AZStd::vector ExtractTestTargetNames(const AZStd::vector testTargets); - - //! Creates the consolidates source covering tests list from the test engine instrumented run jobs. - SourceCoveringTestsList CreateSourceCoveringTestFromTestCoverages( - const AZStd::vector& jobs, - const RepoPath& root); + AZStd::vector ExtractTestTargetNames(const AZStd::vector testTargets); //! Generates a test run failure report from the specified test engine job information. //! @tparam TestJob The test engine job type.