Merge topic 'ctest-timeout-signal'

54c5654f7d ctest: Optionally terminate tests with a custom signal on timeout
e38c05688e CTest/cmProcess: Adopt field tracking reason for the process timeout
25c1468314 cmCTestTestHandler: Remove outdated comment
41e8507ab7 IWYU: Add mapping for bits/chrono.h to chrono

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !8472

7 files changed, 154 insertions, 17 deletions

diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index 44eccb2..7d22a87 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -634,8 +634,9 @@ void cmCTestMultiProcessHandler::FinishTestProcess(
   int test = runner->GetIndex();
   auto* properties = runner->GetTestProperties();
 
-  bool testResult = runner->EndTest(this->Completed, this->Total, started);
-  if (runner->TimedOutForStopTime()) {
+  cmCTestRunTest::EndTestResult testResult =
+    runner->EndTest(this->Completed, this->Total, started);
+  if (testResult.StopTimePassed) {
     this->SetStopTimePassed();
   }
   if (started) {
@@ -646,7 +647,7 @@ void cmCTestMultiProcessHandler::FinishTestProcess(
     }
   }
 
-  if (testResult) {
+  if (testResult.Passed) {
     this->Passed->push_back(properties->Name);
   } else if (!properties->Disabled) {
     this->Failed->push_back(properties->Name);
diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index cd2b230..9b62183 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -95,16 +95,15 @@ void cmCTestRunTest::CheckOutput(std::string const& line)
   }
 }
 
-bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started)
+cmCTestRunTest::EndTestResult cmCTestRunTest::EndTest(size_t completed,
+                                                      size_t total,
+                                                      bool started)
 {
   this->WriteLogOutputTop(completed, total);
   std::string reason;
   bool passed = true;
   cmProcess::State res =
     started ? this->TestProcess->GetProcessStatus() : cmProcess::State::Error;
-  if (res != cmProcess::State::Expired) {
-    this->TimeoutIsForStopTime = false;
-  }
   std::int64_t retVal = this->TestProcess->GetExitValue();
   bool forceFail = false;
   bool forceSkip = false;
@@ -182,6 +181,11 @@ bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started)
     }
   } else if (res == cmProcess::State::Expired) {
     outputStream << "***Timeout ";
+    if (this->TestProperties->TimeoutSignal &&
+        this->TestProcess->GetTerminationStyle() ==
+          cmProcess::Termination::Custom) {
+      outputStream << "(" << this->TestProperties->TimeoutSignal->Name << ") ";
+    }
     this->TestResult.Status = cmCTestTestHandler::TIMEOUT;
     outputTestErrorsToConsole =
       this->CTest->GetOutputTestOutputOnTestFailure();
@@ -344,8 +348,15 @@ bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started)
   if (!this->NeedsToRepeat()) {
     this->TestHandler->TestResults.push_back(this->TestResult);
   }
+  cmCTestRunTest::EndTestResult testResult;
+  testResult.Passed = passed || skipped;
+  if (res == cmProcess::State::Expired &&
+      this->TestProcess->GetTimeoutReason() ==
+        cmProcess::TimeoutReason::StopTime) {
+    testResult.StopTimePassed = true;
+  }
   this->TestProcess.reset();
-  return passed || skipped;
+  return testResult;
 }
 
 bool cmCTestRunTest::StartAgain(std::unique_ptr<cmCTestRunTest> runner,
@@ -534,6 +545,19 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total)
   this->TestResult.Name = this->TestProperties->Name;
   this->TestResult.Path = this->TestProperties->Directory;
 
+  // Reject invalid test properties.
+  if (this->TestProperties->Error) {
+    std::string const& msg = *this->TestProperties->Error;
+    *this->TestHandler->LogFile << msg << std::endl;
+    cmCTestLog(this->CTest, HANDLER_OUTPUT, msg << std::endl);
+    this->TestResult.CompletionStatus = "Invalid Test Properties";
+    this->TestResult.Status = cmCTestTestHandler::NOT_RUN;
+    this->TestResult.Output = msg;
+    this->TestResult.FullCommandLine.clear();
+    this->TestResult.Environment.clear();
+    return false;
+  }
+
   // Return immediately if test is disabled
   if (this->TestProperties->Disabled) {
     this->TestResult.CompletionStatus = "Disabled";
@@ -772,8 +796,8 @@ bool cmCTestRunTest::ForkProcess()
     timeRemaining = cmDuration::zero();
   }
   if (!timeout || timeRemaining < *timeout) {
-    this->TimeoutIsForStopTime = true;
     timeout = timeRemaining;
+    this->TestProcess->SetTimeoutReason(cmProcess::TimeoutReason::StopTime);
   }
 
   if (timeout) {
diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h
index 6a507f4..fed7296 100644
--- a/Source/CTest/cmCTestRunTest.h
+++ b/Source/CTest/cmCTestRunTest.h
@@ -71,10 +71,16 @@ public:
                            std::string const& output,
                            std::string const& detail);
 
