Address PR comments

Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactClientFailureReport.h

@@ -116,7 +116,7 @@ namespace TestImpact
             SequenceFailure(
                 AZStd::vector<ExecutionFailure>&& executionFailures,
                 AZStd::vector<LauncherFailure>&& launcherFailures,
-                AZStd::vector<TargetFailure>&& unexecutionTests);
+                AZStd::vector<TargetFailure>&& unexecutedTests);
 
             //! Returns the test targets in this sequence that failed to execute.
             const AZStd::vector<ExecutionFailure>& GetExecutionFailures() const;
@@ -130,7 +130,7 @@ namespace TestImpact
         private:
             AZStd::vector<ExecutionFailure> m_executionFailures;
             AZStd::vector<LauncherFailure> m_launcherFailures;
-            AZStd::vector<TargetFailure> m_unexecutionTests;
+            AZStd::vector<TargetFailure> m_unexecutedTestsTests;
         };
 
         //! Represents the report for a failed regular test sequence run without test impact analysis.
@@ -142,7 +142,7 @@ namespace TestImpact
                 AZStd::vector<ExecutionFailure>&& executionFailures,
                 AZStd::vector<LauncherFailure>&& launcherFailures,
                 AZStd::vector<TestRunFailure>&& testRunFailures,
-                AZStd::vector<TargetFailure>&& unexecutionTests);
+                AZStd::vector<TargetFailure>&& unexecutedTests);
 
             //! Returns the test targets that contain failing tests.
             const AZStd::vector<TestRunFailure>& GetTestRunFailures() const;
@@ -161,7 +161,7 @@ namespace TestImpact
                 AZStd::vector<LauncherFailure>&& launcherFailures,
                 AZStd::vector<TestRunFailure>&& selectedTestRunFailures,
                 AZStd::vector<TestRunFailure>&& discardedTestRunFailures,
-                AZStd::vector<TargetFailure>&& unexecutionTests);
+                AZStd::vector<TargetFailure>&& unexecutedTests);
 
             //! Returns the test targets that were selected to run but contain failing tests.
             const AZStd::vector<TestRunFailure> GetSelectedTestRunFailures() const;

Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactConfiguration.h

@@ -48,7 +48,8 @@ namespace TestImpact
             RelativePaths m_relativePaths;
         };
 
-        struct Persistent
+        //! Active persistent data workspace configuration.
+        struct Active
         {
             //! Paths relative to root.
             struct RelativePaths
@@ -62,7 +63,7 @@ namespace TestImpact
         };
 
         Temp m_temp;
-        Persistent m_persistent;
+        Active m_active;
     };
 
     //! Build target descriptor configuration.

Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRepoPath.h

@@ -26,7 +26,7 @@ namespace TestImpact
     public:
         constexpr RepoPath() = default;
         constexpr RepoPath(const RepoPath&) = default;
-        constexpr RepoPath(RepoPath&&) = default;
+        constexpr RepoPath(RepoPath&&) noexcept = default;
         constexpr RepoPath(const string_type&) noexcept;
         constexpr RepoPath(const value_type*) noexcept;
         constexpr RepoPath(const AZ::IO::PathView&);

Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactRuntime.h

@@ -73,7 +73,7 @@ namespace TestImpact
     //! @param duration The total duration of this test sequence.
     using ImpactAnalysisTestSequenceCompleteCallback = AZStd::function<void(Client::ImpactAnalysisSequenceFailure&& failureReport, AZStd::chrono::milliseconds duration)>;
 
-    //! Callback for test runs that have completed for any reason
+    //! Callback for test runs that have completed for any reason.
     //! test The test that has completed.
     using TestCompleteCallback = AZStd::function<void(Client::TestRun&& test)>;
 
@@ -106,7 +106,7 @@ namespace TestImpact
         //! @param testTargetTimeout 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.
         //! @param testSequenceCompleteCallback The client function to be called after the test sequence has completed.
