From ac42a9a748fdb889136e5da378273711caa51f4a Mon Sep 17 00:00:00 2001 From: jonawals Date: Fri, 4 Jun 2021 19:33:26 +0100 Subject: [PATCH 01/14] Changes for read-only runs and new test suites --- .../Source/TestImpactCommandLineOptions.cpp | 55 +++----- .../Source/TestImpactCommandLineOptions.h | 9 +- .../Code/Source/TestImpactConsoleMain.cpp | 83 ++++++++---- .../TestImpactRuntimeConfigurationFactory.cpp | 21 ++- .../TestImpactConfiguration.h | 3 +- .../TestImpactFramework/TestImpactRuntime.h | 10 +- .../TestImpactTestSequence.h | 35 +++++ .../TestImpactTestTargetMetaMapFactory.cpp | 49 ++++--- .../TestImpactTestTargetMetaMapFactory.h | 6 +- .../TestImpactDynamicDependencyMap.cpp | 6 +- .../Runtime/Code/Source/TestImpactRuntime.cpp | 127 ++++++++++-------- .../Code/Source/TestImpactRuntimeUtils.cpp | 7 +- .../Code/Source/TestImpactRuntimeUtils.h | 1 + .../testimpactframework_runtime_files.cmake | 92 ++++++++----- ...timpactframework_runtime_tests_files.cmake | 38 ------ cmake/LYTestWrappers.cmake | 50 +++++-- .../ConsoleFrontendConfig.in | 23 +++- .../LYTestImpactFramework.cmake | 115 ++++++++++------ 18 files changed, 449 insertions(+), 281 deletions(-) delete mode 100644 Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake 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 a6eb2e886e..5ff070601a 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp @@ -36,21 +36,21 @@ namespace TestImpact MaxConcurrency, TestTargetTimeout, GlobalTimeout, - SuitesFilter, + SuiteFilter, SafeMode, // Values None, Seed, Regular, ImpactAnalysis, + ImpactAnalysisNoWrite, ImpactAnalysisOrSeed, Locality, Abort, Continue, Ignore, StdOut, - File, - AllSuites + File }; constexpr const char* OptionKeys[] = @@ -70,21 +70,21 @@ namespace TestImpact "maxconcurrency", "ttimeout", "gtimeout", - "suites", + "suite", "safemode", // Values "none", "seed", "regular", "tia", + "tianowrite", "tiaorseed", "locality", "abort", "continue", "ignore", "stdout", - "file", - "*" + "file" }; RepoPath ParseConfigurationFile(const AZ::CommandLine& cmd) @@ -110,6 +110,7 @@ namespace TestImpact {OptionKeys[Seed], TestSequenceType::Seed}, {OptionKeys[Regular], TestSequenceType::Regular}, {OptionKeys[ImpactAnalysis], TestSequenceType::ImpactAnalysis}, + {OptionKeys[ImpactAnalysisNoWrite], TestSequenceType::ImpactAnalysisNoWrite}, {OptionKeys[ImpactAnalysisOrSeed], TestSequenceType::ImpactAnalysisOrSeed} }; @@ -250,32 +251,16 @@ namespace TestImpact return ParseOnOffOption(OptionKeys[SafeMode], states, cmd).value_or(false); } - AZStd::unordered_set ParseSuitesFilter(const AZ::CommandLine& cmd) + SuiteType ParseSuiteFilter(const AZ::CommandLine& cmd) { - AZStd::unordered_set suitesFilter; - if (const auto numSwitchValues = cmd.GetNumSwitchValues(OptionKeys[SuitesFilter]); - numSwitchValues) + const AZStd::vector> states = { - for (auto i = 0; i < numSwitchValues; i++) - { - const auto value = cmd.GetSwitchValue(OptionKeys[SuitesFilter], i); - AZ_TestImpact_Eval(!value.empty(), CommandLineOptionsException, "Suites option value is empty"); - if (value == OptionKeys[AllSuites]) - { - AZ_TestImpact_Eval( - suitesFilter.empty(), CommandLineOptionsException, "The * suite cannot be used with other suites"); - } + {GetSuiteTypeName(SuiteType::Main), SuiteType::Main}, + {GetSuiteTypeName(SuiteType::Periodic), SuiteType::Periodic}, + {GetSuiteTypeName(SuiteType::Sandbox), SuiteType::Sandbox} + }; - suitesFilter.insert(value); - } - } - - if (suitesFilter.find(OptionKeys[AllSuites]) != suitesFilter.end()) - { - return {}; - } - - return suitesFilter; + return ParseMultiStateOption(OptionKeys[SuiteFilter], states, cmd).value_or(SuiteType::Main); } } @@ -299,7 +284,7 @@ namespace TestImpact m_testTargetTimeout = ParseTestTargetTimeout(cmd); m_globalTimeout = ParseGlobalTimeout(cmd); m_safeMode = ParseSafeMode(cmd); - m_suitesFilter = ParseSuitesFilter(cmd); + m_suiteFilter = ParseSuiteFilter(cmd); } bool CommandLineOptions::HasChangeListFile() const @@ -382,9 +367,9 @@ namespace TestImpact return m_globalTimeout; } - const AZStd::unordered_set& CommandLineOptions::GetSuitesFilter() const + SuiteType CommandLineOptions::GetSuiteFilter() const { - return m_suitesFilter; + return m_suiteFilter; } AZStd::string CommandLineOptions::GetCommandLineUsageString() @@ -452,11 +437,7 @@ namespace TestImpact " -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" - " -suites= The test suites to select from for this test sequence (multiple values are \n" - " allowed). The suite all has special significance and will allow tests from \n" - " any suite to be selected, however this particular suite is mutually exclusive\n" - " with other suite. Note: this option is only applicable to the regular sequence\n" - " and, if safe mode is enables, the tia and tiaorseed sequences."; + " -suite= The test suite to select from for this test sequence."; return help; } diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h index 73961b5fcc..99b1b98dba 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h @@ -18,7 +18,6 @@ #include #include #include -#include namespace TestImpact { @@ -29,6 +28,8 @@ namespace TestImpact Seed, //!< Removes any prior coverage data and runs all test targets with instrumentation to reseed the data from scratch. Regular, //!< Runs all of the test targets without any instrumentation to generate coverage data (any prior coverage data is left intact). ImpactAnalysis, //!< Uses any prior coverage data to run the instrumented subset of selected tests (if no prior coverage data a regular run is performed instead). + ImpactAnalysisNoWrite, //!< Uses any prior coverage data to run the uninstrumented subset of selected tests (if no prior coverage data a regular run is performed instead). + //!< The coverage data is not updated with the subset of selected tests. ImpactAnalysisOrSeed //!< Uses any prior coverage data to run the instrumented subset of selected tests (if no prior coverage data a seed run is performed instead). }; @@ -87,8 +88,8 @@ namespace TestImpact //! Returns the global test sequence timeout to use (if any). const AZStd::optional& GetGlobalTimeout() const; - //! Returns the filter for test suites that will be allowed to be run. - const AZStd::unordered_set& GetSuitesFilter() const; + //! Returns the filter for test suite that will be allowed to be run. + SuiteType GetSuiteFilter() const; private: RepoPath m_configurationFile; @@ -105,7 +106,7 @@ namespace TestImpact AZStd::optional m_maxConcurrency; AZStd::optional m_testTargetTimeout; AZStd::optional m_globalTimeout; - AZStd::unordered_set m_suitesFilter; + SuiteType m_suiteFilter; bool m_safeMode = false; }; } // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp index 84e16c2332..fab6509244 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp @@ -86,6 +86,8 @@ namespace TestImpact Runtime& runtime, const AZStd::optional& changeList) { + // Even though it is possible for a regular run to be selected (see below) which does not actually require a change list, + // consider any impact analysis sequence type without a change list to be an error AZ_TestImpact_Eval( changeList.has_value(), CommandLineOptionsException, @@ -94,39 +96,72 @@ namespace TestImpact TestSequenceResult result = TestSequenceResult::Failure; if (options.HasSafeMode()) { - auto [selectedResult, discardedResult] = runtime.SafeImpactAnalysisTestSequence( - changeList.value(), - options.GetSuitesFilter(), - options.GetTestPrioritizationPolicy(), - options.GetTestTargetTimeout(), - options.GetGlobalTimeout(), - AZStd::ref(sequenceEventHandler), - AZStd::ref(sequenceEventHandler), - AZStd::ref(sequenceEventHandler)); + if (options.GetTestSequenceType() == TestSequenceType::ImpactAnalysis) + { + auto [selectedResult, discardedResult] = runtime.SafeImpactAnalysisTestSequence( + changeList.value(), + options.GetTestPrioritizationPolicy(), + options.GetTestTargetTimeout(), + options.GetGlobalTimeout(), + AZStd::ref(sequenceEventHandler), + AZStd::ref(sequenceEventHandler), + AZStd::ref(sequenceEventHandler)); - // Handling the possible timeout and failure permutations of the selected and discarded test results is splitting hairs - // so apply the following, admittedly arbitrary, rules to determine what the composite test sequence result should be - if (selectedResult == TestSequenceResult::Success && discardedResult == TestSequenceResult::Success) - { - // Trivial case: both sequences succeeded - result = TestSequenceResult::Success; + // Handling the possible timeout and failure permutations of the selected and discarded test results is splitting hairs + // so apply the following, admittedly arbitrary, rules to determine what the composite test sequence result should be + if (selectedResult == TestSequenceResult::Success && discardedResult == TestSequenceResult::Success) + { + // Trivial case: both sequences succeeded + result = TestSequenceResult::Success; + } + else if (selectedResult == TestSequenceResult::Failure || discardedResult == TestSequenceResult::Failure) + { + // One sequence failed whilst the other sequence either succeeded or timed out + result = TestSequenceResult::Failure; + } + else + { + // One sequence timed out whilst the other sequence succeeded or both sequences timed out + result = TestSequenceResult::Timeout; + } } - else if (selectedResult == TestSequenceResult::Failure || discardedResult == TestSequenceResult::Failure) + else if (options.GetTestSequenceType() == TestSequenceType::ImpactAnalysisNoWrite) { - // One sequence failed whilst the other sequence either succeeded or timed out - result = TestSequenceResult::Failure; + // A no-write impact analysis sequence with safe mode enabled is functionally identical to a regular sequence type + // due to a) the selected tests being run without instrumentation and b) the discarded tests also being run without + // instrumentation + result = runtime.RegularTestSequence( + options.GetTestTargetTimeout(), + options.GetGlobalTimeout(), + AZStd::ref(sequenceEventHandler), + AZStd::ref(sequenceEventHandler), + AZStd::ref(sequenceEventHandler)); } else { - // One sequence timed out whilst the other sequence succeeded or both sequences timed out - result = TestSequenceResult::Timeout; + throw(Exception("Unexpected sequence type")); } } else { + Policy::DynamicDependencyMap dynamicDependencyMapPolicy; + if (options.GetTestSequenceType() == TestSequenceType::ImpactAnalysis) + { + dynamicDependencyMapPolicy = Policy::DynamicDependencyMap::Update; + } + else if (options.GetTestSequenceType() == TestSequenceType::ImpactAnalysisNoWrite) + { + dynamicDependencyMapPolicy = Policy::DynamicDependencyMap::Discard; + } + else + { + throw(Exception("Unexpected sequence type")); + } + result = runtime.ImpactAnalysisTestSequence( changeList.value(), options.GetTestPrioritizationPolicy(), + dynamicDependencyMapPolicy, options.GetTestTargetTimeout(), options.GetGlobalTimeout(), AZStd::ref(sequenceEventHandler), @@ -164,9 +199,11 @@ namespace TestImpact // As of now, there are no other non-test operations other than printing a change list so getting this far is considered an error AZ_TestImpact_Eval(options.GetTestSequenceType() != TestSequenceType::None, CommandLineOptionsException, "No action specified"); - std::cout << "Constructing in-memory model of source tree and test coverage, this may take a moment...\n"; + std::cout << "Constructing in-memory model of source tree and test coverage for test suite "; + std::cout << GetSuiteTypeName(options.GetSuiteFilter()).c_str() << ", this may take a moment...\n"; Runtime runtime( RuntimeConfigurationFactory(ReadFileContents(options.GetConfigurationFile())), + options.GetSuiteFilter(), options.GetExecutionFailurePolicy(), options.GetExecutionFailureDraftingPolicy(), options.GetTestFailurePolicy(), @@ -184,14 +221,13 @@ namespace TestImpact std::cout << "Test impact analysis data for this repository was not found, seed or regular sequence fallbacks will be used.\n"; } - TestSequenceEventHandler sequenceEventHandler(&options.GetSuitesFilter()); + TestSequenceEventHandler sequenceEventHandler(options.GetSuiteFilter()); switch (const auto type = options.GetTestSequenceType()) { case TestSequenceType::Regular: { const auto result = runtime.RegularTestSequence( - options.GetSuitesFilter(), options.GetTestTargetTimeout(), options.GetGlobalTimeout(), AZStd::ref(sequenceEventHandler), @@ -211,6 +247,7 @@ namespace TestImpact return GetReturnCodeForTestSequenceResult(result); } + case TestSequenceType::ImpactAnalysisNoWrite: case TestSequenceType::ImpactAnalysis: { return WrappedImpactAnalysisTestSequence(sequenceEventHandler, options, runtime, changeList); diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactRuntimeConfigurationFactory.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactRuntimeConfigurationFactory.cpp index 9db99d3c2e..6ef1ce4172 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactRuntimeConfigurationFactory.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactRuntimeConfigurationFactory.cpp @@ -30,7 +30,7 @@ namespace TestImpact "relative_paths", "artifact_dir", "enumeration_cache_dir", - "test_impact_data_file", + "test_impact_data_files", "temp", "active", "target_sources", @@ -75,7 +75,7 @@ namespace TestImpact RelativePaths, ArtifactDir, EnumerationCacheDir, - TestImpactDataFile, + TestImpactDataFiles, TempWorkspace, ActiveWorkspace, TargetSources, @@ -144,6 +144,19 @@ namespace TestImpact return tempWorkspaceConfig; } + AZStd::array ParseTestImpactAnalysisDataFiles(const RepoPath& root, const rapidjson::Value& sparTIAFile) + { + AZStd::array sparTIAFiles; + sparTIAFiles[static_cast(SuiteType::Main)] = + GetAbsPathFromRelPath(root, sparTIAFile[GetSuiteTypeName(SuiteType::Main).c_str()].GetString()); + sparTIAFiles[static_cast(SuiteType::Periodic)] = + GetAbsPathFromRelPath(root, sparTIAFile[GetSuiteTypeName(SuiteType::Periodic).c_str()].GetString()); + sparTIAFiles[static_cast(SuiteType::Sandbox)] = + GetAbsPathFromRelPath(root, sparTIAFile[GetSuiteTypeName(SuiteType::Sandbox).c_str()].GetString()); + + return sparTIAFiles; + } + WorkspaceConfig::Active ParseActiveWorkspaceConfig(const rapidjson::Value& activeWorkspace) { WorkspaceConfig::Active activeWorkspaceConfig; @@ -151,8 +164,8 @@ namespace TestImpact activeWorkspaceConfig.m_root = activeWorkspace[Config::Keys[Config::Root]].GetString(); activeWorkspaceConfig.m_enumerationCacheDirectory = GetAbsPathFromRelPath(activeWorkspaceConfig.m_root, relativePaths[Config::Keys[Config::EnumerationCacheDir]].GetString()); - activeWorkspaceConfig.m_sparTIAFile - = GetAbsPathFromRelPath(activeWorkspaceConfig.m_root, relativePaths[Config::Keys[Config::TestImpactDataFile]].GetString()); + activeWorkspaceConfig.m_sparTIAFiles = + ParseTestImpactAnalysisDataFiles(activeWorkspaceConfig.m_root, relativePaths[Config::Keys[Config::TestImpactDataFiles]]); return activeWorkspaceConfig; } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactConfiguration.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactConfiguration.h index 91d748edfe..ee73ac572d 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactConfiguration.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactConfiguration.h @@ -16,6 +16,7 @@ #include #include +#include #include namespace TestImpact @@ -46,8 +47,8 @@ namespace TestImpact struct Active { RepoPath m_root; //!< Path to the persistent workspace tracked by the repository. - RepoPath m_sparTIAFile; //!< Path to the test impact analysis data. RepoPath m_enumerationCacheDirectory; //!< Path to the test enumerations cache. + AZStd::array m_sparTIAFiles; //!< Paths to the test impact analysis data files for each test suite. }; Temp m_temp; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h index 266bd5ef7a..ae1f78c5c7 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h @@ -92,6 +92,7 @@ namespace TestImpact public: //! Constructs a runtime with the specified configuration and policies. //! @param config The configuration used for this runtime instance. + //! @param suiteFilter The test suite for which the coverage data and test selection will draw from. //! @param executionFailurePolicy Determines how to handle test targets that fail to execute. //! @param executionFailureDraftingPolicy Determines how test targets that previously failed to execute are drafted into subsequent test sequences. //! @param testFailurePolicy Determines how to handle test targets that report test failures. @@ -99,6 +100,7 @@ namespace TestImpact //! @param testShardingPolicy Determines how to handle test targets that have opted in to test sharding. Runtime( RuntimeConfig&& config, + SuiteType suiteFilter, Policy::ExecutionFailure executionFailurePolicy, Policy::ExecutionFailureDrafting executionFailureDraftingPolicy, Policy::TestFailure testFailurePolicy, @@ -110,7 +112,6 @@ namespace TestImpact ~Runtime(); //! Runs a test sequence where all tests with a matching suite in the suite filter and also not on the excluded list are selected. - //! @param suitesFilter The test suites that will be included in the test selection. //! @param testTargetTimeout The maximum duration individual test targets may be in flight for (infinite if empty). //! @param globalTimeout The maximum duration the entire test sequence may run for (infinite if empty). //! @param testSequenceStartCallback The client function to be called after the test targets have been selected but prior to running the tests. @@ -118,7 +119,6 @@ namespace TestImpact //! @param testRunCompleteCallback The client function to be called after an individual test run has completed. //! @returns TestSequenceResult RegularTestSequence( - const AZStd::unordered_set suitesFilter, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, AZStd::optional testSequenceStartCallback, @@ -128,6 +128,7 @@ namespace TestImpact //! Runs a test sequence where tests are selected according to test impact analysis so long as they are not on the excluded list. //! @param changeList The change list used to determine the tests to select. //! @param testPrioritizationPolicy Determines how selected tests will be prioritized. + //! @param dynamicDependencyMapPolicy The policy to determine how the coverage data of produced by test sequences is used to update the dynamic dependency map. //! @param testTargetTimeout The maximum duration individual test targets may be in flight for (infinite if empty). //! @param globalTimeout The maximum duration the entire test sequence may run for (infinite if empty). //! @param testSequenceStartCallback The client function to be called after the test targets have been selected but prior to running the tests. @@ -137,6 +138,7 @@ namespace TestImpact TestSequenceResult ImpactAnalysisTestSequence( const ChangeList& changeList, Policy::TestPrioritization testPrioritizationPolicy, + Policy::DynamicDependencyMap dynamicDependencyMapPolicy, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, AZStd::optional testSequenceStartCallback, @@ -145,7 +147,6 @@ namespace TestImpact //! Runs a test sequence as per the ImpactAnalysisTestSequence where the tests not selected are also run (albeit without instrumentation). //! @param changeList The change list used to determine the tests to select. - //! @param suitesFilter The test suites that will be included in the test selection. //! @param testPrioritizationPolicy Determines how selected tests will be prioritized. //! @param testTargetTimeout The maximum duration individual test targets may be in flight for (infinite if empty). //! @param globalTimeout The maximum duration the entire test sequence may run for (infinite if empty). @@ -155,7 +156,6 @@ namespace TestImpact //! @returns AZStd::pair SafeImpactAnalysisTestSequence( const ChangeList& changeList, - const AZStd::unordered_set suitesFilter, Policy::TestPrioritization testPrioritizationPolicy, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, @@ -207,6 +207,8 @@ namespace TestImpact void UpdateAndSerializeDynamicDependencyMap(const SourceCoveringTestsList& sourceCoverageTestsList); RuntimeConfig m_config; + SuiteType m_suiteFilter; + RepoPath m_sparTIAFile; Policy::ExecutionFailure m_executionFailurePolicy; Policy::ExecutionFailureDrafting m_executionFailureDraftingPolicy; Policy::TestFailure m_testFailurePolicy; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h index 594c997c45..4e8908cf3f 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h @@ -12,6 +12,10 @@ #pragma once +#include + +#include + namespace TestImpact { namespace Policy @@ -55,6 +59,13 @@ namespace TestImpact Continue //!< Continue the test sequence and report the test failures after the run. }; + //! Policy for updating the dynamic dependency map with the coverage data of produced by test sequences. + enum class DynamicDependencyMap + { + Discard, //!< Discard the coverage data produced by test sequences. + Update //!< Update the dynamic dependency map with the coverage data produced by test sequences. + }; + //! Policy for sharding test targets that have been marked for test sharding. enum class TestSharding { @@ -82,6 +93,30 @@ namespace TestImpact TestInterleaved //!< Tests are interlaced across shards agnostic of fixtures (fastest but prone to inter-test dependency problems). }; + //! Test suite types to select from. + enum class SuiteType : AZ::u8 + { + Main = 0, + Periodic, + Sandbox + }; + + //! User-friendly names for the test suite types. + inline AZStd::string GetSuiteTypeName(SuiteType suiteType) + { + switch (suiteType) + { + case SuiteType::Main: + return "main"; + case SuiteType::Periodic: + return "periodic"; + case SuiteType::Sandbox: + return "sandbox"; + default: + throw(RuntimeException("Unexpected suite type")); + } + } + //! Result of a test sequence that was run. enum class TestSequenceResult { diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.cpp index 8bbb5200bb..4b247086ed 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.cpp @@ -19,7 +19,7 @@ namespace TestImpact { - TestTargetMetaMap TestTargetMetaMapFactory(const AZStd::string& masterTestListData) + TestTargetMetaMap TestTargetMetaMapFactory(const AZStd::string& masterTestListData, SuiteType suiteType) { // Keys for pertinent JSON node and attribute names constexpr const char* Keys[] = @@ -27,6 +27,7 @@ namespace TestImpact "google", "test", "tests", + "suites", "suite", "launch_method", "test_runner", @@ -41,6 +42,7 @@ namespace TestImpact GoogleKey, TestKey, TestsKey, + TestSuitesKey, SuiteKey, LaunchMethodKey, TestRunnerKey, @@ -64,26 +66,35 @@ namespace TestImpact for (const auto& test : tests) { TestTargetMeta testMeta; - testMeta.m_suite = test[Keys[SuiteKey]].GetString(); - testMeta.m_customArgs = test[Keys[CommandKey]].GetString(); - testMeta.m_timeout = AZStd::chrono::seconds{ test[Keys[TimeoutKey]].GetUint() }; + const auto testSuites = test[Keys[TestSuitesKey]].GetArray(); + for (const auto& suite : testSuites) + { + // Check to see if this test target has the suite we're looking for + if (const auto suiteName = suite[Keys[SuiteKey]].GetString(); + strcmp(GetSuiteTypeName(suiteType).c_str(), suiteName) == 0) + { + testMeta.m_suite = suiteName; + testMeta.m_customArgs = suite[Keys[CommandKey]].GetString(); + testMeta.m_timeout = AZStd::chrono::seconds{ suite[Keys[TimeoutKey]].GetUint() }; + if (const auto buildTypeString = test[Keys[LaunchMethodKey]].GetString(); strcmp(buildTypeString, Keys[TestRunnerKey]) == 0) + { + testMeta.m_launchMethod = LaunchMethod::TestRunner; + } + else if (strcmp(buildTypeString, Keys[StandAloneKey]) == 0) + { + testMeta.m_launchMethod = LaunchMethod::StandAlone; + } + else + { + throw(ArtifactException("Unexpected test build type")); + } - if (const auto buildTypeString = test[Keys[LaunchMethodKey]].GetString(); strcmp(buildTypeString, Keys[TestRunnerKey]) == 0) - { - testMeta.m_launchMethod = LaunchMethod::TestRunner; + AZStd::string name = test[Keys[NameKey]].GetString(); + AZ_TestImpact_Eval(!name.empty(), ArtifactException, "Test name field cannot be empty"); + testMetas.emplace(AZStd::move(name), AZStd::move(testMeta)); + break; + } } - else if (strcmp(buildTypeString, Keys[StandAloneKey]) == 0) - { - testMeta.m_launchMethod = LaunchMethod::StandAlone; - } - else - { - throw(ArtifactException("Unexpected test build type")); - } - - AZStd::string name = test[Keys[NameKey]].GetString(); - AZ_TestImpact_Eval(!name.empty(), ArtifactException, "Test name field cannot be empty"); - testMetas.emplace(AZStd::move(name), AZStd::move(testMeta)); } // If there's no tests in the repo then something is seriously wrong diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.h index 039a9e89e2..b08babf528 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.h @@ -12,14 +12,16 @@ #pragma once +#include #include #include namespace TestImpact { - //! Constructs a list of test target meta-data artifacts from the specified master test list data. + //! Constructs a list of test target meta-data artifacts of the specified suite type from the specified master test list data. //! @param masterTestListData The raw master test list data in JSON format. + //! @param suiteType The suite type to select the target meta-data artifacts from. //! @return The constructed list of test target meta-data artifacts. - TestTargetMetaMap TestTargetMetaMapFactory(const AZStd::string& masterTestListData); + TestTargetMetaMap TestTargetMetaMapFactory(const AZStd::string& masterTestListData, SuiteType suiteType); } // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp index 826fa0a87d..ece5a7c13b 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp @@ -371,11 +371,11 @@ namespace TestImpact { if (sourceDependency->GetNumCoveringTestTargets()) { - AZ_Warning( - "File Update", false, AZStd::string::format("Source file %s is potentially an orphan (used by build targets " + AZ_Printf( + "File Update", AZStd::string::format("Source file '%s' is potentially an orphan (used by build targets " "without explicitly being added to the build system, e.g. an include directive pulling in a header from the " "repository). Running the covering tests for this file with instrumentation will confirm whether or nor this " - "is the case", updatedFile.c_str()).c_str()); + "is the case.\n", updatedFile.c_str()).c_str()); updateDependencies.emplace_back(AZStd::move(*sourceDependency)); coverageToDelete.push_back(updatedFile); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp index 83808f4ed2..2331c76880 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -72,6 +72,7 @@ namespace TestImpact Runtime::Runtime( RuntimeConfig&& config, + SuiteType suiteFilter, Policy::ExecutionFailure executionFailurePolicy, Policy::ExecutionFailureDrafting executionFailureDraftingPolicy, Policy::TestFailure testFailurePolicy, @@ -80,6 +81,7 @@ namespace TestImpact Policy::TargetOutputCapture targetOutputCapture, AZStd::optional maxConcurrency) : m_config(AZStd::move(config)) + , m_suiteFilter(suiteFilter) , m_executionFailurePolicy(executionFailurePolicy) , m_executionFailureDraftingPolicy(executionFailureDraftingPolicy) , m_testFailurePolicy(testFailurePolicy) @@ -89,7 +91,7 @@ namespace TestImpact , m_maxConcurrency(maxConcurrency.value_or(AZStd::thread::hardware_concurrency())) { // Construct the dynamic dependency map from the build target descriptors - m_dynamicDependencyMap = ConstructDynamicDependencyMap(m_config.m_buildTargetDescriptor, m_config.m_testTargetMeta); + m_dynamicDependencyMap = ConstructDynamicDependencyMap(suiteFilter, m_config.m_buildTargetDescriptor, m_config.m_testTargetMeta); // Construct the test selector and prioritizer from the dependency graph data (NOTE: currently not implemented) m_testSelectorAndPrioritizer = AZStd::make_unique(m_dynamicDependencyMap.get(), DependencyGraphDataMap{}); @@ -110,7 +112,8 @@ namespace TestImpact try { // Populate the dynamic dependency map with the existing source coverage data (if any) - const auto tiaDataRaw = ReadFileContents(m_config.m_workspace.m_active.m_sparTIAFile); + m_sparTIAFile = m_config.m_workspace.m_active.m_sparTIAFiles[static_cast(m_suiteFilter)].String(); + const auto tiaDataRaw = ReadFileContents(m_sparTIAFile); const auto tiaData = DeserializeSourceCoveringTestsList(tiaDataRaw); if (tiaData.GetNumSources()) { @@ -118,13 +121,17 @@ namespace TestImpact m_hasImpactAnalysisData = true; // Enumerate new test targets - m_testEngine->UpdateEnumerationCache( - m_dynamicDependencyMap->GetNotCoveringTests(), - Policy::ExecutionFailure::Ignore, - Policy::TestFailure::Continue, - AZStd::nullopt, - AZStd::nullopt, - AZStd::nullopt); + const auto testTargetsWithNoEnumeration = m_dynamicDependencyMap->GetNotCoveringTests(); + if (!testTargetsWithNoEnumeration.empty()) + { + m_testEngine->UpdateEnumerationCache( + testTargetsWithNoEnumeration, + Policy::ExecutionFailure::Ignore, + Policy::TestFailure::Continue, + AZStd::nullopt, + AZStd::nullopt, + AZStd::nullopt); + } } } catch (const DependencyException& e) @@ -136,8 +143,10 @@ namespace TestImpact } catch ([[maybe_unused]]const Exception& e) { - AZ_Printf("No test impact analysis data found at %s", m_config.m_workspace.m_active.m_sparTIAFile.c_str()); - } + AZ_Printf("TestImpactRuntime", + AZStd::string::format( + "No test impact analysis data found for suite '%s' at %s", GetSuiteTypeName(m_suiteFilter).c_str(), m_sparTIAFile.c_str()).c_str()); + } } Runtime::~Runtime() = default; @@ -168,13 +177,16 @@ namespace TestImpact addMutatedTestTargetsToEnumerationList(changeDependencyList.GetDeleteSourceDependencies()); // Enumerate the mutated test targets to ensure their enumeration caches are up to date - m_testEngine->UpdateEnumerationCache( - testTargets, - Policy::ExecutionFailure::Ignore, - Policy::TestFailure::Continue, - AZStd::nullopt, - AZStd::nullopt, - AZStd::nullopt); + if (!testTargets.empty()) + { + m_testEngine->UpdateEnumerationCache( + testTargets, + Policy::ExecutionFailure::Ignore, + Policy::TestFailure::Continue, + AZStd::nullopt, + AZStd::nullopt, + AZStd::nullopt); + } } AZStd::pair, AZStd::vector> Runtime::SelectCoveringTestTargetsAndUpdateEnumerationCache( @@ -233,7 +245,7 @@ namespace TestImpact void Runtime::ClearDynamicDependencyMapAndRemoveExistingFile() { - DeleteFile(m_config.m_workspace.m_active.m_sparTIAFile); + DeleteFile(m_sparTIAFile); m_dynamicDependencyMap->ClearAllSourceCoverage(); } @@ -247,12 +259,11 @@ namespace TestImpact m_dynamicDependencyMap->ReplaceSourceCoverage(sourceCoverageTestsList); const auto sparTIA = m_dynamicDependencyMap->ExportSourceCoverage(); const auto sparTIAData = SerializeSourceCoveringTestsList(sparTIA); - WriteFileContents(sparTIAData, m_config.m_workspace.m_active.m_sparTIAFile); + WriteFileContents(sparTIAData, m_sparTIAFile); m_hasImpactAnalysisData = true; } TestSequenceResult Runtime::RegularTestSequence( - const AZStd::unordered_set suitesFilter, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, AZStd::optional testSequenceStartCallback, @@ -268,21 +279,7 @@ namespace TestImpact { if (!m_testTargetExcludeList.contains(&testTarget)) { - if (suitesFilter.empty()) - { - // Suite filter is empty, all tests that are not on the excluded list are included - includedTestTargets.push_back(&testTarget); - } - else if(suitesFilter.contains(testTarget.GetSuite())) - { - // Test target belonging to a suite in the suite filter are included, provided that are not on the exclude list - includedTestTargets.push_back(&testTarget); - } - else - { - // Test target not belonging to a suite in the suite filter are excluded - excludedTestTargets.push_back(&testTarget); - } + includedTestTargets.push_back(&testTarget); } else { @@ -318,6 +315,7 @@ namespace TestImpact TestSequenceResult Runtime::ImpactAnalysisTestSequence( const ChangeList& changeList, Policy::TestPrioritization testPrioritizationPolicy, + Policy::DynamicDependencyMap dynamicDependencyMapPolicy, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, AZStd::optional testSequenceStartCallback, @@ -338,30 +336,51 @@ namespace TestImpact ExtractTestTargetNames(draftedTestTargets)); } - const auto [result, testJobs] = m_testEngine->InstrumentedRun( - includedSelectedTestTargets, - m_testShardingPolicy, - m_executionFailurePolicy, - Policy::IntegrityFailure::Continue, - m_testFailurePolicy, - m_targetOutputCapture, - testTargetTimeout, - globalTimeout, - TestRunCompleteCallbackHandler(testCompleteCallback)); - - UpdateAndSerializeDynamicDependencyMap(CreateSourceCoveringTestFromTestCoverages(testJobs, m_config.m_repo.m_root)); - - if (testSequenceEndCallback.has_value()) + if (dynamicDependencyMapPolicy == Policy::DynamicDependencyMap::Update) { - (*testSequenceEndCallback)(GenerateSequenceFailureReport(testJobs), timer.Elapsed()); - } + const auto [result, testJobs] = m_testEngine->InstrumentedRun( + includedSelectedTestTargets, + m_testShardingPolicy, + m_executionFailurePolicy, + Policy::IntegrityFailure::Continue, + m_testFailurePolicy, + m_targetOutputCapture, + testTargetTimeout, + globalTimeout, + TestRunCompleteCallbackHandler(testCompleteCallback)); - return result; + UpdateAndSerializeDynamicDependencyMap(CreateSourceCoveringTestFromTestCoverages(testJobs, m_config.m_repo.m_root)); + + if (testSequenceEndCallback.has_value()) + { + (*testSequenceEndCallback)(GenerateSequenceFailureReport(testJobs), timer.Elapsed()); + } + + return result; + } + else + { + const auto [result, testJobs] = m_testEngine->RegularRun( + includedSelectedTestTargets, + m_testShardingPolicy, + m_executionFailurePolicy, + m_testFailurePolicy, + m_targetOutputCapture, + testTargetTimeout, + globalTimeout, + TestRunCompleteCallbackHandler(testCompleteCallback)); + + if (testSequenceEndCallback.has_value()) + { + (*testSequenceEndCallback)(GenerateSequenceFailureReport(testJobs), timer.Elapsed()); + } + + return result; + } } AZStd::pair Runtime::SafeImpactAnalysisTestSequence( const ChangeList& changeList, - const AZStd::unordered_set suitesFilter, Policy::TestPrioritization testPrioritizationPolicy, AZStd::optional testTargetTimeout, AZStd::optional globalTimeout, diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp index 87a402bd6a..47c0e83233 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.cpp @@ -22,10 +22,10 @@ namespace TestImpact { - TestTargetMetaMap ReadTestTargetMetaMapFile(const RepoPath& testTargetMetaConfigFile) + TestTargetMetaMap ReadTestTargetMetaMapFile(SuiteType suiteFilter, const RepoPath& testTargetMetaConfigFile) { const auto masterTestListData = ReadFileContents(testTargetMetaConfigFile); - return TestTargetMetaMapFactory(masterTestListData); + return TestTargetMetaMapFactory(masterTestListData, suiteFilter); } AZStd::vector ReadBuildTargetDescriptorFiles(const BuildTargetDescriptorConfig& buildTargetDescriptorConfig) @@ -46,10 +46,11 @@ namespace TestImpact } AZStd::unique_ptr ConstructDynamicDependencyMap( + SuiteType suiteFilter, const BuildTargetDescriptorConfig& buildTargetDescriptorConfig, const TestTargetMetaConfig& testTargetMetaConfig) { - auto testTargetmetaMap = ReadTestTargetMetaMapFile(testTargetMetaConfig.m_metaFile); + auto testTargetmetaMap = ReadTestTargetMetaMapFile(suiteFilter, testTargetMetaConfig.m_metaFile); auto buildTargetDescriptors = ReadBuildTargetDescriptorFiles(buildTargetDescriptorConfig); auto buildTargets = CompileTargetDescriptors(AZStd::move(buildTargetDescriptors), AZStd::move(testTargetmetaMap)); auto&& [productionTargets, testTargets] = buildTargets; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h index 56775b1c58..943b0cbec4 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntimeUtils.h @@ -30,6 +30,7 @@ namespace TestImpact { //! Construct a dynamic dependency map from the build target descriptors and test target metas. AZStd::unique_ptr ConstructDynamicDependencyMap( + SuiteType suiteFilter, const BuildTargetDescriptorConfig& buildTargetDescriptorConfig, const TestTargetMetaConfig& testTargetMetaConfig); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake index da3693433e..2053a1ea5e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake +++ b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_files.cmake @@ -10,15 +10,23 @@ # set(FILES - Include/TestImpactFramework/TestImpactBitwise.h - Include/TestImpactFramework/TestImpactCallback.h Include/TestImpactFramework/TestImpactException.h - Include/TestImpactFramework/TestImpactFrameworkPath.h + Include/TestImpactFramework/TestImpactRepoPath.h + Include/TestImpactFramework/TestImpactRuntime.h + Include/TestImpactFramework/TestImpactRuntimeException.h + Include/TestImpactFramework/TestImpactConfiguration.h + Include/TestImpactFramework/TestImpactConfigurationException.h + Include/TestImpactFramework/TestImpactChangelist.h + Include/TestImpactFramework/TestImpactChangelistSerializer.h + Include/TestImpactFramework/TestImpactChangelistException.h + Include/TestImpactFramework/TestImpactTestSequence.h + Include/TestImpactFramework/TestImpactClientTestSelection.h + Include/TestImpactFramework/TestImpactClientTestRun.h + Include/TestImpactFramework/TestImpactClientFailureReport.h + Include/TestImpactFramework/TestImpactFileUtils.h Source/Artifact/TestImpactArtifactException.h Source/Artifact/Factory/TestImpactBuildTargetDescriptorFactory.cpp Source/Artifact/Factory/TestImpactBuildTargetDescriptorFactory.h - Source/Artifact/Factory/TestImpactChangeListFactory.cpp - Source/Artifact/Factory/TestImpactChangeListFactory.h Source/Artifact/Factory/TestImpactTestEnumerationSuiteFactory.cpp Source/Artifact/Factory/TestImpactTestEnumerationSuiteFactory.h Source/Artifact/Factory/TestImpactTestRunSuiteFactory.cpp @@ -27,6 +35,8 @@ set(FILES Source/Artifact/Factory/TestImpactTestTargetMetaMapFactory.h Source/Artifact/Factory/TestImpactModuleCoverageFactory.cpp Source/Artifact/Factory/TestImpactModuleCoverageFactory.h + Source/Artifact/Factory/TestImpactDependencyGraphDataFactory.cpp + Source/Artifact/Factory/TestImpactDependencyGraphDataFactory.h Source/Artifact/Static/TestImpactBuildTargetDescriptor.cpp Source/Artifact/Static/TestImpactBuildTargetDescriptor.h Source/Artifact/Static/TestImpactTargetDescriptorCompiler.cpp @@ -37,7 +47,6 @@ set(FILES Source/Artifact/Static/TestImpactTestTargetDescriptor.cpp Source/Artifact/Static/TestImpactTestTargetDescriptor.h Source/Artifact/Static/TestImpactDependencyGraphData.h - Source/Artifact/Dynamic/TestImpactChangelist.h Source/Artifact/Dynamic/TestImpactTestEnumerationSuite.h Source/Artifact/Dynamic/TestImpactTestRunSuite.h Source/Artifact/Dynamic/TestImpactTestSuite.h @@ -50,6 +59,8 @@ set(FILES Source/Process/TestImpactProcessLauncher.h Source/Process/JobRunner/TestImpactProcessJob.h Source/Process/JobRunner/TestImpactProcessJobInfo.h + Source/Process/JobRunner/TestImpactProcessJobMeta.cpp + Source/Process/JobRunner/TestImpactProcessJobMeta.h Source/Process/JobRunner/TestImpactProcessJobRunner.h Source/Process/Scheduler/TestImpactProcessScheduler.cpp Source/Process/Scheduler/TestImpactProcessScheduler.h @@ -64,6 +75,8 @@ set(FILES Source/Dependency/TestImpactTestSelectorAndPrioritizer.cpp Source/Dependency/TestImpactSourceCoveringTestsList.h Source/Dependency/TestImpactSourceCoveringTestsList.cpp + Source/Dependency/TestImpactSourceCoveringTestsSerializer.cpp + Source/Dependency/TestImpactSourceCoveringTestsSerializer.h Source/Target/TestImpactBuildTarget.cpp Source/Target/TestImpactBuildTarget.h Source/Target/TestImpactBuildTargetList.h @@ -74,29 +87,48 @@ set(FILES Source/Target/TestImpactTestTarget.cpp Source/Target/TestImpactTestTarget.h Source/Target/TestImpactTestTargetList.h - Source/Test/Enumeration/TestImpactTestEnumeration.h - Source/Test/Enumeration/TestImpactTestEnumerationException.h - Source/Test/Enumeration/TestImpactTestEnumerationSerializer.cpp - Source/Test/Enumeration/TestImpactTestEnumerationSerializer.h - Source/Test/Enumeration/TestImpactTestEnumerator.cpp - Source/Test/Enumeration/TestImpactTestEnumerator.h - Source/Test/Run/TestImpactTestRunSerializer.cpp - Source/Test/Run/TestImpactTestRunSerializer.h - Source/Test/Run/TestImpactTestRunner.cpp - Source/Test/Run/TestImpactTestRunner.h - Source/Test/Run/TestImpactInstrumentedTestRunner.cpp - Source/Test/Run/TestImpactInstrumentedTestRunner.h - Source/Test/Run/TestImpactTestRun.cpp - Source/Test/Run/TestImpactTestRun.h - Source/Test/Run/TestImpactTestRunJobData.cpp - Source/Test/Run/TestImpactTestRunJobData.h - Source/Test/Run/TestImpactTestCoverage.cpp - Source/Test/Run/TestImpactTestCoverage.h - Source/Test/Run/TestImpactTestRunException.h - Source/Test/Job/TestImpactTestJobRunner.h - Source/Test/Job/TestImpactTestJobException.h - Source/Test/Job/TestImpactTestJobCommon.h - Source/Test/TestImpactTestSuiteContainer.h + Source/TestEngine/Enumeration/TestImpactTestEnumeration.h + Source/TestEngine/Enumeration/TestImpactTestEnumerationSerializer.cpp + Source/TestEngine/Enumeration/TestImpactTestEnumerationSerializer.h + Source/TestEngine/Enumeration/TestImpactTestEnumerator.cpp + Source/TestEngine/Enumeration/TestImpactTestEnumerator.h + Source/TestEngine/Run/TestImpactTestRunSerializer.cpp + Source/TestEngine/Run/TestImpactTestRunSerializer.h + Source/TestEngine/Run/TestImpactTestRunner.cpp + Source/TestEngine/Run/TestImpactTestRunner.h + Source/TestEngine/Run/TestImpactInstrumentedTestRunner.cpp + Source/TestEngine/Run/TestImpactInstrumentedTestRunner.h + Source/TestEngine/Run/TestImpactTestRun.cpp + Source/TestEngine/Run/TestImpactTestRun.h + Source/TestEngine/Run/TestImpactTestRunJobData.cpp + Source/TestEngine/Run/TestImpactTestRunJobData.h + Source/TestEngine/Run/TestImpactTestCoverage.cpp + Source/TestEngine/Run/TestImpactTestCoverage.h + Source/TestEngine/JobRunner/TestImpactTestJobRunner.h + Source/TestEngine/JobRunner/TestImpactTestJobInfoGenerator.cpp + Source/TestEngine/JobRunner/TestImpactTestJobInfoGenerator.h + Source/TestEngine/JobRunner/TestImpactTestTargetExtension.h + Source/TestEngine/TestImpactTestEngineJobFailure.cpp + Source/TestEngine/TestImpactTestEngineJobFailure.h + Source/TestEngine/TestImpactTestSuiteContainer.h + Source/TestEngine/TestImpactTestEngine.cpp + Source/TestEngine/TestImpactTestEngine.h + Source/TestEngine/TestImpactTestEngineJob.cpp + Source/TestEngine/TestImpactTestEngineJob.h + Source/TestEngine/TestImpactTestEngineEnumeration.cpp + Source/TestEngine/TestImpactTestEngineEnumeration.h + Source/TestEngine/TestImpactTestEngineRegularRun.cpp + Source/TestEngine/TestImpactTestEngineRegularRun.h + Source/TestEngine/TestImpactTestEngineInstrumentedRun.cpp + Source/TestEngine/TestImpactTestEngineInstrumentedRun.h + Source/TestEngine/TestImpactTestEngineException.h Source/TestImpactException.cpp - Source/TestImpactFrameworkPath.cpp + Source/TestImpactRuntime.cpp + Source/TestImpactRuntimeUtils.cpp + Source/TestImpactRuntimeUtils.h + Source/TestImpactClientTestSelection.cpp + Source/TestImpactClientTestRun.cpp + Source/TestImpactClientFailureReport.cpp + Source/TestImpactChangeListSerializer.cpp + Source/TestImpactRepoPath.cpp ) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake b/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake deleted file mode 100644 index 13f788c37b..0000000000 --- a/Code/Tools/TestImpactFramework/Runtime/Code/testimpactframework_runtime_tests_files.cmake +++ /dev/null @@ -1,38 +0,0 @@ -# -# All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or -# its licensors. -# -# For complete copyright and license terms please see the LICENSE at the root of this -# distribution (the "License"). All use of this software is governed by the License, -# or, if provided, by the license below or the license accompanying this file. Do not -# remove or modify any license notices. This file is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# - -set(FILES - Tests/Artifact/TestImpactTargetDescriptorCompilerTest.cpp - Tests/Artifact/TestImpactBuildTargetDescriptorFactoryTest.cpp - Tests/Artifact/TestImpactModuleCoverageFactoryTest.cpp - Tests/Artifact/TestImpactChangeListFactoryTest.cpp - Tests/Artifact/TestImpactTestEnumerationSuiteFactoryTest.cpp - Tests/Artifact/TestImpactTestRunSuiteFactoryTest.cpp - Tests/Artifact/TestImpactTestTargetMetaMapFactoryTest.cpp - - Tests/Process/TestImpactProcessSchedulerTest.cpp - - Tests/Process/TestImpactProcessTest.cpp - Tests/Target/TestImpactBuildTargetTest.cpp - Tests/TestImpactExceptionTest.cpp - Tests/TestImpactFrameworkPathTest.cpp - Tests/Test/TestImpactTestEnumeratorTest.cpp - Tests/Test/TestImpactTestEumerationSerializerTest.cpp - Tests/Test/TestImpactTestRunSerializerTest.cpp - Tests/Test/TestImpactTestRunnerTest.cpp - Tests/Test/TestImpactInstrumentedTestRunnerTest.cpp - Tests/Test/TestImpactTestCoverageTest.cpp - Tests/TestImpactTestJobRunnerCommon.h - Tests/TestImpactTestMain.cpp - Tests/TestImpactTestUtils.cpp - Tests/TestImpactTestUtils.h - -) diff --git a/cmake/LYTestWrappers.cmake b/cmake/LYTestWrappers.cmake index 5b7e1c766b..3308c4411a 100644 --- a/cmake/LYTestWrappers.cmake +++ b/cmake/LYTestWrappers.cmake @@ -241,12 +241,18 @@ function(ly_add_test) endif() - # Store the test so we can walk through all of them in LYTestImpactFramework.cmake - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS ${ly_add_test_NAME}) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_test_NAME}_TEST_SUITE ${ly_add_test_TEST_SUITE}) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_test_NAME}_TEST_LIBRARY ${ly_add_test_TEST_LIBRARY}) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_test_NAME}_TEST_TIMEOUT ${ly_add_test_TIMEOUT}) - + # Check to see whether or not this test target has been stored in the global list for walking by the test impact analysis framework + get_property(all_tests GLOBAL PROPERTY LY_ALL_TESTS) + if(NOT "${LY_ALL_TESTS_TARGET_NAME}" IN_LIST all_tests) + # This is the first reference to this test target so add it to the global list + set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS ${LY_ALL_TESTS_TARGET_NAME}) + set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${LY_ALL_TESTS_TARGET_NAME}_TEST_LIBRARY ${ly_add_test_TEST_LIBRARY}) + endif() + # Add the test suite and timeout value to the test target params + set(LY_TEST_PARAMS "${LY_TEST_PARAMS}#${ly_add_test_TEST_SUITE}") + set(LY_TEST_PARAMS "${LY_TEST_PARAMS}#${ly_add_test_TIMEOUT}") + # Store the params for this test target + set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${LY_ALL_TESTS_TARGET_NAME}_PARAMS ${LY_TEST_PARAMS}) endfunction() #! ly_add_pytest: registers target PyTest-based test with CTest @@ -288,6 +294,11 @@ function(ly_add_pytest) string(REPLACE "::" "_" pytest_report_directory "${PYTEST_XML_OUTPUT_DIR}/${ly_add_pytest_NAME}.xml") + # Set the name of the current test target for storage in the global list + set(LY_ALL_TESTS_TARGET_NAME ${ly_add_pytest_NAME}) + # Add the script path to the test target params + set(LY_TEST_PARAMS "${ly_add_pytest_PATH}") + ly_add_test( NAME ${ly_add_pytest_NAME} TEST_SUITE ${ly_add_pytest_TEST_SUITE} @@ -298,7 +309,6 @@ function(ly_add_pytest) ${ly_add_pytest_UNPARSED_ARGUMENTS} ) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_pytest_NAME}_SCRIPT_PATH ${ly_add_pytest_PATH}) set_tests_properties(${LY_ADDED_TEST_NAME} PROPERTIES RUN_SERIAL "${ly_add_pytest_TEST_SERIAL}") endfunction() @@ -341,6 +351,10 @@ function(ly_add_editor_python_test) file(REAL_PATH ${ly_add_editor_python_test_TEST_PROJECT} project_real_path BASE_DIRECTORY ${LY_ROOT_FOLDER}) + # Set the name of the current test target for storage in the global list + set(LY_ALL_TESTS_TARGET_NAME ${ly_add_editor_python_test_NAME}) + # Add the script path to the test target params + set(LY_TEST_PARAMS "${ly_add_editor_python_test_PATH}") # Run test via the run_epbtest.cmake script. # Parameters used are explained in run_epbtest.cmake. ly_add_test( @@ -362,8 +376,6 @@ function(ly_add_editor_python_test) TIMEOUT ${ly_add_editor_python_test_TIMEOUT} COMPONENT ${ly_add_editor_python_test_COMPONENT} ) - - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_editor_python_test_NAME}_SCRIPT_PATH ${ly_add_editor_python_test_PATH}) set_tests_properties(${LY_ADDED_TEST_NAME} PROPERTIES RUN_SERIAL "${ly_add_editor_python_test_TEST_SERIAL}") endfunction() @@ -431,18 +443,23 @@ function(ly_add_googletest) set(full_test_command $ $ AzRunUnitTests) # Add AzTestRunner as a build dependency ly_add_dependencies(${build_target} AZ::AzTestRunner) + # Start the test target params and dd the command runner command # Ideally, we would populate the full command procedurally but the generator expressions won't be expanded by the time we need this data - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_googletest_NAME}_TEST_COMMAND "AzRunUnitTests") + set(LY_TEST_PARAMS "AzRunUnitTests") else() set(full_test_command ${ly_add_googletest_TEST_COMMAND}) # Remove the generator expressions so we are left with the argument(s) required to run unit tests for executable targets string(REPLACE ";" "" stripped_test_command ${full_test_command}) string(GENEX_STRIP ${stripped_test_command} stripped_test_command) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_googletest_NAME}_TEST_COMMAND ${stripped_test_command}) + # Start the test target params and dd the command runner command + set(LY_TEST_PARAMS "${stripped_test_command}") endif() string(REPLACE "::" "_" report_directory "${GTEST_XML_OUTPUT_DIR}/${ly_add_googletest_NAME}.xml") + # Set the name of the current test target for storage in the global list + set(LY_ALL_TESTS_TARGET_NAME ${target_name}) + # Invoke the lower level ly_add_test command to add the actual ctest and setup the test labels to add_dependencies on the target ly_add_test( NAME ${ly_add_googletest_NAME} @@ -520,16 +537,21 @@ function(ly_add_googlebenchmark) # If command is not supplied attempts, uses the AzTestRunner to run googlebenchmarks on the supplied TARGET set(full_test_command $ $ AzRunBenchmarks ${output_format_args}) + # Start the test target params and dd the command runner command # Ideally, we would populate the full command procedurally but the generator expressions won't be expanded by the time we need this data - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_googlebenchmark_NAME}_TEST_COMMAND "AzRunUnitTests") + set(LY_TEST_PARAMS "AzRunUnitTests") else() set(full_test_command ${ly_add_googlebenchmark_TEST_COMMAND}) # Remove the generator expressions so we are left with the argument(s) required to run unit tests for executable targets string(REPLACE ";" "" stripped_test_command ${full_test_command}) string(GENEX_STRIP ${stripped_test_command} stripped_test_command) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${ly_add_googletest_NAME}_TEST_COMMAND ${stripped_test_command}) + # Start the test target params and dd the command runner command + set(LY_TEST_PARAMS "${stripped_test_command}") endif() - + + # Set the name of the current test target for storage in the global list + set(LY_ALL_TESTS_TARGET_NAME ${ly_add_googlebenchmark_NAME}) + # Set the name of the current test target for storage in the global list ly_add_test( NAME ${ly_add_googlebenchmark_NAME} TEST_REQUIRES ${ly_add_googlebenchmark_TEST_REQUIRES} diff --git a/cmake/TestImpactFramework/ConsoleFrontendConfig.in b/cmake/TestImpactFramework/ConsoleFrontendConfig.in index f6af3d801d..357499771c 100644 --- a/cmake/TestImpactFramework/ConsoleFrontendConfig.in +++ b/cmake/TestImpactFramework/ConsoleFrontendConfig.in @@ -4,7 +4,8 @@ "timestamp": "${timestamp}" }, "repo": { - "root": "${repo_dir}" + "root": "${repo_dir}", + "tiaf_bin": "${tiaf_bin}" }, "workspace": { "temp": { @@ -13,11 +14,23 @@ "artifact_dir": "RuntimeArtifact" } }, - "persistent": { - "root": "${persistent_dir}", + "active": { + "root": "${active_dir}", "relative_paths": { - "test_impact_data_file": "TestImpactData.spartia", - "enumeration_cache_dir": "EnumerationCache" + "test_impact_data_files": { + "main": "TestImpactData.main.spartia", + "periodic": "TestImpactData.periodic.spartia", + "sandbox": "TestImpactData.sandbox.spartia" + }, + "enumeration_cache_dir": "EnumerationCache", + "last_build_target_list_file": "LastRunBuildTargets.json" + } + }, + "historic": { + "root": "${historic_dir}", + "relative_paths": { + "last_run_hash_file": "last_run.hash", + "last_build_target_list_file": "LastRunBuildTargets.json" } } }, diff --git a/cmake/TestImpactFramework/LYTestImpactFramework.cmake b/cmake/TestImpactFramework/LYTestImpactFramework.cmake index 3081a4c89c..54dadbd8ad 100644 --- a/cmake/TestImpactFramework/LYTestImpactFramework.cmake +++ b/cmake/TestImpactFramework/LYTestImpactFramework.cmake @@ -89,7 +89,7 @@ function(ly_test_impact_get_test_launch_method TARGET_NAME LAUNCH_METHOD) elseif("${target_type}" STREQUAL "EXECUTABLE") set(${LAUNCH_METHOD} "stand_alone" PARENT_SCOPE) else() - message(FATAL_ERROR "Cannot deduce test target launch method for the target type ${target_type}") + message(FATAL_ERROR "Cannot deduce test target launch method for the target ${TARGET_NAME} with type ${target_type}") endif() endfunction() @@ -131,33 +131,50 @@ function(ly_test_impact_extract_python_test COMPOSITE_TEST TEST_NAME) set(${TEST_NAME} ${test_name} PARENT_SCOPE) endfunction() -#! ly_test_impact_extract_google_test_params: extracts the google test name and command parameters. +#! ly_test_impact_extract_google_test_params: extracts the suites for the given google test. # # \arg:COMPOSITE_TEST test in the form 'namespace::test' +# \arg:COMPOSITE_SUITES composite list of suites for this target # \arg:TEST_NAME name of test -# \arg:TEST_COMMAND optional command arguments to run the test -function(ly_test_impact_extract_google_test_params COMPOSITE_TEST TEST_NAME TEST_COMMAND) - get_property(test_command GLOBAL PROPERTY LY_ALL_TESTS_${COMPOSITE_TEST}_TEST_COMMAND) +# \arg:TEST_SUITES extracted list of suites for this target in JSON format +function(ly_test_impact_extract_google_test_params COMPOSITE_TEST COMPOSITE_SUITES TEST_NAME TEST_SUITES) # Namespace and test are mandatory string(REPLACE "::" ";" test_components ${COMPOSITE_TEST}) list(LENGTH test_components num_test_components) if(num_test_components LESS 2) message(FATAL_ERROR "The test ${test_components} appears to have been specified without a namespace, i.e.:\ly_add_googletest/benchmark(NAME ${test_components})\nInstead of (perhaps):\ly_add_googletest/benchmark(NAME Gem::${test_components})\nPlease add the missing namespace before proceeding.") endif() - + list(GET test_components 0 test_namespace) list(GET test_components 1 test_name) set(${TEST_NAMESPACE} ${test_namespace} PARENT_SCOPE) set(${TEST_NAME} ${test_name} PARENT_SCOPE) - set(${TEST_COMMAND} ${test_command} PARENT_SCOPE) + + set(test_suites "") + foreach(composite_suite ${COMPOSITE_SUITES}) + # Command, suite, timeout + string(REPLACE "#" ";" suite_components ${composite_suite}) + list(LENGTH suite_components num_suite_components) + if(num_suite_components LESS 3) + message(FATAL_ERROR "The suite components ${composite_suite} are required to be in the following format: command#suite#string.") + endif() + list(GET suite_components 0 test_command) + list(GET suite_components 1 test_suite) + list(GET suite_components 2 test_timeout) + set(suite_params "{ \"suite\": \"${test_suite}\", \"command\": \"${test_command}\", \"timeout\": ${test_timeout} }") + list(APPEND test_suites "${suite_params}") + endforeach() + string(REPLACE ";" ", " test_suites "${test_suites}") + set(${TEST_SUITES} ${test_suites} PARENT_SCOPE) endfunction() #! ly_test_impact_extract_python_test_params: extracts the python test name and relative script path parameters. # # \arg:COMPOSITE_TEST test in form 'namespace::test' or 'test' +# \arg:COMPOSITE_SUITES composite list of suites for this target # \arg:TEST_NAME name of test -# \arg:SCRIPT_PATH name of test -function(ly_test_impact_extract_python_test_params COMPOSITE_TEST TEST_NAME SCRIPT_PATH) +# \arg:TEST_SUITES extracted list of suites for this target in JSON format +function(ly_test_impact_extract_python_test_params COMPOSITE_TEST COMPOSITE_SUITES TEST_NAME TEST_SUITES) get_property(script_path GLOBAL PROPERTY LY_ALL_TESTS_${COMPOSITE_TEST}_SCRIPT_PATH) # namespace is optional, in which case this component will be simply the test name @@ -169,15 +186,30 @@ function(ly_test_impact_extract_python_test_params COMPOSITE_TEST TEST_NAME SCRI set(test_name ${test_components}) endif() - # Get python script path relative to repo root - ly_test_impact_rebase_file_to_repo_root( - ${script_path} - script_path - ${LY_ROOT_FOLDER} - ) - set(${TEST_NAME} ${test_name} PARENT_SCOPE) - set(${SCRIPT_PATH} ${script_path} PARENT_SCOPE) + + set(test_suites "") + foreach(composite_suite ${COMPOSITE_SUITES}) + # Script path, suite, timeout + string(REPLACE "#" ";" suite_components ${composite_suite}) + list(LENGTH suite_components num_suite_components) + if(num_suite_components LESS 3) + message(FATAL_ERROR "The suite components ${composite_suite} are required to be in the following format: script_path#suite#string.") + endif() + list(GET suite_components 0 script_path) + list(GET suite_components 1 test_suite) + list(GET suite_components 2 test_timeout) + # Get python script path relative to repo root + ly_test_impact_rebase_file_to_repo_root( + ${script_path} + script_path + ${LY_ROOT_FOLDER} + ) + set(suite_params "{ \"suite\": \"${test_suite}\", \"script\": \"${script_path}\", \"timeout\": ${test_timeout} }") + list(APPEND test_suites "${suite_params}") + endforeach() + string(REPLACE ";" ", " test_suites "${test_suites}") + set(${TEST_SUITES} ${test_suites} PARENT_SCOPE) endfunction() #! ly_test_impact_write_test_enumeration_file: exports the master test lists to file. @@ -185,7 +217,6 @@ endfunction() # \arg:TEST_ENUMERATION_TEMPLATE_FILE path to test enumeration template file function(ly_test_impact_write_test_enumeration_file TEST_ENUMERATION_TEMPLATE_FILE) get_property(LY_ALL_TESTS GLOBAL PROPERTY LY_ALL_TESTS) - # Enumerated tests for each type set(google_tests "") set(google_benchmarks "") @@ -196,29 +227,28 @@ function(ly_test_impact_write_test_enumeration_file TEST_ENUMERATION_TEMPLATE_FI # Walk the test list foreach(test ${LY_ALL_TESTS}) message(TRACE "Parsing ${test}") + get_property(test_params GLOBAL PROPERTY LY_ALL_TESTS_${test}_PARAMS) get_property(test_type GLOBAL PROPERTY LY_ALL_TESTS_${test}_TEST_LIBRARY) - get_property(test_suite GLOBAL PROPERTY LY_ALL_TESTS_${test}_TEST_SUITE) - get_property(test_timeout GLOBAL PROPERTY LY_ALL_TESTS_${test}_TEST_TIMEOUT) if("${test_type}" STREQUAL "pytest") # Python tests - ly_test_impact_extract_python_test_params(${test} test_name script_path) - list(APPEND python_tests " { \"name\": \"${test_name}\", \"suite\": \"${test_suite}\", \"script\": \"${script_path}\", \"timeout\":${test_timeout} }") + ly_test_impact_extract_python_test_params(${test} "${test_params}" test_name test_suites) + list(APPEND python_tests " { \"name\": \"${test_name}\", \"suites\": [${test_suites}] }") elseif("${test_type}" STREQUAL "pytest_editor") - # Python editor tests - ly_test_impact_extract_python_test_params(${test} test_name script_path) - list(APPEND python_editor_tests " { \"name\": \"${test_name}\", \"suite\": \"${test_suite}\", \"script\": \"${script_path}\", \"timeout\":${test_timeout} }") + # Python editor tests + ly_test_impact_extract_python_test_params(${test} "${test_params}" test_name test_suites) + list(APPEND python_editor_tests " { \"name\": \"${test_name}\", \"suites\": [${test_suites}] }") elseif("${test_type}" STREQUAL "googletest") # Google tests - ly_test_impact_extract_google_test_params(${test} test_name test_command) - ly_test_impact_get_test_launch_method(${test_name} launch_method) - list(APPEND google_tests " { \"name\": \"${test_name}\", \"suite\": \"${test_suite}\", \"command\": \"${test_command}\", \"timeout\":${test_timeout}, \"launch_method\": \"${launch_method}\" }") + ly_test_impact_extract_google_test_params(${test} "${test_params}" test_name test_suites) + ly_test_impact_get_test_launch_method(${test} launch_method) + list(APPEND google_tests " { \"name\": \"${test_name}\", \"launch_method\": \"${launch_method}\", \"suites\": [${test_suites}] }") elseif("${test_type}" STREQUAL "googlebenchmark") # Google benchmarks - ly_test_impact_extract_google_test_params(${test} test_name test_command) - list(APPEND google_benchmarks " { \"name\": \"${test_name}\", \"suite\": \"${test_suite}\", \"command\": \"${test_command}\", \"timeout\":${test_timeout} }") + ly_test_impact_extract_google_test_params(${test} "${test_params}" test_name test_suites) + list(APPEND google_benchmarks " { \"name\": \"${test_name}\", \"launch_method\": \"${launch_method}\", \"suites\": [${test_suites}] }") else() message("${test_name} is of unknown type (TEST_LIBRARY property is empty)") - list(APPEND unknown_tests " { \"name\": \"${test}\" }") + list(APPEND unknown_tests " { \"name\": \"${test}\", \"type\": \"${test_type}\" }") endif() endforeach() @@ -330,8 +360,11 @@ function(ly_test_impact_write_config_file CONFIG_TEMPLATE_FILE PERSISTENT_DATA_D # Temp dir set(temp_dir "${LY_TEST_IMPACT_TEMP_DIR}") - # Persistent dir - set(persistent_dir "${PERSISTENT_DATA_DIR}") + # Active persistent data dir + set(active_dir "${PERSISTENT_DATA_DIR}/active") + + # Historic persistent data dir + set(historic_dir "${PERSISTENT_DATA_DIR}/historic") # Source to target mappings dir set(source_target_mapping_dir "${LY_TEST_IMPACT_SOURCE_TARGET_MAPPING_DIR}") @@ -341,6 +374,9 @@ function(ly_test_impact_write_config_file CONFIG_TEMPLATE_FILE PERSISTENT_DATA_D # Build dependency artifact dir set(target_dependency_dir "${LY_TEST_IMPACT_TARGET_DEPENDENCY_DIR}") + + # Test impact analysis framework binary + set(tiaf_bin "$") # Substitute config file template with above vars file(READ "${CONFIG_TEMPLATE_FILE}" config_file) @@ -348,9 +384,13 @@ function(ly_test_impact_write_config_file CONFIG_TEMPLATE_FILE PERSISTENT_DATA_D # Write out entire config contents to a file in the build directory of the test impact framework console target file(GENERATE - OUTPUT "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/$/$.$.json" + OUTPUT "${PERSISTENT_DATA_DIR}/$.$.json" CONTENT ${config_file} ) + + # Set the above config file as the default config file to use for the test impact framework console target + target_compile_definitions(${LY_TEST_IMPACT_CONSOLE_STATIC_TARGET} PUBLIC "LY_TEST_IMPACT_DEFAULT_CONFIG_FILE=\"${PERSISTENT_DATA_DIR}/$.$.json\"") + message(DEBUG "Test impact framework post steps complete") endfunction() #! ly_test_impact_post_step: runs the post steps to be executed after all other cmake scripts have been executed. @@ -360,8 +400,7 @@ function(ly_test_impact_post_step) endif() # Directory per build config for persistent test impact data (to be checked in) - set(persistent_data_dir "${LY_ROOT_FOLDER}/Tests/test_impact_framework/${CMAKE_SYSTEM_NAME}/$") - + set(persistent_data_dir "${LY_TEST_IMPACT_WORKING_DIR}/persistent") # Directory for binaries built for this profile set(bin_dir "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/$") @@ -388,8 +427,4 @@ function(ly_test_impact_post_step) # Copy over the graphviz options file for the build dependency graphs message(DEBUG "Test impact framework config file written") file(COPY "cmake/TestImpactFramework/CMakeGraphVizOptions.cmake" DESTINATION ${CMAKE_BINARY_DIR}) - - # Set the above config file as the default config file to use for the test impact framework console target - target_compile_definitions(${LY_TEST_IMPACT_CONSOLE_STATIC_TARGET} PUBLIC "LY_TEST_IMPACT_DEFAULT_CONFIG_FILE=\"${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/$/$.$.json\"") - message(DEBUG "Test impact framework post steps complete") endfunction() From b00f56909027667fe99fc6e713175f91f401c128 Mon Sep 17 00:00:00 2001 From: jonawals Date: Sun, 6 Jun 2021 19:09:24 +0100 Subject: [PATCH 02/14] Address PR comments --- .../Code/Source/TestImpactConsoleMain.cpp | 2 +- cmake/LYTestWrappers.cmake | 32 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp index fab6509244..4853c6da15 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp @@ -121,7 +121,7 @@ namespace TestImpact } else { - // One sequence timed out whilst the other sequence succeeded or both sequences timed out + // One or both sequences timed out or failed result = TestSequenceResult::Timeout; } } diff --git a/cmake/LYTestWrappers.cmake b/cmake/LYTestWrappers.cmake index 3308c4411a..eebc281d06 100644 --- a/cmake/LYTestWrappers.cmake +++ b/cmake/LYTestWrappers.cmake @@ -71,6 +71,7 @@ endfunction() #! ly_add_test: Adds a new RUN_TEST using for the specified target using the supplied command # # \arg:NAME - Name to for the test run target +# \arg:PARENT_NAME(optional) - Name of the parent test run target (if this is a subsequent call to specify a suite) # \arg:TEST_REQUIRES(optional) - List of system resources that are required to run this test. # Only available option is "gpu" # \arg:TEST_SUITE(optional) - "smoke" or "periodic" or "sandbox" - prevents the test from running normally @@ -94,7 +95,7 @@ endfunction() # sets LY_ADDED_TEST_NAME to the fully qualified name of the test, in parent scope function(ly_add_test) set(options EXCLUDE_TEST_RUN_TARGET_FROM_IDE) - set(one_value_args NAME TEST_LIBRARY TEST_SUITE TIMEOUT) + set(one_value_args NAME PARENT_NAME TEST_LIBRARY TEST_SUITE TIMEOUT) set(multi_value_args TEST_REQUIRES TEST_COMMAND NON_IDE_PARAMS RUNTIME_DEPENDENCIES COMPONENT LABELS) # note that we dont use TEST_LIBRARY here, but PAL files might so do not remove! @@ -241,18 +242,24 @@ function(ly_add_test) endif() + if(NOT ly_add_test_PARENT_NAME) + set(test_target ${ly_add_test_NAME}) + else() + set(test_target ${ly_add_test_PARENT_NAME}) + endif() + # Check to see whether or not this test target has been stored in the global list for walking by the test impact analysis framework get_property(all_tests GLOBAL PROPERTY LY_ALL_TESTS) - if(NOT "${LY_ALL_TESTS_TARGET_NAME}" IN_LIST all_tests) + if(NOT "${test_target}" IN_LIST all_tests) # This is the first reference to this test target so add it to the global list - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS ${LY_ALL_TESTS_TARGET_NAME}) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${LY_ALL_TESTS_TARGET_NAME}_TEST_LIBRARY ${ly_add_test_TEST_LIBRARY}) + set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS ${test_target}) + set_property(GLOBAL PROPERTY LY_ALL_TESTS_${test_target}_TEST_LIBRARY ${ly_add_test_TEST_LIBRARY}) endif() # Add the test suite and timeout value to the test target params set(LY_TEST_PARAMS "${LY_TEST_PARAMS}#${ly_add_test_TEST_SUITE}") set(LY_TEST_PARAMS "${LY_TEST_PARAMS}#${ly_add_test_TIMEOUT}") # Store the params for this test target - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${LY_ALL_TESTS_TARGET_NAME}_PARAMS ${LY_TEST_PARAMS}) + set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${test_target}_PARAMS ${LY_TEST_PARAMS}) endfunction() #! ly_add_pytest: registers target PyTest-based test with CTest @@ -294,13 +301,12 @@ function(ly_add_pytest) string(REPLACE "::" "_" pytest_report_directory "${PYTEST_XML_OUTPUT_DIR}/${ly_add_pytest_NAME}.xml") - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${ly_add_pytest_NAME}) # Add the script path to the test target params set(LY_TEST_PARAMS "${ly_add_pytest_PATH}") ly_add_test( NAME ${ly_add_pytest_NAME} + PARENT_NAME ${ly_add_pytest_NAME} TEST_SUITE ${ly_add_pytest_TEST_SUITE} LABELS FRAMEWORK_pytest TEST_COMMAND ${LY_PYTEST_EXECUTABLE} ${ly_add_pytest_PATH} ${ly_add_pytest_EXTRA_ARGS} --junitxml=${pytest_report_directory} ${custom_marks_args} @@ -351,14 +357,13 @@ function(ly_add_editor_python_test) file(REAL_PATH ${ly_add_editor_python_test_TEST_PROJECT} project_real_path BASE_DIRECTORY ${LY_ROOT_FOLDER}) - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${ly_add_editor_python_test_NAME}) # Add the script path to the test target params set(LY_TEST_PARAMS "${ly_add_editor_python_test_PATH}") # Run test via the run_epbtest.cmake script. # Parameters used are explained in run_epbtest.cmake. ly_add_test( NAME ${ly_add_editor_python_test_NAME} + PARENT_NAME ${ly_add_editor_python_test_NAME} TEST_REQUIRES ${ly_add_editor_python_test_TEST_REQUIRES} TEST_COMMAND ${CMAKE_COMMAND} -DCMD_ARG_TEST_PROJECT=${project_real_path} @@ -457,12 +462,10 @@ function(ly_add_googletest) string(REPLACE "::" "_" report_directory "${GTEST_XML_OUTPUT_DIR}/${ly_add_googletest_NAME}.xml") - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${target_name}) - # Invoke the lower level ly_add_test command to add the actual ctest and setup the test labels to add_dependencies on the target ly_add_test( NAME ${ly_add_googletest_NAME} + PARENT_NAME ${target_name} TEST_SUITE ${ly_add_googletest_TEST_SUITE} LABELS FRAMEWORK_googletest TEST_COMMAND ${full_test_command} --gtest_output=xml:${report_directory} ${LY_GOOGLETEST_EXTRA_PARAMS} @@ -548,12 +551,11 @@ function(ly_add_googlebenchmark) # Start the test target params and dd the command runner command set(LY_TEST_PARAMS "${stripped_test_command}") endif() - - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${ly_add_googlebenchmark_NAME}) + # Set the name of the current test target for storage in the global list ly_add_test( NAME ${ly_add_googlebenchmark_NAME} + PARENT_NAME ${ly_add_googlebenchmark_NAME} TEST_REQUIRES ${ly_add_googlebenchmark_TEST_REQUIRES} TEST_COMMAND ${full_test_command} ${LY_GOOGLETEST_EXTRA_PARAMS} TEST_SUITE "benchmark" From 4ed0c7b1d8ccaf70bec71674763dc0dd65ad5f9f Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 08:52:20 +0100 Subject: [PATCH 03/14] Add drafting of failed tests --- .../Source/TestImpactCommandLineOptions.cpp | 50 ++++++++++-------- .../Source/TestImpactCommandLineOptions.h | 6 +-- .../Code/Source/TestImpactConsoleMain.cpp | 2 +- .../TestImpactFramework/TestImpactRuntime.h | 7 +-- .../TestImpactTestSequence.h | 8 +-- .../TestImpactDynamicDependencyMap.cpp | 48 +++++++++++------ .../TestImpactDynamicDependencyMap.h | 9 ++-- .../Runtime/Code/Source/TestImpactRuntime.cpp | 51 ++++++++++++++----- 8 files changed, 119 insertions(+), 62 deletions(-) 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 5ff070601a..aad9319cf9 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp @@ -28,7 +28,7 @@ namespace TestImpact Sequence, TestPrioritizationPolicy, ExecutionFailurePolicy, - ExecutionFailureDraftingPolicy, + FailedTestCoveragePolicy, TestFailurePolicy, IntegrityFailurePolicy, TestShardingPolicy, @@ -50,7 +50,9 @@ namespace TestImpact Continue, Ignore, StdOut, - File + File, + Remove, + Keep }; constexpr const char* OptionKeys[] = @@ -62,7 +64,7 @@ namespace TestImpact "sequence", "ppolicy", "epolicy", - "rexecfailures", + "cpolicy", "fpolicy", "ipolicy", "shard", @@ -84,7 +86,9 @@ namespace TestImpact "continue", "ignore", "stdout", - "file" + "file", + "remove", + "keep" }; RepoPath ParseConfigurationFile(const AZ::CommandLine& cmd) @@ -139,15 +143,15 @@ namespace TestImpact return ParseMultiStateOption(OptionKeys[ExecutionFailurePolicy], states, cmd).value_or(Policy::ExecutionFailure::Continue); } - Policy::ExecutionFailureDrafting ParseExecutionFailureDraftingPolicy(const AZ::CommandLine& cmd) + Policy::FailedTestCoverage ParseFailedTestCoveragePolicy(const AZ::CommandLine& cmd) { - const BinaryStateValue states = + const AZStd::vector> states = { - Policy::ExecutionFailureDrafting::Never, - Policy::ExecutionFailureDrafting::Always + {OptionKeys[Remove], Policy::FailedTestCoverage::Remove}, + {OptionKeys[Keep], Policy::FailedTestCoverage::Keep} }; - return ParseOnOffOption(OptionKeys[ExecutionFailureDraftingPolicy], states, cmd).value_or(Policy::ExecutionFailureDrafting::Always); + return ParseMultiStateOption(OptionKeys[FailedTestCoveragePolicy], states, cmd).value_or(Policy::FailedTestCoverage::Keep); } Policy::TestFailure ParseTestFailurePolicy(const AZ::CommandLine& cmd) @@ -275,7 +279,7 @@ namespace TestImpact m_testSequenceType = ParseTestSequenceType(cmd); m_testPrioritizationPolicy = ParseTestPrioritizationPolicy(cmd); m_executionFailurePolicy = ParseExecutionFailurePolicy(cmd); - m_executionFailureDraftingPolicy = ParseExecutionFailureDraftingPolicy(cmd); + m_failedTestCoveragePolicy = ParseFailedTestCoveragePolicy(cmd); m_testFailurePolicy = ParseTestFailurePolicy(cmd); m_integrityFailurePolicy = ParseIntegrityFailurePolicy(cmd); m_testShardingPolicy = ParseTestShardingPolicy(cmd); @@ -327,9 +331,9 @@ namespace TestImpact return m_executionFailurePolicy; } - Policy::ExecutionFailureDrafting CommandLineOptions::GetExecutionFailureDraftingPolicy() const + Policy::FailedTestCoverage CommandLineOptions::GetFailedTestCoveragePolicy() const { - return m_executionFailureDraftingPolicy; + return m_failedTestCoveragePolicy; } Policy::TestFailure CommandLineOptions::GetTestFailurePolicy() const @@ -401,7 +405,11 @@ namespace TestImpact " 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" - " -rexecfailures= Attempt to execute test targets that previously failed to execute.\n" + " -cpolicy= Policy for handling the coverage data of failed tests (both test 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" @@ -409,25 +417,25 @@ namespace TestImpact " -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 failureand report a failure(along with the return \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 failuresand treat the test targets that failed\n" - " to launch as as test passes(along with the return codes of the test \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 sequenceupon the first test \n" - " failureand report a failure and continue will continue with the test\n" - " sequence in the event of test failuresand report the test failures.\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 regularand seed sequence \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 abortand report a failure(this option has \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" diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h index 99b1b98dba..b215261a87 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.h @@ -64,8 +64,8 @@ namespace TestImpact //! Returns the test execution failure policy to use. Policy::ExecutionFailure GetExecutionFailurePolicy() const; - //! Returns the test historic test execution failure drafting policy to use. - Policy::ExecutionFailureDrafting GetExecutionFailureDraftingPolicy() const; + //! Returns failed test coverage drafting policy to use. + Policy::FailedTestCoverage GetFailedTestCoveragePolicy() const; //! Returns the test failure policy to use. Policy::TestFailure GetTestFailurePolicy() const; @@ -98,7 +98,7 @@ namespace TestImpact TestSequenceType m_testSequenceType; Policy::TestPrioritization m_testPrioritizationPolicy = Policy::TestPrioritization::None; Policy::ExecutionFailure m_executionFailurePolicy = Policy::ExecutionFailure::Continue; - Policy::ExecutionFailureDrafting m_executionFailureDraftingPolicy = Policy::ExecutionFailureDrafting::Always; + Policy::FailedTestCoverage m_failedTestCoveragePolicy = Policy::FailedTestCoverage::Keep; Policy::TestFailure m_testFailurePolicy = Policy::TestFailure::Abort; Policy::IntegrityFailure m_integrityFailurePolicy = Policy::IntegrityFailure::Abort; Policy::TestSharding m_testShardingPolicy = Policy::TestSharding::Never; diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp index 4853c6da15..77b1d98b3c 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp @@ -205,7 +205,7 @@ namespace TestImpact RuntimeConfigurationFactory(ReadFileContents(options.GetConfigurationFile())), options.GetSuiteFilter(), options.GetExecutionFailurePolicy(), - options.GetExecutionFailureDraftingPolicy(), + options.GetFailedTestCoveragePolicy(), options.GetTestFailurePolicy(), options.GetIntegrityFailurePolicy(), options.GetTestShardingPolicy(), diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h index ae1f78c5c7..86f2907ab7 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h @@ -35,6 +35,7 @@ namespace TestImpact class TestEngine; class TestTarget; class SourceCoveringTestsList; + class TestEngineInstrumentedRun; //! Callback for a test sequence that isn't using test impact analysis to determine selected tests. //! @param tests The tests that will be run for this sequence. @@ -102,7 +103,7 @@ namespace TestImpact RuntimeConfig&& config, SuiteType suiteFilter, Policy::ExecutionFailure executionFailurePolicy, - Policy::ExecutionFailureDrafting executionFailureDraftingPolicy, + Policy::FailedTestCoverage failedTestCoveragePolicy, Policy::TestFailure testFailurePolicy, Policy::IntegrityFailure integrationFailurePolicy, Policy::TestSharding testShardingPolicy, @@ -204,13 +205,13 @@ namespace TestImpact void ClearDynamicDependencyMapAndRemoveExistingFile(); //! Updates the dynamic dependency map and serializes the entire map to disk. - void UpdateAndSerializeDynamicDependencyMap(const SourceCoveringTestsList& sourceCoverageTestsList); + void UpdateAndSerializeDynamicDependencyMap(const AZStd::vector& jobs); RuntimeConfig m_config; SuiteType m_suiteFilter; RepoPath m_sparTIAFile; Policy::ExecutionFailure m_executionFailurePolicy; - Policy::ExecutionFailureDrafting m_executionFailureDraftingPolicy; + Policy::FailedTestCoverage m_failedTestCoveragePolicy; Policy::TestFailure m_testFailurePolicy; Policy::IntegrityFailure m_integrationFailurePolicy; Policy::TestSharding m_testShardingPolicy; diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h index 4e8908cf3f..5f9bff952f 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h @@ -31,11 +31,11 @@ namespace TestImpact Ignore //!< Continue the test sequence and ignore the execution failures. }; - //! Policy for reattempting the execution of test targets that failed to execute in previous runs. - enum class ExecutionFailureDrafting + //! 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 { - Never, //!< Do not attempt to execute historic execution failures. - Always //!< Reattempt the exectution of historic execution failures. + Remove, //!< Remove the failed test targets from the all coverage data (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. }; //! Policy for prioritizing selected tests. diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp index ece5a7c13b..22bcc97c2a 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.cpp @@ -56,7 +56,7 @@ namespace TestImpact for (const auto& target : m_testTargets.GetTargets()) { mapBuildTargetSources(&target); - m_testTargetSourceCoverageCount[&target] = 0; + m_testTargetSourceCoverage[&target] = {}; } } @@ -144,18 +144,15 @@ namespace TestImpact sourceCoverage.GetPath().c_str()).c_str()); auto [sourceDependencyIt, inserted] = m_sourceDependencyMap.insert(sourceCoverage.GetPath().String()); - auto& [key, sourceDependency] = *sourceDependencyIt; + auto& [source, sourceDependency] = *sourceDependencyIt; - // Knock down the source coverage count for the test targets and clear any existing coverage for the delta + // Remove the source from the test target covering sources map and clear any existing coverage for the delta for (const auto& testTarget : sourceDependency.m_coveringTestTargets) { - if (auto coveringTestTargetIt = m_testTargetSourceCoverageCount.find(testTarget); - coveringTestTargetIt != m_testTargetSourceCoverageCount.end()) + if (auto coveringTestTargetIt = m_testTargetSourceCoverage.find(testTarget); + coveringTestTargetIt != m_testTargetSourceCoverage.end()) { - if (coveringTestTargetIt->second > 0) - { - coveringTestTargetIt->second--; - } + coveringTestTargetIt->second.erase(source); } } sourceDependency.m_coveringTestTargets.clear(); @@ -169,8 +166,8 @@ namespace TestImpact // Source to covering test target mapping sourceDependency.m_coveringTestTargets.insert(testTarget); - // Test target covering sources count - m_testTargetSourceCoverageCount[testTarget]++; + // Add the source to the test target covering sources map + m_testTargetSourceCoverage[testTarget].insert(source); // Build target to covering test target mapping for (const auto& parentTarget : sourceDependency.m_parentTargets) @@ -429,12 +426,33 @@ namespace TestImpact return ChangeDependencyList(AZStd::move(createDependencies), AZStd::move(updateDependencies), AZStd::move(deleteDependencies)); } + void DynamicDependencyMap::RemoveTestTargetFromSourceCoverage(const TestTarget* testTarget) + { + if (const auto& it = m_testTargetSourceCoverage.find(testTarget); + it != m_testTargetSourceCoverage.end()) + { + for (const auto& source : it->second) + { + const auto sourceDependency = m_sourceDependencyMap.find(source); + AZ_TestImpact_Eval( + sourceDependency != m_sourceDependencyMap.end(), + DependencyException, + AZStd::string::format("Test target '%s' has covering source '%s' yet cannot be found in the dependency map", + testTarget->GetName().c_str(), source.c_str())); + + sourceDependency->second.m_coveringTestTargets.erase(testTarget); + } + + m_testTargetSourceCoverage.erase(testTarget); + } + } + AZStd::vector DynamicDependencyMap::GetCoveringTests() const { AZStd::vector covering; - for (const auto& [testTarget, coveringSources] : m_testTargetSourceCoverageCount) + for (const auto& [testTarget, coveringSources] : m_testTargetSourceCoverage) { - if (coveringSources > 0) + if (!coveringSources.empty()) { covering.push_back(testTarget); } @@ -446,9 +464,9 @@ namespace TestImpact AZStd::vector DynamicDependencyMap::GetNotCoveringTests() const { AZStd::vector notCovering; - for(const auto& [testTarget, coveringSources] : m_testTargetSourceCoverageCount) + for(const auto& [testTarget, coveringSources] : m_testTargetSourceCoverage) { - if(coveringSources == 0) + if (coveringSources.empty()) { notCovering.push_back(testTarget); } diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h index f209e22823..cd01a1a728 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/Dependency/TestImpactDynamicDependencyMap.h @@ -100,6 +100,9 @@ namespace TestImpact //! @returns The change list as resolved to the appropriate source dependencies. [[nodiscard]] ChangeDependencyList ApplyAndResoveChangeList(const ChangeList& changeList); + //! Removes the specified test target from all source coverage. + void RemoveTestTargetFromSourceCoverage(const TestTarget* testTarget); + //! Returns the test targets that cover one or more sources in the repository. AZStd::vector GetCoveringTests() const; @@ -120,13 +123,13 @@ namespace TestImpact //! The dependency map of sources to their parent build targets and covering test targets. AZStd::unordered_map m_sourceDependencyMap; + //! Map of all test targets and the sources they cover. + AZStd::unordered_map> m_testTargetSourceCoverage; + //! The map of build targets and their covering test targets. AZStd::unordered_map> m_buildTargetCoverage; //! Mapping of autogen input sources to their generated output sources. AZStd::unordered_map> m_autogenInputToOutputMap; - - //! Number of sources that each test target in the repository covers. - AZStd::unordered_map m_testTargetSourceCoverageCount; }; } // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp index 2331c76880..44657dad21 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -70,11 +70,20 @@ namespace TestImpact }; } + template + AZStd::vector ConcatenateVectors(const AZStd::vector& v1, const AZStd::vector& v2) + { + AZStd::vector result; + result.reserve(v1.size() + v2.size()); + result.insert(result.end(), v1.begin(), v1.end()); + result.insert(result.end(), v2.begin(), v2.end()); + return result; + } Runtime::Runtime( RuntimeConfig&& config, SuiteType suiteFilter, Policy::ExecutionFailure executionFailurePolicy, - Policy::ExecutionFailureDrafting executionFailureDraftingPolicy, + Policy::FailedTestCoverage failedTestCoveragePolicy, Policy::TestFailure testFailurePolicy, Policy::IntegrityFailure integrationFailurePolicy, Policy::TestSharding testShardingPolicy, @@ -83,7 +92,7 @@ namespace TestImpact : m_config(AZStd::move(config)) , m_suiteFilter(suiteFilter) , m_executionFailurePolicy(executionFailurePolicy) - , m_executionFailureDraftingPolicy(executionFailureDraftingPolicy) + , m_failedTestCoveragePolicy(failedTestCoveragePolicy) , m_testFailurePolicy(testFailurePolicy) , m_integrationFailurePolicy(integrationFailurePolicy) , m_testShardingPolicy(testShardingPolicy) @@ -145,7 +154,7 @@ namespace TestImpact { AZ_Printf("TestImpactRuntime", AZStd::string::format( - "No test impact analysis data found for suite '%s' at %s", GetSuiteTypeName(m_suiteFilter).c_str(), m_sparTIAFile.c_str()).c_str()); + "No test impact analysis data found for suite '%s' at %s\n", GetSuiteTypeName(m_suiteFilter).c_str(), m_sparTIAFile.c_str()).c_str()); } } @@ -249,14 +258,28 @@ namespace TestImpact m_dynamicDependencyMap->ClearAllSourceCoverage(); } - void Runtime::UpdateAndSerializeDynamicDependencyMap(const SourceCoveringTestsList& sourceCoverageTestsList) + void Runtime::UpdateAndSerializeDynamicDependencyMap(const AZStd::vector& jobs) { + const auto sourceCoverageTestsList = CreateSourceCoveringTestFromTestCoverages(jobs, m_config.m_repo.m_root); 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); @@ -323,11 +346,12 @@ namespace TestImpact AZStd::optional testCompleteCallback) { Timer timer; - AZStd::vector draftedTestTargets; + AZStd::vector draftedTestTargets = m_dynamicDependencyMap->GetNotCoveringTests(); auto [selectedTestTargets, discardedTestTargets] = SelectCoveringTestTargetsAndUpdateEnumerationCache(changeList, testPrioritizationPolicy); auto [includedSelectedTestTargets, excludedSelectedTestTargets] = SelectTestTargetsByExcludeList(selectedTestTargets); + AZStd::vector testTargetsToRun = ConcatenateVectors(includedSelectedTestTargets, draftedTestTargets); if (testSequenceStartCallback.has_value()) { (*testSequenceStartCallback)( @@ -336,10 +360,11 @@ namespace TestImpact ExtractTestTargetNames(draftedTestTargets)); } + if (dynamicDependencyMapPolicy == Policy::DynamicDependencyMap::Update) { const auto [result, testJobs] = m_testEngine->InstrumentedRun( - includedSelectedTestTargets, + testTargetsToRun, m_testShardingPolicy, m_executionFailurePolicy, Policy::IntegrityFailure::Continue, @@ -349,7 +374,7 @@ namespace TestImpact globalTimeout, TestRunCompleteCallbackHandler(testCompleteCallback)); - UpdateAndSerializeDynamicDependencyMap(CreateSourceCoveringTestFromTestCoverages(testJobs, m_config.m_repo.m_root)); + UpdateAndSerializeDynamicDependencyMap(testJobs); if (testSequenceEndCallback.has_value()) { @@ -361,7 +386,7 @@ namespace TestImpact else { const auto [result, testJobs] = m_testEngine->RegularRun( - includedSelectedTestTargets, + testTargetsToRun, m_testShardingPolicy, m_executionFailurePolicy, m_testFailurePolicy, @@ -389,12 +414,13 @@ namespace TestImpact AZStd::optional testCompleteCallback) { Timer timer; - AZStd::vector draftedTestTargets; + AZStd::vector draftedTestTargets = m_dynamicDependencyMap->GetNotCoveringTests(); auto [selectedTestTargets, discardedTestTargets] = SelectCoveringTestTargetsAndUpdateEnumerationCache(changeList, testPrioritizationPolicy); auto [includedSelectedTestTargets, excludedSelectedTestTargets] = SelectTestTargetsByExcludeList(selectedTestTargets); auto [includedDiscardedTestTargets, excludedDiscardedTestTargets] = SelectTestTargetsByExcludeList(discardedTestTargets); + AZStd::vector testTargetsToRun = ConcatenateVectors(includedSelectedTestTargets, draftedTestTargets); if (testSequenceStartCallback.has_value()) { (*testSequenceStartCallback)( @@ -403,9 +429,10 @@ namespace TestImpact ExtractTestTargetNames(draftedTestTargets)); } + // Impact analysis run of the selected test targets const auto [selectedResult, selectedTestJobs] = m_testEngine->InstrumentedRun( - includedSelectedTestTargets, + testTargetsToRun, m_testShardingPolicy, m_executionFailurePolicy, Policy::IntegrityFailure::Continue, @@ -433,7 +460,7 @@ namespace TestImpact globalTimeout, TestRunCompleteCallbackHandler(testCompleteCallback)); - UpdateAndSerializeDynamicDependencyMap(CreateSourceCoveringTestFromTestCoverages(selectedTestJobs, m_config.m_repo.m_root)); + UpdateAndSerializeDynamicDependencyMap(selectedTestJobs); if (testSequenceEndCallback.has_value()) { @@ -486,7 +513,7 @@ namespace TestImpact TestRunCompleteCallbackHandler(testCompleteCallback)); ClearDynamicDependencyMapAndRemoveExistingFile(); - UpdateAndSerializeDynamicDependencyMap(CreateSourceCoveringTestFromTestCoverages(testJobs, m_config.m_repo.m_root)); + UpdateAndSerializeDynamicDependencyMap(testJobs); if (testSequenceEndCallback.has_value()) { From 0da739ef18c41ec1ecb29cb563b69d676ef5d7cd Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 09:05:29 +0100 Subject: [PATCH 04/14] Add curiously missing comments --- .../Runtime/Code/Source/TestImpactRuntime.cpp | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp index 44657dad21..d83976b488 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -70,6 +70,7 @@ namespace TestImpact }; } + //! Utility for concatenating two vectors. template AZStd::vector ConcatenateVectors(const AZStd::vector& v1, const AZStd::vector& v2) { @@ -79,6 +80,7 @@ namespace TestImpact result.insert(result.end(), v2.begin(), v2.end()); return result; } + Runtime::Runtime( RuntimeConfig&& config, SuiteType suiteFilter, @@ -346,12 +348,20 @@ namespace TestImpact AZStd::optional testCompleteCallback) { Timer timer; + + // Draft in the test targets that have no coverage entries in the dynamic dependency map AZStd::vector draftedTestTargets = m_dynamicDependencyMap->GetNotCoveringTests(); + // The test targets that were selected for the change list by the dynamic dependency map and the test targets that were not auto [selectedTestTargets, discardedTestTargets] = SelectCoveringTestTargetsAndUpdateEnumerationCache(changeList, testPrioritizationPolicy); + + // The subset of selected test targets that are not on the configuration's exclude list and those that are auto [includedSelectedTestTargets, excludedSelectedTestTargets] = SelectTestTargetsByExcludeList(selectedTestTargets); + // We present to the client the included selected test targets and the drafted test targets as distinct sets but internally + // we consider the concatenated set of the two the actual set of tests to run AZStd::vector testTargetsToRun = ConcatenateVectors(includedSelectedTestTargets, draftedTestTargets); + if (testSequenceStartCallback.has_value()) { (*testSequenceStartCallback)( @@ -360,7 +370,6 @@ namespace TestImpact ExtractTestTargetNames(draftedTestTargets)); } - if (dynamicDependencyMapPolicy == Policy::DynamicDependencyMap::Update) { const auto [result, testJobs] = m_testEngine->InstrumentedRun( @@ -413,14 +422,24 @@ 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(); + // The test targets that were selected for the change list by the dynamic dependency map and the test targets that were not auto [selectedTestTargets, discardedTestTargets] = SelectCoveringTestTargetsAndUpdateEnumerationCache(changeList, testPrioritizationPolicy); + + // The subset of selected test targets that are not on the configuration's exclude list and those that are auto [includedSelectedTestTargets, excludedSelectedTestTargets] = SelectTestTargetsByExcludeList(selectedTestTargets); + + // The subset of discarded test targets that are not on the configuration's exclude list and those that are auto [includedDiscardedTestTargets, excludedDiscardedTestTargets] = SelectTestTargetsByExcludeList(discardedTestTargets); + // We present to the client the included selected test targets and the drafted test targets as distinct sets but internally + // we consider the concatenated set of the two the actual set of tests to run AZStd::vector testTargetsToRun = ConcatenateVectors(includedSelectedTestTargets, draftedTestTargets); + if (testSequenceStartCallback.has_value()) { (*testSequenceStartCallback)( @@ -429,7 +448,6 @@ namespace TestImpact ExtractTestTargetNames(draftedTestTargets)); } - // Impact analysis run of the selected test targets const auto [selectedResult, selectedTestJobs] = m_testEngine->InstrumentedRun( testTargetsToRun, From d341dab7f54d866849a2c1f96190808e7e0f622b Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 10:39:58 +0100 Subject: [PATCH 05/14] Address PR comments --- .../Static/Code/Source/TestImpactCommandLineOptions.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 aad9319cf9..01005718bc 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp @@ -405,9 +405,9 @@ namespace TestImpact " 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 test that \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" + " 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" From a0feeec608c694c24367b129a61a74749525a32f Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 10:56:07 +0100 Subject: [PATCH 06/14] Add console output to runtime --- ...tImpactConsoleTestSequenceEventHandler.cpp | 234 ++++++++++++++++++ ...estImpactConsoleTestSequenceEventHandler.h | 72 ++++++ .../Code/Source/TestImpactConsoleUtils.cpp | 34 +++ .../Code/Source/TestImpactConsoleUtils.h | 56 +++++ 4 files changed, 396 insertions(+) create mode 100644 Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp create mode 100644 Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h create mode 100644 Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp create mode 100644 Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp new file mode 100644 index 0000000000..8595fc0683 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp @@ -0,0 +1,234 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#include + +#include + +#include + +namespace TestImpact +{ + namespace Console + { + namespace Output + { + void TestSuiteFilter(SuiteType filter) + { + std::cout << "Test suite filter: " << GetSuiteTypeName(filter).c_str() << "\n"; + } + + void ImpactAnalysisTestSelection(size_t numSelectedTests, size_t numDiscardedTests, size_t numExcludedTests, size_t numDraftedTests) + { + const float totalTests = numSelectedTests + numDiscardedTests; + const float saving = (1.0 - (numSelectedTests / totalTests)) * 100.0f; + + std::cout << numSelectedTests << " tests selected, " << numDiscardedTests << " tests discarded (" << saving << "% test saving)\n"; + std::cout << "Of which " << numExcludedTests << " tests have been excluded and " << numDraftedTests << " tests have been drafted.\n"; + } + + void FailureReport(const Client::SequenceFailure& failureReport, AZStd::chrono::milliseconds duration) + { + std::cout << "Sequence completed in " << (duration.count() / 1000.f) << "s with"; + + if (failureReport.GetExecutionFailures().size() || + failureReport.GetTestRunFailures().size() || + failureReport.GetTimedOutTests().size() || + failureReport.GetUnexecutedTests().size()) + { + std::cout << ":\n"; + std::cout << SetColor(Foreground::White, Background::Red).c_str() + << failureReport.GetTestRunFailures().size() + << ResetColor().c_str() << " test failures\n"; + + std::cout << SetColor(Foreground::White, Background::Red).c_str() + << failureReport.GetExecutionFailures().size() + << ResetColor().c_str() << " execution failures\n"; + + std::cout << SetColor(Foreground::White, Background::Red).c_str() + << failureReport.GetTimedOutTests().size() + << ResetColor().c_str() << " test timeouts\n"; + + std::cout << SetColor(Foreground::White, Background::Red).c_str() + << failureReport.GetUnexecutedTests().size() + << ResetColor().c_str() << " unexecuted tests\n"; + + if (!failureReport.GetTestRunFailures().empty()) + { + std::cout << "\nTest failures:\n"; + for (const auto& testRunFailure : failureReport.GetTestRunFailures()) + { + std::cout << " " << testRunFailure.GetTargetName().c_str(); + for (const auto& testCaseFailure : testRunFailure.GetTestCaseFailures()) + { + std::cout << "." << testCaseFailure.GetName().c_str(); + for (const auto& testFailure : testCaseFailure.GetTestFailures()) + { + std::cout << "." << testFailure.GetName().c_str() << "\n"; + } + } + } + } + + if (!failureReport.GetExecutionFailures().empty()) + { + std::cout << "\nExecution failures:\n"; + for (const auto& executionFailure : failureReport.GetExecutionFailures()) + { + std::cout << " " << executionFailure.GetTargetName().c_str() << "\n"; + std::cout << executionFailure.GetCommandString().c_str() << "\n"; + } + } + + if (!failureReport.GetTimedOutTests().empty()) + { + std::cout << "\nTimed out tests:\n"; + for (const auto& testTimeout : failureReport.GetTimedOutTests()) + { + std::cout << " " << testTimeout.GetTargetName().c_str() << "\n"; + } + } + + if (!failureReport.GetUnexecutedTests().empty()) + { + std::cout << "\nUnexecuted tests:\n"; + for (const auto& unexecutedTest : failureReport.GetUnexecutedTests()) + { + std::cout << " " << unexecutedTest.GetTargetName().c_str() << "\n"; + } + } + } + else + { + std::cout << SetColor(Foreground::White, Background::Green).c_str() << " \100% passes!\n" << ResetColor().c_str(); + } + } + } + + TestSequenceEventHandler::TestSequenceEventHandler(SuiteType suiteFilter) + : m_suiteFilter(suiteFilter) + { + } + + // TestSequenceStartCallback + void TestSequenceEventHandler::operator()(Client::TestRunSelection&& selectedTests) + { + ClearState(); + m_numTests = selectedTests.GetNumIncludedTestRuns(); + + Output::TestSuiteFilter(m_suiteFilter); + std::cout << selectedTests.GetNumIncludedTestRuns() << " tests selected, " << selectedTests.GetNumExcludedTestRuns() << " excluded.\n"; + } + + // ImpactAnalysisTestSequenceStartCallback + void TestSequenceEventHandler::operator()( + Client::TestRunSelection&& selectedTests, + AZStd::vector&& discardedTests, + AZStd::vector&& draftedTests) + { + ClearState(); + m_numTests = selectedTests.GetNumIncludedTestRuns() + draftedTests.size(); + + Output::TestSuiteFilter(m_suiteFilter); + Output::ImpactAnalysisTestSelection( + selectedTests.GetTotalNumTests(), discardedTests.size(), selectedTests.GetNumExcludedTestRuns(), draftedTests.size()); + } + + // SafeImpactAnalysisTestSequenceStartCallback + void TestSequenceEventHandler::operator()( + Client::TestRunSelection&& selectedTests, + Client::TestRunSelection&& discardedTests, + AZStd::vector&& draftedTests) + { + ClearState(); + m_numTests = selectedTests.GetNumIncludedTestRuns() + draftedTests.size(); + + Output::TestSuiteFilter(m_suiteFilter); + Output::ImpactAnalysisTestSelection( + selectedTests.GetTotalNumTests(), + discardedTests.GetTotalNumTests(), + selectedTests.GetNumExcludedTestRuns() + discardedTests.GetNumExcludedTestRuns(), + draftedTests.size()); + } + + // TestSequenceCompleteCallback + void TestSequenceEventHandler::operator()( + Client::SequenceFailure&& failureReport, + AZStd::chrono::milliseconds duration) + { + + Output::FailureReport(failureReport, duration); + std::cout << "Updating and serializing the test impact analysis data, this may take a moment...\n"; + } + + // SafeTestSequenceCompleteCallback + void TestSequenceEventHandler::operator()( + Client::SequenceFailure&& selectedFailureReport, + Client::SequenceFailure&& discardedFailureReport, + AZStd::chrono::milliseconds selectedDuration, + AZStd::chrono::milliseconds discaredDuration) + { + std::cout << "Selected test run:\n"; + Output::FailureReport(selectedFailureReport, selectedDuration); + + std::cout << "Discarded test run:\n"; + Output::FailureReport(discardedFailureReport, discaredDuration); + + std::cout << "Updating and serializing the test impact analysis data, this may take a moment...\n"; + } + + // TestRunCompleteCallback + void TestSequenceEventHandler::operator()([[maybe_unused]] Client::TestRun&& test) + { + m_numTestsComplete++; + const auto progress = AZStd::string::format("(%03u/%03u)", m_numTestsComplete, m_numTests, test.GetTargetName().c_str()); + + AZStd::string result; + switch (test.GetResult()) + { + case Client::TestRunResult::AllTestsPass: + { + result = SetColorForString(Foreground::White, Background::Green, "PASS"); + break; + } + case Client::TestRunResult::FailedToExecute: + { + result = SetColorForString(Foreground::White, Background::Red, "EXEC"); + break; + } + case Client::TestRunResult::NotRun: + { + result = SetColorForString(Foreground::White, Background::Yellow, "SKIP"); + break; + } + case Client::TestRunResult::TestFailures: + { + result = SetColorForString(Foreground::White, Background::Red, "FAIL"); + break; + } + case Client::TestRunResult::Timeout: + { + result = SetColorForString(Foreground::White, Background::Magenta, "TIME"); + break; + } + } + + std::cout << progress.c_str() << " " << result.c_str() << " " << test.GetTargetName().c_str() << " (" << (test.GetDuration().count() / 1000.f) << "s)\n"; + } + + void TestSequenceEventHandler::ClearState() + { + m_numTests = 0; + m_numTestsComplete = 0; + } + } // namespace Console +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h new file mode 100644 index 0000000000..01f59b2c64 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h @@ -0,0 +1,72 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#include +#include +#include +#include + +#include +#include +#include + +#pragma once + +namespace TestImpact +{ + namespace Console + { + //! Event handler for all test sequence types. + class TestSequenceEventHandler + { + public: + TestSequenceEventHandler(SuiteType suiteFilter); + + //! TestSequenceStartCallback. + void operator()(Client::TestRunSelection&& selectedTests); + + //! ImpactAnalysisTestSequenceStartCallback. + void operator()( + Client::TestRunSelection&& selectedTests, + AZStd::vector&& discardedTests, + AZStd::vector&& draftedTests); + + //! SafeImpactAnalysisTestSequenceStartCallback. + void operator()( + Client::TestRunSelection&& selectedTests, + Client::TestRunSelection&& discardedTests, + AZStd::vector&& draftedTests); + + //! TestSequenceCompleteCallback. + void operator()( + Client::SequenceFailure&& failureReport, + AZStd::chrono::milliseconds duration); + + //! SafeTestSequenceCompleteCallback. + void operator()( + Client::SequenceFailure&& selectedFailureReport, + Client::SequenceFailure&& discardedFailureReport, + AZStd::chrono::milliseconds selectedDuration, + AZStd::chrono::milliseconds discaredDuration); + + //! TestRunCompleteCallback. + void operator()(Client::TestRun&& test); + + private: + void ClearState(); + + SuiteType m_suiteFilter; + size_t m_numTests = 0; + size_t m_numTestsComplete = 0; + }; + } // namespace Console +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp new file mode 100644 index 0000000000..e00a766eb5 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp @@ -0,0 +1,34 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#include + +namespace TestImpact +{ + namespace Console + { + AZStd::string SetColor(Foreground fgd, Background bgd) + { + return AZStd::string::format("\033[%u;%um", static_cast(fgd), static_cast(bgd)); + } + + AZStd::string SetColorForString(Foreground fgd, Background bgd, const AZStd::string& str) + { + return AZStd::string::format("%s%s%s", SetColor(fgd, bgd).c_str(), str.c_str(), ResetColor().c_str()); + } + + AZStd::string ResetColor() + { + return "\033[0m"; + } + } // namespace Console +} // namespace TestImpact diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h new file mode 100644 index 0000000000..d0fc2495d3 --- /dev/null +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h @@ -0,0 +1,56 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#pragma once + +#include + +namespace TestImpact +{ + namespace Console + { + //! The set of available foreground colors. + enum class Foreground + { + Black = 30, + Red, + Green, + Yellow, + Blue, + Magenta, + Cyan, + White + }; + + //! The set of available background colors. + enum class Background + { + Black = 40, + Red, + Green, + Yellow, + Blue, + Magenta, + Cyan, + White + }; + + //! Returns a string to be used to set the specified foreground and background color. + AZStd::string SetColor(Foreground fgd, Background bgd); + + //! Returns a string with the specified string set to the specified foreground and background color followed by a color reset. + AZStd::string SetColorForString(Foreground fgd, Background bgd, const AZStd::string& str); + + //! Returns a string to be used to reset the color back to white foreground on black background. + AZStd::string ResetColor(); + } // namespace Console +} // namespace TestImpact From 796b5be286a713d9e42f2363a3cdfcf0bdf5dafd Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 15:32:17 +0100 Subject: [PATCH 07/14] Address PR comments --- .../Source/TestImpactConsoleTestSequenceEventHandler.cpp | 8 ++++---- .../Source/TestImpactConsoleTestSequenceEventHandler.h | 2 +- .../Console/Static/Code/Source/TestImpactConsoleUtils.cpp | 8 ++++---- .../Console/Static/Code/Source/TestImpactConsoleUtils.h | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp index 8595fc0683..0a745514a5 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.cpp @@ -40,10 +40,10 @@ namespace TestImpact { std::cout << "Sequence completed in " << (duration.count() / 1000.f) << "s with"; - if (failureReport.GetExecutionFailures().size() || - failureReport.GetTestRunFailures().size() || - failureReport.GetTimedOutTests().size() || - failureReport.GetUnexecutedTests().size()) + if (!failureReport.GetExecutionFailures().empty() || + !failureReport.GetTestRunFailures().empty() || + !failureReport.GetTimedOutTests().empty() || + !failureReport.GetUnexecutedTests().empty()) { std::cout << ":\n"; std::cout << SetColor(Foreground::White, Background::Red).c_str() diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h index 01f59b2c64..ee5eee0bf8 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleTestSequenceEventHandler.h @@ -29,7 +29,7 @@ namespace TestImpact class TestSequenceEventHandler { public: - TestSequenceEventHandler(SuiteType suiteFilter); + explicit TestSequenceEventHandler(SuiteType suiteFilter); //! TestSequenceStartCallback. void operator()(Client::TestRunSelection&& selectedTests); diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp index e00a766eb5..76ce830371 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.cpp @@ -16,14 +16,14 @@ namespace TestImpact { namespace Console { - AZStd::string SetColor(Foreground fgd, Background bgd) + AZStd::string SetColor(Foreground foreground, Background background) { - return AZStd::string::format("\033[%u;%um", static_cast(fgd), static_cast(bgd)); + return AZStd::string::format("\033[%u;%um", aznumeric_cast(foreground), aznumeric_cast(background)); } - AZStd::string SetColorForString(Foreground fgd, Background bgd, const AZStd::string& str) + AZStd::string SetColorForString(Foreground foreground, Background background, const AZStd::string& str) { - return AZStd::string::format("%s%s%s", SetColor(fgd, bgd).c_str(), str.c_str(), ResetColor().c_str()); + return AZStd::string::format("%s%s%s", SetColor(foreground, background).c_str(), str.c_str(), ResetColor().c_str()); } AZStd::string ResetColor() diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h index d0fc2495d3..a003e2db26 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleUtils.h @@ -45,10 +45,10 @@ namespace TestImpact }; //! Returns a string to be used to set the specified foreground and background color. - AZStd::string SetColor(Foreground fgd, Background bgd); + AZStd::string SetColor(Foreground foreground, Background background); //! Returns a string with the specified string set to the specified foreground and background color followed by a color reset. - AZStd::string SetColorForString(Foreground fgd, Background bgd, const AZStd::string& str); + AZStd::string SetColorForString(Foreground foreground, Background background, const AZStd::string& str); //! Returns a string to be used to reset the color back to white foreground on black background. AZStd::string ResetColor(); From cc9e8a72e447a65dd67f9b252a79dacfcce7b9c6 Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 17:45:48 +0100 Subject: [PATCH 08/14] Fix coverage clear bug and other minor refactors --- .../Source/TestImpactCommandLineOptions.cpp | 16 ++-- .../TestImpactFramework/TestImpactRuntime.h | 7 +- .../TestImpactTestSequence.h | 2 +- .../TestImpactDynamicDependencyMap.cpp | 39 +++++++- .../TestImpactDynamicDependencyMap.h | 9 +- .../Enumeration/TestImpactTestEnumerator.cpp | 2 +- .../Run/TestImpactInstrumentedTestRunner.cpp | 2 +- .../TestEngine/Run/TestImpactTestRunner.cpp | 2 +- .../Runtime/Code/Source/TestImpactRuntime.cpp | 91 ++++++++++++++----- .../Code/Source/TestImpactRuntimeUtils.cpp | 52 +---------- .../Code/Source/TestImpactRuntimeUtils.h | 7 +- 11 files changed, 132 insertions(+), 97 deletions(-) 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..992b466a54 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} }; @@ -405,11 +405,11 @@ namespace TestImpact " 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" + " -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" diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h index 86f2907ab7..2a7720ee55 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 consolidates 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..79f5269188 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -256,32 +256,77 @@ 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 + if (const auto testResult = job.GetTestResult(); + testResult == Client::TestRunResult::AllTestsPass || + (m_failedTestCoveragePolicy == Policy::FailedTestCoverage::Keep && 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(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 +467,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 +504,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 +525,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 +579,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. From f603b08d0542983c18c1b76cbed72aae88851f7f Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 18:37:20 +0100 Subject: [PATCH 09/14] Address PR comments --- .../Runtime/Code/Source/TestImpactRuntime.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp index 79f5269188..ef4bf5cd03 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -271,9 +271,15 @@ namespace TestImpact // 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 - if (const auto testResult = job.GetTestResult(); - testResult == Client::TestRunResult::AllTestsPass || - (m_failedTestCoveragePolicy == Policy::FailedTestCoverage::Keep && testResult == Client::TestRunResult::TestFailures)) + 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) { From a56591fa126862e58ec6ab5b384282d2acaea538 Mon Sep 17 00:00:00 2001 From: jonawals Date: Mon, 7 Jun 2021 18:38:54 +0100 Subject: [PATCH 10/14] Address PR comments --- .../Runtime/Code/Source/TestImpactRuntime.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp index ef4bf5cd03..784a46ffa0 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestImpactRuntime.cpp @@ -291,7 +291,8 @@ namespace TestImpact "Test target '%s' completed its test run successfully but produced no coverage data", job.GetTestTarget()->GetName().c_str())); } - else if (!job.GetTestCoverge().has_value()) + + 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 From e0f953cf6bf787b979c59bf5607978b91c504c11 Mon Sep 17 00:00:00 2001 From: jonawals Date: Tue, 8 Jun 2021 11:59:06 +0100 Subject: [PATCH 11/14] Address PR comments --- .../Source/TestImpactCommandLineOptions.cpp | 133 +++++++++--------- .../TestImpactFramework/TestImpactRuntime.h | 2 +- 2 files changed, 69 insertions(+), 66 deletions(-) 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 992b466a54..0a4c80afa4 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactCommandLineOptions.cpp @@ -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 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 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 2a7720ee55..485b966a1e 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h @@ -202,7 +202,7 @@ namespace TestImpact AZStd::pair, AZStd::vector> SelectTestTargetsByExcludeList( AZStd::vector testTargets) const; - //! Prunes the existing coverage for the specified jobs and creates the consolidates source covering tests list from the + //! 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); From c9351f4d45714bbb6b8432ed26fb3e4286d84f58 Mon Sep 17 00:00:00 2001 From: jonawals Date: Tue, 8 Jun 2021 12:05:38 +0100 Subject: [PATCH 12/14] Make TestEngine clean up artifact dir before runs --- .../Code/Source/TestEngine/TestImpactTestEngine.cpp | 12 ++++++++++++ .../Code/Source/TestEngine/TestImpactTestEngine.h | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp index b563c1846b..f3ce777938 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp @@ -10,6 +10,8 @@ * */ +#include + #include #include #include @@ -247,11 +249,17 @@ namespace TestImpact , m_testEnumerator(AZStd::make_unique(maxConcurrentRuns)) , m_instrumentedTestRunner(AZStd::make_unique(maxConcurrentRuns)) , m_testRunner(AZStd::make_unique(maxConcurrentRuns)) + , m_artifactDir(artifactDir) { } TestEngine::~TestEngine() = default; + void TestEngine::CleanArtifactDir() const + { + DeleteFiles(m_artifactDir, "*xml*"); + } + AZStd::pair> TestEngine::UpdateEnumerationCache( const AZStd::vector& testTargets, Policy::ExecutionFailure executionFailurePolicy, @@ -283,6 +291,8 @@ namespace TestImpact AZStd::optional globalTimeout, AZStd::optional callback) { + CleanArtifactDir(); + TestEngineJobMap engineJobs; const auto jobInfos = m_testJobInfoGenerator->GenerateRegularTestRunJobInfos(testTargets); @@ -308,6 +318,8 @@ namespace TestImpact AZStd::optional globalTimeout, AZStd::optional callback) { + CleanArtifactDir(); + TestEngineJobMap engineJobs; const auto jobInfos = m_testJobInfoGenerator->GenerateInstrumentedTestRunJobInfos(testTargets, CoverageLevel::Source); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h index 83f57f00ef..5192a1d3b7 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h @@ -118,10 +118,14 @@ namespace TestImpact AZStd::optional callback); private: + //! Cleans up the artifacts directory of any artifacts from previous runs. + void CleanArtifactDir() const; + size_t m_maxConcurrentRuns = 0; AZStd::unique_ptr m_testJobInfoGenerator; AZStd::unique_ptr m_testEnumerator; AZStd::unique_ptr m_instrumentedTestRunner; AZStd::unique_ptr m_testRunner; + RepoPath m_artifactDir; }; } // namespace TestImpact From 9e187d67a4f272bd4e9bdb928fc02c48ab172a53 Mon Sep 17 00:00:00 2001 From: jonawals Date: Tue, 8 Jun 2021 12:08:42 +0100 Subject: [PATCH 13/14] Fix typo with delete files filter --- .../Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp index f3ce777938..4c7d660004 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp @@ -257,7 +257,7 @@ namespace TestImpact void TestEngine::CleanArtifactDir() const { - DeleteFiles(m_artifactDir, "*xml*"); + DeleteFiles(m_artifactDir, "*.xml"); } AZStd::pair> TestEngine::UpdateEnumerationCache( From 7a884c85b37d7bfa810b53d7a9720fcc5e3f5d9c Mon Sep 17 00:00:00 2001 From: jonawals Date: Tue, 8 Jun 2021 12:19:07 +0100 Subject: [PATCH 14/14] Address PR comments --- .../Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp | 6 +++--- .../Runtime/Code/Source/TestEngine/TestImpactTestEngine.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp index 4c7d660004..a9d0e15781 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.cpp @@ -255,7 +255,7 @@ namespace TestImpact TestEngine::~TestEngine() = default; - void TestEngine::CleanArtifactDir() const + void TestEngine::DeleteArtifactXmls() const { DeleteFiles(m_artifactDir, "*.xml"); } @@ -291,7 +291,7 @@ namespace TestImpact AZStd::optional globalTimeout, AZStd::optional callback) { - CleanArtifactDir(); + DeleteArtifactXmls(); TestEngineJobMap engineJobs; const auto jobInfos = m_testJobInfoGenerator->GenerateRegularTestRunJobInfos(testTargets); @@ -318,7 +318,7 @@ namespace TestImpact AZStd::optional globalTimeout, AZStd::optional callback) { - CleanArtifactDir(); + DeleteArtifactXmls(); TestEngineJobMap engineJobs; const auto jobInfos = m_testJobInfoGenerator->GenerateInstrumentedTestRunJobInfos(testTargets, CoverageLevel::Source); diff --git a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h index 5192a1d3b7..7d16f352f3 100644 --- a/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h +++ b/Code/Tools/TestImpactFramework/Runtime/Code/Source/TestEngine/TestImpactTestEngine.h @@ -119,7 +119,7 @@ namespace TestImpact private: //! Cleans up the artifacts directory of any artifacts from previous runs. - void CleanArtifactDir() const; + void DeleteArtifactXmls() const; size_t m_maxConcurrentRuns = 0; AZStd::unique_ptr m_testJobInfoGenerator;