cmCTestRunTest: Consolidate test timeout selection logic

Test timeout selection was previously spread out over several locations.
Consolidate it in a single place to make it easier to follow.

5 files changed, 71 insertions, 59 deletions

diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index abd1aa67c9..44eccb2275 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -19,6 +19,7 @@
 #include <vector>
 
 #include <cm/memory>
+#include <cm/optional>
 #include <cmext/algorithm>
 
 #include <cm3p/json/value.h>
@@ -1095,9 +1096,9 @@ static Json::Value DumpCTestProperties(
     properties.append(
       DumpCTestProperty("SKIP_RETURN_CODE", testProperties.SkipReturnCode));
   }
-  if (testProperties.ExplicitTimeout) {
+  if (testProperties.Timeout) {
     properties.append(
-      DumpCTestProperty("TIMEOUT", testProperties.Timeout.count()));
+      DumpCTestProperty("TIMEOUT", testProperties.Timeout->count()));
   }
   if (!testProperties.TimeoutRegularExpressions.empty()) {
     properties.append(DumpCTestProperty(
diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index 46cb54e7a8..cd2b23088a 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -14,12 +14,14 @@
 #include <utility>
 
 #include <cm/memory>
+#include <cm/optional>
 
 #include "cmsys/RegularExpression.hxx"
 
 #include "cmCTest.h"
 #include "cmCTestMemCheckHandler.h"
 #include "cmCTestMultiProcessHandler.h"
+#include "cmDuration.h"
 #include "cmProcess.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
@@ -618,25 +620,7 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total)
   }
   this->StartTime = this->CTest->CurrentTime();
 
-  auto timeout = this->TestProperties->Timeout;
-
-  this->TimeoutIsForStopTime = false;
-  std::chrono::system_clock::time_point stop_time = this->CTest->GetStopTime();
-  if (stop_time != std::chrono::system_clock::time_point()) {
-    std::chrono::duration<double> stop_timeout =
-      (stop_time - std::chrono::system_clock::now()) % std::chrono::hours(24);
-
-    if (stop_timeout <= std::chrono::duration<double>::zero()) {
-      stop_timeout = std::chrono::duration<double>::zero();
-    }
-    if (timeout == std::chrono::duration<double>::zero() ||
-        stop_timeout < timeout) {
-      this->TimeoutIsForStopTime = true;
-      timeout = stop_timeout;
-    }
-  }
-
-  return this->ForkProcess(timeout);
+  return this->ForkProcess();
 }
 
 void cmCTestRunTest::ComputeArguments()
@@ -734,46 +718,79 @@ void cmCTestRunTest::ParseOutputForMeasurements()
   }
 }
 