-        //! @param testRunCompleteCallback The client function to be called after an individual test run has compelted.
+        //! @param testRunCompleteCallback The client function to be called after an individual test run has completed.
         TestSequenceResult RegularTestSequence(
             const AZStd::unordered_set<AZStd::string> suitesFilter,
             AZStd::optional<AZStd::chrono::milliseconds> testTargetTimeout,
@@ -122,7 +122,7 @@ namespace TestImpact
         //! @param testTargetTimeout 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.
         //! @param testSequenceCompleteCallback The client function to be called after the test sequence has completed.
-        //! @param testRunCompleteCallback The client function to be called after an individual test run has compelted.
+        //! @param testRunCompleteCallback The client function to be called after an individual test run has completed.
         TestSequenceResult ImpactAnalysisTestSequence(
             const ChangeList& changeList,
             Policy::TestPrioritization testPrioritizationPolicy,
@@ -140,7 +140,7 @@ namespace TestImpact
         //! @param testTargetTimeout 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.
         //! @param testSequenceCompleteCallback The client function to be called after the test sequence has completed.
-        //! @param testRunCompleteCallback The client function to be called after an individual test run has compelted.
+        //! @param testRunCompleteCallback The client function to be called after an individual test run has completed.
         TestSequenceResult SafeImpactAnalysisTestSequence(
             const ChangeList& changeList,
             const AZStd::unordered_set<AZStd::string> suitesFilter,
@@ -155,7 +155,7 @@ namespace TestImpact
         //! @param testTargetTimeout 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.
         //! @param testSequenceCompleteCallback The client function to be called after the test sequence has completed.
-        //! @param testRunCompleteCallback The client function to be called after an individual test run has compelted.
+        //! @param testRunCompleteCallback The client function to be called after an individual test run has completed.
         TestSequenceResult SeededTestSequence(
             AZStd::optional<AZStd::chrono::milliseconds> globalTimeout,
             AZStd::optional<TestSequenceStartCallback> testSequenceStartCallback,
@@ -173,7 +173,7 @@ namespace TestImpact
         Policy::IntegrityFailure m_integrationFailurePolicy;
         Policy::TestSharding m_testShardingPolicy;
         TargetOutputCapture m_targetOutputCapture;
-        size_t m_maxConcurrency;
+        size_t m_maxConcurrency = 0;
         AZStd::unique_ptr<DynamicDependencyMap> m_dynamicDependencyMap;
         AZStd::unique_ptr<TestEngine> m_testEngine;
         AZStd::unordered_set<const TestTarget*> m_testTargetExcludeList;

Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactTestSequence.h

@@ -85,8 +85,8 @@ namespace TestImpact
     //! Result of a test sequence that was run.
     enum class TestSequenceResult
     {
-        Success, //! All tests ran with no failures.
-        Failure, //! One or more tests failed and/or timed out and/or failed to launch (if execution failure policy is not Ignore) and/or an integrity failure was encountered.
-        Timeout //! The global timeout for the sequence was exceeded.
+        Success, //!< All tests ran with no failures.
+        Failure, //!< One or more tests failed and/or timed out and/or failed to launch and/or an integrity failure was encountered.
+        Timeout //!< The global timeout for the sequence was exceeded.
     };
 } // namespace TestImpact

Code/Tools/TestImpactFramework/Runtime/Code/Include/TestImpactFramework/TestImpactUtils.h

@@ -56,8 +56,7 @@ namespace TestImpact
             ExceptionType,
             AZStd::string::format("Couldn't open file %s for writing", path.c_str()));
 
-        AZ_TestImpact_Eval(file.Write(bytes.data(), bytes.size()), ExceptionType, AZStd::string::format("Couldn't write contents for file %s", path.c_str()));
-
-        return;
+        AZ_TestImpact_Eval(
+            file.Write(bytes.data(), bytes.size()), ExceptionType, AZStd::string::format("Couldn't write contents for file %s", path.c_str()));
     }
 } // namespace TestImpact