Browse Source

Merge topic 'ctest-fix-test-load'

292ec157b6 CTest: Fix --test-load regression

Acked-by: Kitware Robot <[email protected]>
Merge-request: !2362
Brad King 7 years ago
parent
commit
c669b59095

+ 39 - 20
Source/CTest/cmCTestMultiProcessHandler.cxx

@@ -5,7 +5,6 @@
 #include "cmAffinity.h"
 #include "cmAffinity.h"
 #include "cmCTest.h"
 #include "cmCTest.h"
 #include "cmCTestRunTest.h"
 #include "cmCTestRunTest.h"
-#include "cmCTestScriptHandler.h"
 #include "cmCTestTestHandler.h"
 #include "cmCTestTestHandler.h"
 #include "cmSystemTools.h"
 #include "cmSystemTools.h"
 #include "cmWorkingDirectory.h"
 #include "cmWorkingDirectory.h"
@@ -52,6 +51,7 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler()
 {
 {
   this->ParallelLevel = 1;
   this->ParallelLevel = 1;
   this->TestLoad = 0;
   this->TestLoad = 0;
+  this->FakeLoadForTesting = 0;
   this->Completed = 0;
   this->Completed = 0;
   this->RunningCount = 0;
   this->RunningCount = 0;
   this->ProcessorsAvailable = cmAffinity::GetProcessorsAvailable();
   this->ProcessorsAvailable = cmAffinity::GetProcessorsAvailable();
@@ -96,6 +96,16 @@ void cmCTestMultiProcessHandler::SetParallelLevel(size_t level)
 void cmCTestMultiProcessHandler::SetTestLoad(unsigned long load)
 void cmCTestMultiProcessHandler::SetTestLoad(unsigned long load)
 {
 {
   this->TestLoad = load;
   this->TestLoad = load;
+
+  std::string fake_load_value;
+  if (cmSystemTools::GetEnv("__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING",
+                            fake_load_value)) {
+    if (!cmSystemTools::StringToULong(fake_load_value.c_str(),
+                                      &this->FakeLoadForTesting)) {
+      cmSystemTools::Error("Failed to parse fake load value: ",
+                           fake_load_value.c_str());
+    }
+  }
 }
 }
 
 
 void cmCTestMultiProcessHandler::RunTests()
 void cmCTestMultiProcessHandler::RunTests()
@@ -258,12 +268,19 @@ bool cmCTestMultiProcessHandler::StartTest(int test)
 
 
 void cmCTestMultiProcessHandler::StartNextTests()
 void cmCTestMultiProcessHandler::StartNextTests()
 {
 {
-  size_t numToStart = 0;
+  if (this->TestLoadRetryTimer.get() != nullptr) {
+    // This timer may be waiting to call StartNextTests again.
+    // Since we have been called it is no longer needed.
+    uv_timer_stop(this->TestLoadRetryTimer);
+  }
 
 
   if (this->Tests.empty()) {
   if (this->Tests.empty()) {
+    this->TestLoadRetryTimer.reset();
     return;
     return;
   }
   }
 
 
+  size_t numToStart = 0;
+
   if (this->RunningCount < this->ParallelLevel) {
   if (this->RunningCount < this->ParallelLevel) {
     numToStart = this->ParallelLevel - this->RunningCount;
     numToStart = this->ParallelLevel - this->RunningCount;
   }
   }
@@ -279,7 +296,6 @@ void cmCTestMultiProcessHandler::StartNextTests()
   }
   }
 
 
   bool allTestsFailedTestLoadCheck = false;
   bool allTestsFailedTestLoadCheck = false;
-  bool usedFakeLoadForTesting = false;
   size_t minProcessorsRequired = this->ParallelLevel;
   size_t minProcessorsRequired = this->ParallelLevel;
   std::string testWithMinProcessors;
   std::string testWithMinProcessors;
 
 
@@ -292,15 +308,11 @@ void cmCTestMultiProcessHandler::StartNextTests()
     allTestsFailedTestLoadCheck = true;
     allTestsFailedTestLoadCheck = true;
 
 
     // Check for a fake load average value used in testing.
     // Check for a fake load average value used in testing.
-    std::string fake_load_value;
-    if (cmSystemTools::GetEnv("__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING",
-                              fake_load_value)) {
-      usedFakeLoadForTesting = true;
-      if (!cmSystemTools::StringToULong(fake_load_value.c_str(),
-                                        &systemLoad)) {
-        cmSystemTools::Error("Failed to parse fake load value: ",
-                             fake_load_value.c_str());
-      }
+    if (this->FakeLoadForTesting > 0) {
+      systemLoad = this->FakeLoadForTesting;
+      // Drop the fake load for the next iteration to a value low enough
+      // that the next iteration will start tests.
+      this->FakeLoadForTesting = 1;
     }
     }
     // If it's not set, look up the true load average.
     // If it's not set, look up the true load average.
     else {
     else {
@@ -384,18 +396,25 @@ void cmCTestMultiProcessHandler::StartNextTests()
     }
     }
     cmCTestLog(this->CTest, HANDLER_OUTPUT, "*****" << std::endl);
     cmCTestLog(this->CTest, HANDLER_OUTPUT, "*****" << std::endl);
 
 
-    if (usedFakeLoadForTesting) {
-      // Break out of the infinite loop of waiting for our fake load
-      // to come down.
-      this->StopTimePassed = true;
-    } else {
-      // Wait between 1 and 5 seconds before trying again.
-      cmCTestScriptHandler::SleepInSeconds(cmSystemTools::RandomSeed() % 5 +
-                                           1);
+    // Wait between 1 and 5 seconds before trying again.
+    unsigned int milliseconds = (cmSystemTools::RandomSeed() % 5 + 1) * 1000;
+    if (this->FakeLoadForTesting) {
+      milliseconds = 10;
+    }
+    if (this->TestLoadRetryTimer.get() == nullptr) {
+      this->TestLoadRetryTimer.init(this->Loop, this);
     }
     }
+    this->TestLoadRetryTimer.start(
+      &cmCTestMultiProcessHandler::OnTestLoadRetryCB, milliseconds, 0);
   }
   }
 }
 }
 
 