+  struct EndTestResult
+  {
+    bool Passed = false;
+    bool StopTimePassed = false;
+  };
+
   // launch the test process, return whether it started correctly
   bool StartTest(size_t completed, size_t total);
   // capture and report the test results
-  bool EndTest(size_t completed, size_t total, bool started);
+  EndTestResult EndTest(size_t completed, size_t total, bool started);
   // Called by ctest -N to log the command string
   void ComputeArguments();
 
@@ -90,8 +96,6 @@ public:
 
   void FinalizeTest(bool started = true);
 
-  bool TimedOutForStopTime() const { return this->TimeoutIsForStopTime; }
-
   void SetUseAllocatedResources(bool use)
   {
     this->UseAllocatedResources = use;
@@ -120,7 +124,6 @@ private:
   std::string GetTestPrefix(size_t completed, size_t total) const;
 
   cmCTestTestHandler::cmCTestTestProperties* TestProperties;
-  bool TimeoutIsForStopTime = false;
   // Pointer back to the "parent"; the handler that invoked this test run
   cmCTestTestHandler* TestHandler;
   cmCTest* CTest;
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index 7764f2b..6b02a5e 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -17,6 +17,10 @@
 #include <sstream>
 #include <utility>
 
+#ifndef _WIN32
+#  include <csignal>
+#endif
+
 #include <cm/memory>
 #include <cm/string_view>
 #include <cmext/algorithm>
@@ -2171,6 +2175,16 @@ void cmCTestTestHandler::CleanTestOutput(std::string& output, size_t length,
   }
 }
 
+void cmCTestTestHandler::cmCTestTestProperties::AppendError(
+  cm::string_view err)
+{
+  if (this->Error) {
+    *this->Error = cmStrCat(*this->Error, '\n', err);
+  } else {
+    this->Error = err;
+  }
+}
+
 bool cmCTestTestHandler::SetTestsProperties(
   const std::vector<std::string>& args)
 {
@@ -2247,6 +2261,53 @@ bool cmCTestTestHandler::SetTestsProperties(
             rt.FixturesRequired.insert(lval.begin(), lval.end());
           } else if (key == "TIMEOUT"_s) {
             rt.Timeout = cmDuration(atof(val.c_str()));
+          } else if (key == "TIMEOUT_SIGNAL_NAME"_s) {
+#ifdef _WIN32
+            rt.AppendError("TIMEOUT_SIGNAL_NAME is not supported on Windows.");
+#else
+            std::string const& signalName = val;
+            Signal s;
+            if (signalName == "SIGINT"_s) {
+              s.Number = SIGINT;
+            } else if (signalName == "SIGQUIT"_s) {
+              s.Number = SIGQUIT;
+            } else if (signalName == "SIGTERM"_s) {
+              s.Number = SIGTERM;
+            } else if (signalName == "SIGUSR1"_s) {
+              s.Number = SIGUSR1;
+            } else if (signalName == "SIGUSR2"_s) {
+              s.Number = SIGUSR2;
+            }
+            if (s.Number) {
+              s.Name = signalName;
+              rt.TimeoutSignal = std::move(s);
+            } else {
+              rt.AppendError(cmStrCat("TIMEOUT_SIGNAL_NAME \"", signalName,
+                                      "\" not supported on this platform."));
+            }
+#endif
+          } else if (key == "TIMEOUT_SIGNAL_GRACE_PERIOD"_s) {
+#ifdef _WIN32
+            rt.AppendError(
+              "TIMEOUT_SIGNAL_GRACE_PERIOD is not supported on Windows.");
+#else
+            std::string const& gracePeriod = val;
+            static cmDuration minGracePeriod{ 0 };
+            static cmDuration maxGracePeriod{ 60 };
+            cmDuration gp = cmDuration(atof(gracePeriod.c_str()));
+            if (gp <= minGracePeriod) {
+              rt.AppendError(cmStrCat("TIMEOUT_SIGNAL_GRACE_PERIOD \"",
+                                      gracePeriod, "\" is not greater than \"",
+                                      minGracePeriod.count(), "\" seconds."));
+            } else if (gp > maxGracePeriod) {
+              rt.AppendError(cmStrCat("TIMEOUT_SIGNAL_GRACE_PERIOD \"",
+                                      gracePeriod,
+                                      "\" is not less than the maximum of \"",
+                                      maxGracePeriod.count(), "\" seconds."));
+            } else {
+              rt.TimeoutGracePeriod = gp;
+            }
+#endif
           } 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 b7c0faf..315a5b7 100644
--- a/Source/CTest/cmCTestTestHandler.h
+++ b/Source/CTest/cmCTestTestHandler.h
@@ -15,6 +15,7 @@
 #include <vector>
 
 #include <cm/optional>
+#include <cm/string_view>
 
 #include "cmsys/RegularExpression.hxx"
 
@@ -119,12 +120,16 @@ public:
     bool operator!=(const cmCTestTestResourceRequirement& other) const;
   };
 
