From 086a41c0f3fec3068b9bf1b7ebe094bf1958ee1b Mon Sep 17 00:00:00 2001
From: Brad King <brad.king@kitware.com>
Date: Mon, 20 Nov 2023 19:09:03 -0500
Subject: cmCTestMultiProcessHandler: Simplify test startup batching

Once a test is ready to run, count it against the number of tests to
start in the current batch whether or not the test process actually
starts successfully.  If a test process does fail to start, simply
schedule a new startup batch on the next loop iteration.
---
 Source/CTest/cmCTestMultiProcessHandler.cxx | 23 ++++++++++-------------
 Source/CTest/cmCTestMultiProcessHandler.h   |  4 ++--
 Source/CTest/cmCTestRunTest.cxx             |  5 +----
 Source/CTest/cmCTestRunTest.h               |  2 +-
 4 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index a673370..a9930df 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -162,7 +162,7 @@ void cmCTestMultiProcessHandler::RunTests()
   this->UpdateCostData();
 }
 
-bool cmCTestMultiProcessHandler::StartTestProcess(int test)
+void cmCTestMultiProcessHandler::StartTestProcess(int test)
 {
   if (this->HaveAffinity && this->Properties[test]->WantAffinity) {
     size_t needProcessors = this->GetProcessorsUsed(test);
@@ -241,7 +241,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
     e << "Resource spec file:\n\n  " << this->ResourceSpecFile;
     cmCTestRunTest::StartFailure(std::move(testRun), this->Total, e.str(),
                                  "Insufficient resources");
-    return false;
+    return;
   }
 
   cmWorkingDirectory workdir(this->Properties[test]->Directory);
@@ -251,13 +251,12 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
                                    this->Properties[test]->Directory + " : " +
                                    std::strerror(workdir.GetLastResult()),
                                  "Failed to change working directory");
-    return false;
+    return;
   }
 
   // Ownership of 'testRun' has moved to another structure.
   // When the test finishes, FinishTestProcess will be called.
-  return cmCTestRunTest::StartTest(std::move(testRun), this->Completed,
-                                   this->Total);
+  cmCTestRunTest::StartTest(std::move(testRun), this->Completed, this->Total);
 }
 
 bool cmCTestMultiProcessHandler::AllocateResources(int index)
@@ -461,9 +460,9 @@ std::string cmCTestMultiProcessHandler::GetName(int test)
   return this->Properties[test]->Name;
 }
 
-bool cmCTestMultiProcessHandler::StartTest(int test)
+void cmCTestMultiProcessHandler::StartTest(int test)
 {
-  return this->StartTestProcess(test);
+  this->StartTestProcess(test);
 }
 
 void cmCTestMultiProcessHandler::StartNextTests()
@@ -583,9 +582,8 @@ void cmCTestMultiProcessHandler::StartNextTests()
     }
 
     // The test is ready to run.
-    if (this->StartTest(test)) {
-      numToStart -= processors;
-    }
+    numToStart -= processors;
+    this->StartTest(test);
   }
 
   if (allTestsFailedTestLoadCheck) {
@@ -694,9 +692,8 @@ void cmCTestMultiProcessHandler::FinishTestProcess(
   properties->Affinity.clear();
 
   runner.reset();
-  if (started) {
-    this->StartNextTestsOnIdle();
-  }
+
+  this->StartNextTestsOnIdle();
 }
 
 void cmCTestMultiProcessHandler::UpdateCostData()
diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h
index cc230e7..ffbe16f 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.h
+++ b/Source/CTest/cmCTestMultiProcessHandler.h
@@ -108,8 +108,8 @@ protected:
   // Start the next test or tests as many as are allowed by
   // ParallelLevel
   void StartNextTests();
-  bool StartTestProcess(int test);
-  bool StartTest(int test);
+  void StartTestProcess(int test);
+  void StartTest(int test);
   // Mark the checkpoint for the given test
   void WriteCheckpoint(int index);
 
diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index e96dc1f..483b3b4 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -530,7 +530,7 @@ std::string cmCTestRunTest::GetTestPrefix(size_t completed, size_t total) const
   return outputStream.str();
 }
 
-bool cmCTestRunTest::StartTest(std::unique_ptr<cmCTestRunTest> runner,
+void cmCTestRunTest::StartTest(std::unique_ptr<cmCTestRunTest> runner,
                                size_t completed, size_t total)
 {
   auto* testRun = runner.get();
@@ -539,10 +539,7 @@ bool cmCTestRunTest::StartTest(std::unique_ptr<cmCTestRunTest> runner,
 
   if (!testRun->StartTest(completed, total)) {
     testRun->FinalizeTest(false);
-    return false;
   }
-
-  return true;
 }
 
 // Starts the execution of a test.  Returns once it has started
diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h
index 154abca..71d0865 100644
--- a/Source/CTest/cmCTestRunTest.h
+++ b/Source/CTest/cmCTestRunTest.h
@@ -56,7 +56,7 @@ public:
   // Read and store output.  Returns true if it must be called again.
   void CheckOutput(std::string const& line);
 
-  static bool StartTest(std::unique_ptr<cmCTestRunTest> runner,
+  static void StartTest(std::unique_ptr<cmCTestRunTest> runner,
                         size_t completed, size_t total);
   static bool StartAgain(std::unique_ptr<cmCTestRunTest> runner,
                          size_t completed);
-- 
cgit v0.12