+void cmCTestMultiProcessHandler::OnTestLoadRetryCB(uv_timer_t* timer)
+{
+  auto self = static_cast<cmCTestMultiProcessHandler*>(timer->data);
+  self->StartNextTests();
+}
+
 void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner,
 void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner,
                                                    bool started)
                                                    bool started)
 {
 {

+ 5 - 0
Source/CTest/cmCTestMultiProcessHandler.h

@@ -12,6 +12,7 @@
 #include <string>
 #include <string>
 #include <vector>
 #include <vector>
 
 
+#include "cmUVHandlePtr.h"
 #include "cm_uv.h"
 #include "cm_uv.h"
 
 
 class cmCTest;
 class cmCTest;
@@ -101,6 +102,8 @@ protected:
   void EraseTest(int index);
   void EraseTest(int index);
   void FinishTestProcess(cmCTestRunTest* runner, bool started);
   void FinishTestProcess(cmCTestRunTest* runner, bool started);
 
 
+  static void OnTestLoadRetryCB(uv_timer_t* timer);
+
   void RemoveTest(int index);
   void RemoveTest(int index);
   // Check if we need to resume an interrupted test set
   // Check if we need to resume an interrupted test set
   void CheckResume();
   void CheckResume();
@@ -135,7 +138,9 @@ protected:
   std::vector<cmCTestTestHandler::cmCTestTestResult>* TestResults;
   std::vector<cmCTestTestHandler::cmCTestTestResult>* TestResults;
   size_t ParallelLevel; // max number of process that can be run at once
   size_t ParallelLevel; // max number of process that can be run at once
   unsigned long TestLoad;
   unsigned long TestLoad;
+  unsigned long FakeLoadForTesting;
   uv_loop_t Loop;
   uv_loop_t Loop;
+  cm::uv_timer_ptr TestLoadRetryTimer;
   cmCTestTestHandler* TestHandler;
   cmCTestTestHandler* TestHandler;
   cmCTest* CTest;
   cmCTest* CTest;
   bool HasCycles;
   bool HasCycles;

+ 2 - 2
Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake

@@ -111,8 +111,8 @@ endfunction()
 set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 5)
 set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 5)
 
 
 # Verify that new tests are not started when the load average exceeds
 # Verify that new tests are not started when the load average exceeds
-# our threshold.
-run_TestLoad(test-load-fail 2)
+# our threshold and that they then run once the load average drops.
+run_TestLoad(test-load-wait 3)
 
 
 # Verify that warning message is displayed but tests still start when
 # Verify that warning message is displayed but tests still start when
 # an invalid argument is given.
 # an invalid argument is given.

+ 0 - 1
Tests/RunCMake/CTestCommandLine/test-load-fail-stderr.txt

@@ -1 +0,0 @@
-No tests were found!!!

+ 0 - 2
Tests/RunCMake/CTestCommandLine/test-load-fail-stdout.txt

@@ -1,2 +0,0 @@
-^Test project .*/Tests/RunCMake/CTestCommandLine/TestLoad
-\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 2, Smallest test TestLoad[1-2] requires 1\*\*\*\*\*

+ 0 - 1
Tests/RunCMake/CTestCommandLine/test-load-pass-stderr.txt

@@ -1 +0,0 @@
-^$

+ 8 - 0
Tests/RunCMake/CTestCommandLine/test-load-wait-stdout.txt

