CTest: Improve stop-time implementation

The CTestTestStopTime test has been failing sporadically because the
stop time causes the first internal test to have a timeout short enough
that we might hit it and start the second test just before the stop time
is reached.  Instead we should track when a timeout is shortened in
order to stay within the stop time.  If a test times out for this reason
then we should consider the stop time reached and not start any more
tests.

1 files changed, 32 insertions, 17 deletions

diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index 54b4be2..3e0c1ac 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -56,7 +56,6 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler()
   this->RunningCount = 0;
   this->ProcessorsAvailable = cmAffinity::GetProcessorsAvailable();
   this->HaveAffinity = this->ProcessorsAvailable.size();
-  this->StopTimePassed = false;
   this->HasCycles = false;
   this->SerialTestRunning = false;
 }
@@ -130,17 +129,6 @@ void cmCTestMultiProcessHandler::RunTests()
 
 bool cmCTestMultiProcessHandler::StartTestProcess(int test)
 {
-  std::chrono::system_clock::time_point stop_time = this->CTest->GetStopTime();
-  if (stop_time != std::chrono::system_clock::time_point() &&
-      stop_time <= std::chrono::system_clock::now()) {
-    cmCTestLog(this->CTest, ERROR_MESSAGE,
-               "The stop time has been passed. "
-               "Stopping all tests."
-                 << std::endl);
-    this->StopTimePassed = true;
-    return false;
-  }
-
   if (this->HaveAffinity && this->Properties[test]->WantAffinity) {
     size_t needProcessors = this->GetProcessorsUsed(test);
     if (needProcessors > this->ProcessorsAvailable.size()) {
@@ -199,6 +187,30 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
   return false;
 }
 
+bool cmCTestMultiProcessHandler::CheckStopTimePassed()
+{
+  if (!this->StopTimePassed) {
+    std::chrono::system_clock::time_point stop_time =
+      this->CTest->GetStopTime();
+    if (stop_time != std::chrono::system_clock::time_point() &&
+        stop_time <= std::chrono::system_clock::now()) {
+      this->SetStopTimePassed();
+    }
+  }
+  return this->StopTimePassed;
+}
+
+void cmCTestMultiProcessHandler::SetStopTimePassed()
+{
+  if (!this->StopTimePassed) {
+    cmCTestLog(this->CTest, ERROR_MESSAGE,
+               "The stop time has been passed. "
+               "Stopping all tests."
+                 << std::endl);
+    this->StopTimePassed = true;
+  }
+}
+
 void cmCTestMultiProcessHandler::LockResources(int index)
 {
   this->LockedResources.insert(
@@ -279,6 +291,10 @@ void cmCTestMultiProcessHandler::StartNextTests()
     return;
   }
 
+  if (this->CheckStopTimePassed()) {
+    return;
+  }
+
   size_t numToStart = 0;
 
   if (this->RunningCount < this->ParallelLevel) {
@@ -358,10 +374,6 @@ void cmCTestMultiProcessHandler::StartNextTests()
     }
 
     if (testLoadOk && processors <= numToStart && this->StartTest(test)) {
-      if (this->StopTimePassed) {
-        return;
-      }
-
       numToStart -= processors;
     } else if (numToStart == 0) {
       break;
@@ -424,8 +436,11 @@ void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner,
   auto properties = runner->GetTestProperties();
 
   bool testResult = runner->EndTest(this->Completed, this->Total, started);
+  if (runner->TimedOutForStopTime()) {
+    this->SetStopTimePassed();
+  }
   if (started) {
-    if (runner->StartAgain()) {
+    if (!this->StopTimePassed && runner->StartAgain()) {
       this->Completed--; // remove the completed test because run again
       return;
     }