-bool cmCTestRunTest::ForkProcess(cmDuration testTimeOut)
+bool cmCTestRunTest::ForkProcess()
 {
   this->TestProcess->SetId(this->Index);
   this->TestProcess->SetWorkingDirectory(this->TestProperties->Directory);
   this->TestProcess->SetCommand(this->ActualCommand);
   this->TestProcess->SetCommandArguments(this->Arguments);
 
-  // determine how much time we have
-  cmDuration timeout = this->CTest->GetRemainingTimeAllowed();
-  if (timeout != cmCTest::MaxDuration()) {
-    timeout -= std::chrono::minutes(2);
+  cm::optional<cmDuration> timeout;
+
+  // Check TIMEOUT test property.
+  if (this->TestProperties->Timeout &&
+      *this->TestProperties->Timeout >= cmDuration::zero()) {
+    timeout = this->TestProperties->Timeout;
   }
-  if (this->CTest->GetTimeOut() > cmDuration::zero() &&
-      this->CTest->GetTimeOut() < timeout) {
-    timeout = this->CTest->GetTimeOut();
+
+  // An explicit TIMEOUT=0 test property means "no timeout".
+  if (timeout && *timeout == std::chrono::duration<double>::zero()) {
+    timeout = cm::nullopt;
+  } else {
+    // Check --timeout.
+    if (!timeout && this->CTest->GetGlobalTimeout() > cmDuration::zero()) {
+      timeout = this->CTest->GetGlobalTimeout();
+    }
+
+    // Check CTEST_TEST_TIMEOUT.
+    cmDuration ctestTestTimeout = this->CTest->GetTimeOut();
+    if (ctestTestTimeout > cmDuration::zero() &&
+        (!timeout || ctestTestTimeout < *timeout)) {
+      timeout = ctestTestTimeout;
+    }
+  }
+
+  // Check CTEST_TIME_LIMIT.
+  cmDuration timeRemaining = this->CTest->GetRemainingTimeAllowed();
+  if (timeRemaining != cmCTest::MaxDuration()) {
+    // This two minute buffer is historical.
+    timeRemaining -= std::chrono::minutes(2);
   }
-  if (testTimeOut > cmDuration::zero() &&
-      testTimeOut < this->CTest->GetRemainingTimeAllowed()) {
-    timeout = testTimeOut;
+
+  // Check --stop-time.
+  std::chrono::system_clock::time_point stop_time = this->CTest->GetStopTime();
+  if (stop_time != std::chrono::system_clock::time_point()) {
+    cmDuration timeUntilStop =
+      (stop_time - std::chrono::system_clock::now()) % std::chrono::hours(24);
+    if (timeUntilStop < timeRemaining) {
+      timeRemaining = timeUntilStop;
+    }
   }
-  // always have at least 1 second if we got to here
-  if (timeout <= cmDuration::zero()) {
-    timeout = std::chrono::seconds(1);
+
+  // Enforce remaining time even over explicit TIMEOUT=0.
+  if (timeRemaining <= cmDuration::zero()) {
+    timeRemaining = cmDuration::zero();
   }
-  // handle timeout explicitly set to 0
-  if (testTimeOut == cmDuration::zero() &&
-      this->TestProperties->ExplicitTimeout) {
-    timeout = cmDuration::zero();
+  if (!timeout || timeRemaining < *timeout) {
+    this->TimeoutIsForStopTime = true;
+    timeout = timeRemaining;
   }
-  cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
-                     this->Index << ": "
-                                 << "Test timeout computed to be: "
-                                 << cmDurationTo<unsigned int>(timeout)
-                                 << "\n",
-                     this->TestHandler->GetQuiet());
 
-  // An explicit TIMEOUT=0 test property means "no timeout".
-  if (timeout != cmDuration::zero() ||
-      !this->TestProperties->ExplicitTimeout) {
-    this->TestProcess->SetTimeout(timeout);
+  if (timeout) {
+    cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
+                       this->Index << ": "
+                                   << "Test timeout computed to be: "
+                                   << cmDurationTo<unsigned int>(*timeout)
+                                   << "\n",
+                       this->TestHandler->GetQuiet());
+
+    this->TestProcess->SetTimeout(*timeout);
+  } else {
+    cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
+                       this->Index
+                         << ": "
+                         << "Test timeout suppressed by TIMEOUT property.\n",
+                       this->TestHandler->GetQuiet());
   }
 
   cmSystemTools::SaveRestoreEnvironment sre;
diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h
index b2f4377645..6a507f4fc4 100644
--- a/Source/CTest/cmCTestRunTest.h
+++ b/Source/CTest/cmCTestRunTest.h
@@ -14,7 +14,6 @@
 #include "cmCTest.h"
 #include "cmCTestMultiProcessHandler.h"
 #include "cmCTestTestHandler.h"
-#include "cmDuration.h"
 #include "cmProcess.h"
 
 /** \class cmRunTest
@@ -110,7 +109,7 @@ private:
   bool NeedsToRepeat();
   void ParseOutputForMeasurements();
   void ExeNotFound(std::string exe);
-  bool ForkProcess(cmDuration testTimeOut);
+  bool ForkProcess();
   void WriteLogOutputTop(size_t completed, size_t total);
   // Run post processing of the process output for MemCheck
   void MemCheckPostProcess();
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index cd92e0ab09..7764f2b7c1 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -1379,11 +1379,6 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector<std::string>& passed,
       p.Cost = static_cast<float>(rand());
     }
 
-    if (p.Timeout == cmDuration::zero() && !p.ExplicitTimeout &&
-        this->CTest->GetGlobalTimeout() != cmDuration::zero()) {
-      p.Timeout = this->CTest->GetGlobalTimeout();
-    }
-
     if (!p.Depends.empty()) {
       for (std::string const& i : p.Depends) {
         for (cmCTestTestProperties const& it2 : this->TestList) {
@@ -2252,7 +2247,6 @@ bool cmCTestTestHandler::SetTestsProperties(
             rt.FixturesRequired.insert(lval.begin(), lval.end());
           } else if (key == "TIMEOUT"_s) {
             rt.Timeout = cmDuration(atof(val.c_str()));
-            rt.ExplicitTimeout = true;
           } else if (key == "COST"_s) {
             rt.Cost = static_cast<float>(atof(val.c_str()));
           } else if (key == "REQUIRED_FILES"_s) {
diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h
index 29d24e08e9..b7c0fafbde 100644
--- a/Source/CTest/cmCTestTestHandler.h
+++ b/Source/CTest/cmCTestTestHandler.h
@@ -14,6 +14,8 @@
 #include <utility>
 #include <vector>
 
+#include <cm/optional>
+
 #include "cmsys/RegularExpression.hxx"
 
 #include "cmCTest.h"
@@ -145,8 +147,7 @@ public:
     float Cost = 0;
     int PreviousRuns = 0;
     bool RunSerial = false;
-    cmDuration Timeout = cmDuration::zero();
-    bool ExplicitTimeout = false;
+    cm::optional<cmDuration> Timeout;
     cmDuration AlternateTimeout;
     int Index = 0;
     // Requested number of process slots