-  // NOTE: This struct is Saved/Restored
-  // in cmCTestTestHandler, if you add to this class
-  // then you must add the new members to that code or
-  // ctest -j N will break for that feature
+  struct Signal
+  {
+    int Number = 0;
+    std::string Name;
+  };
+
   struct cmCTestTestProperties
   {
+    void AppendError(cm::string_view err);
+    cm::optional<std::string> Error;
     std::string Name;
     std::string Directory;
     std::vector<std::string> Args;
@@ -148,6 +153,8 @@ public:
     int PreviousRuns = 0;
     bool RunSerial = false;
     cm::optional<cmDuration> Timeout;
+    cm::optional<Signal> TimeoutSignal;
+    cm::optional<cmDuration> TimeoutGracePeriod;
     cmDuration AlternateTimeout;
     int Index = 0;
     // Requested number of process slots
diff --git a/Source/CTest/cmProcess.cxx b/Source/CTest/cmProcess.cxx
index 780d626..269b92c 100644
--- a/Source/CTest/cmProcess.cxx
+++ b/Source/CTest/cmProcess.cxx
@@ -13,6 +13,7 @@
 
 #include "cmCTest.h"
 #include "cmCTestRunTest.h"
+#include "cmCTestTestHandler.h"
 #include "cmGetPipes.h"
 #include "cmStringAlgorithms.h"
 #if defined(_WIN32)
@@ -274,7 +275,29 @@ void cmProcess::OnTimeoutCB(uv_timer_t* timer)
 
 void cmProcess::OnTimeout()
 {
+  bool const wasExecuting = this->ProcessState == cmProcess::State::Executing;
   this->ProcessState = cmProcess::State::Expired;
+
+  // If the test process is still executing normally, and we timed out because
+  // the test timeout was reached, send the custom timeout signal, if any.
+  if (wasExecuting && this->TimeoutReason_ == TimeoutReason::Normal) {
+    cmCTestTestHandler::cmCTestTestProperties* p =
+      this->Runner->GetTestProperties();
+    if (p->TimeoutSignal) {
+      this->TerminationStyle = Termination::Custom;
+      uv_process_kill(this->Process, p->TimeoutSignal->Number);
+      if (p->TimeoutGracePeriod) {
+        this->Timeout = *p->TimeoutGracePeriod;
+      } else {
+        static const cmDuration defaultGracePeriod{ 1.0 };
+        this->Timeout = defaultGracePeriod;
+      }
+      this->StartTimer();
+      return;
+    }
+  }
+
+  this->TerminationStyle = Termination::Forced;
   bool const was_still_reading = !this->ReadHandleClosed;
   if (!this->ReadHandleClosed) {
     this->ReadHandleClosed = true;
diff --git a/Source/CTest/cmProcess.h b/Source/CTest/cmProcess.h
index 1578687..dc755eb 100644
--- a/Source/CTest/cmProcess.h
+++ b/Source/CTest/cmProcess.h
@@ -41,6 +41,14 @@ public:
   // Return true if the process starts
   bool StartProcess(uv_loop_t& loop, std::vector<size_t>* affinity);
 
+  enum class TimeoutReason
+  {
+    Normal,
+    StopTime,
+  };
+  void SetTimeoutReason(TimeoutReason r) { this->TimeoutReason_ = r; }
+  TimeoutReason GetTimeoutReason() const { return this->TimeoutReason_; }
+
   enum class State
   {
     Starting,
@@ -77,8 +85,17 @@ public:
     return std::move(this->Runner);
   }
 
+  enum class Termination
+  {
+    Normal,
+    Custom,
+    Forced,
+  };
+  Termination GetTerminationStyle() const { return this->TerminationStyle; }
+
 private:
   cm::optional<cmDuration> Timeout;
+  TimeoutReason TimeoutReason_ = TimeoutReason::Normal;
   std::chrono::steady_clock::time_point StartTime;
   cmDuration TotalTime;
   bool ReadHandleClosed = false;
@@ -128,4 +145,5 @@ private:
   std::vector<const char*> ProcessArgs;
   int Id;
   int64_t ExitValue;
+  Termination TerminationStyle = Termination::Normal;
 };