@@ -0,0 +1,8 @@
+^Test project .*/Tests/RunCMake/CTestCommandLine/TestLoad
+\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 3, Smallest test TestLoad[1-2] requires 1\*\*\*\*\*
+    Start 1: TestLoad1
+    Start 2: TestLoad2
+1/2 Test #[1-2]: TestLoad[1-2] ........................   Passed +[0-9.]+ sec
+2/2 Test #[1-2]: TestLoad[1-2] ........................   Passed +[0-9.]+ sec
++
+100% tests passed, 0 tests failed out of 2

+ 0 - 1
Tests/RunCMake/ctest_test/CTestTestLoadFail-result.txt

@@ -1 +0,0 @@
-(-1|255)

+ 0 - 1
Tests/RunCMake/ctest_test/CTestTestLoadFail-stderr.txt

@@ -1 +0,0 @@
-No tests were found!!!

+ 0 - 2
Tests/RunCMake/ctest_test/CTestTestLoadFail-stdout.txt

@@ -1,2 +0,0 @@
-Test project .*/Tests/RunCMake/ctest_test/CTestTestLoadFail-build
-\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 4, Smallest test RunCMakeVersion requires 1\*\*\*\*\*$

+ 8 - 0
Tests/RunCMake/ctest_test/CTestTestLoadWait-stdout.txt

@@ -0,0 +1,8 @@
+Test project .*/Tests/RunCMake/ctest_test/CTestTestLoadWait-build
+\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 4, Smallest test RunCMakeVersion requires 1\*\*\*\*\*
+    Start 1: RunCMakeVersion
+1/1 Test #1: RunCMakeVersion ..................   Passed +[0-9.]+ sec
++
+100% tests passed, 0 tests failed out of 1
++
+Total Test time \(real\) = +[0-9.]+ sec$

+ 4 - 4
Tests/RunCMake/ctest_test/RunCMakeTest.cmake

@@ -21,8 +21,8 @@ set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 5)
 run_ctest_test(TestLoadPass TEST_LOAD 6)
 run_ctest_test(TestLoadPass TEST_LOAD 6)
 
 
 # Verify that new tests are not started when the load average exceeds
 # Verify that new tests are not started when the load average exceeds
-# our threshold.
-run_ctest_test(TestLoadFail TEST_LOAD 2)
+# our threshold and that they then run once the load average drops.
+run_ctest_test(TestLoadWait TEST_LOAD 2)
 
 
 # Verify that when an invalid "TEST_LOAD" value is given, a warning
 # Verify that when an invalid "TEST_LOAD" value is given, a warning
 # message is displayed and the value is ignored.
 # message is displayed and the value is ignored.
@@ -34,9 +34,9 @@ set(CASE_CTEST_TEST_LOAD 7)
 run_ctest_test(CTestTestLoadPass)
 run_ctest_test(CTestTestLoadPass)
 
 
 # Verify that new tests are not started when the load average exceeds
 # Verify that new tests are not started when the load average exceeds
-# our threshold.
+# our threshold and that they then run once the load average drops.
 set(CASE_CTEST_TEST_LOAD 4)
 set(CASE_CTEST_TEST_LOAD 4)
-run_ctest_test(CTestTestLoadFail)
+run_ctest_test(CTestTestLoadWait)
 
 
 # Verify that when an invalid "CTEST_TEST_LOAD" value is given,
 # Verify that when an invalid "CTEST_TEST_LOAD" value is given,
 # a warning message is displayed and the value is ignored.
 # a warning message is displayed and the value is ignored.

+ 0 - 1
Tests/RunCMake/ctest_test/TestLoadFail-result.txt

@@ -1 +0,0 @@
-(-1|255)

+ 0 - 1
Tests/RunCMake/ctest_test/TestLoadFail-stderr.txt

@@ -1 +0,0 @@
-No tests were found!!!

+ 0 - 2
Tests/RunCMake/ctest_test/TestLoadFail-stdout.txt

@@ -1,2 +0,0 @@
-Test project .*/Tests/RunCMake/ctest_test/TestLoadFail-build
-\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 2, Smallest test RunCMakeVersion requires 1\*\*\*\*\*$

+ 8 - 0
Tests/RunCMake/ctest_test/TestLoadWait-stdout.txt

@@ -0,0 +1,8 @@
+Test project .*/Tests/RunCMake/ctest_test/TestLoadWait-build
+\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 2, Smallest test RunCMakeVersion requires 1\*\*\*\*\*
+    Start 1: RunCMakeVersion
+1/1 Test #1: RunCMakeVersion ..................   Passed +[0-9.]+ sec
++
+100% tests passed, 0 tests failed out of 1
++
+Total Test time \(real\) = +[0-9.]+ sec$