ENH: Replaced the EXPENSIVE test property with a COST test property taking a floating point value. Tests are now started in descending order of their cost, which defaults to 0 if none is specified.

4 files changed, 72 insertions, 64 deletions

diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index fadf164..41b764d 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -30,25 +30,20 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler()
   // Set the tests
 void 
 cmCTestMultiProcessHandler::SetTests(TestMap& tests,
-                                     TestMap& expensiveTests,
+                                     TestCostMap& testCosts,
                                      PropertiesMap& properties)
 {
+  this->Tests = tests;
+  this->TestCosts = testCosts;
+  this->Properties = properties;
+  this->Total = this->Tests.size();
   // set test run map to false for all
   for(TestMap::iterator i = this->Tests.begin();
       i != this->Tests.end(); ++i)
     {
     this->TestRunningMap[i->first] = false;
     this->TestFinishMap[i->first] = false;
-
-    if(this->Properties[i->first]->Expensive)
-      {
-      this->ExpensiveTests[i->first] = i->second;
-      }
     }
-  this->Tests = tests;
-  this->ExpensiveTests = expensiveTests;
-  this->Properties = properties;
-  this->Total = this->Tests.size();
 }
 
   // Set the max number of tests that can be run at the same time.
@@ -57,6 +52,7 @@ void cmCTestMultiProcessHandler::SetParallelLevel(size_t level)
   this->ParallelLevel = level < 1 ? 1 : level;
 }
 
+//---------------------------------------------------------
 void cmCTestMultiProcessHandler::RunTests()
 {
   if(this->CTest->GetBatchJobs())
@@ -67,7 +63,7 @@ void cmCTestMultiProcessHandler::RunTests()
   this->CheckResume();
   this->TestHandler->SetMaxIndex(this->FindMaxIndex());
   this->StartNextTests();
-  while(this->Tests.size() != 0 || this->ExpensiveTests.size() != 0)
+  while(this->Tests.size() != 0)
     {
     this->CheckOutput();
     this->StartNextTests();
@@ -79,6 +75,7 @@ void cmCTestMultiProcessHandler::RunTests()
   this->MarkFinished();
 }
 
+//---------------------------------------------------------
 void cmCTestMultiProcessHandler::SubmitBatchTests()
 {
   for(cmCTest::CTestConfigurationMap::iterator i =
@@ -90,17 +87,14 @@ void cmCTestMultiProcessHandler::SubmitBatchTests()
     }
 }
 
+//---------------------------------------------------------
 void cmCTestMultiProcessHandler::StartTestProcess(int test)
 {
-  cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, test << ": "
-            << " test " << test << "\n");
+  cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, " test " << test << "\n");
   this->TestRunningMap[test] = true; // mark the test as running
   // now remove the test itself
-  if(this->ExpensiveTests.size() > 0)
-    {
-    this->ExpensiveTests.erase(test);
-    }
-  this->Tests.erase(test);
+  this->EraseTest(test);
+
   cmCTestRunTest* testRun = new cmCTestRunTest;
   testRun->SetCTest(this->CTest);
   testRun->SetTestHandler(this->TestHandler);
@@ -118,6 +112,22 @@ void cmCTestMultiProcessHandler::StartTestProcess(int test)
     }
 }
 
+//---------------------------------------------------------
+void cmCTestMultiProcessHandler::EraseTest(int test)
+{
+  this->Tests.erase(test);
+  for(TestCostMap::iterator i = this->TestCosts.begin();
+      i != this->TestCosts.end(); ++i)
+    {
+    if(i->second.find(test) != i->second.end())
+      {
+      i->second.erase(test);
+      return;
+      }
+    }
+}
+
+//---------------------------------------------------------
 inline size_t cmCTestMultiProcessHandler::GetProcessorsUsed(int test)
 {
   size_t processors = 
@@ -130,10 +140,10 @@ inline size_t cmCTestMultiProcessHandler::GetProcessorsUsed(int test)
     {
     processors = this->ParallelLevel;
     }
-
   return processors;
 }
 
+//---------------------------------------------------------
 bool cmCTestMultiProcessHandler::StartTest(int test)
 {
   // copy the depend tests locally because when 
@@ -176,6 +186,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test)
   return false;
 }
 
+//---------------------------------------------------------
 void cmCTestMultiProcessHandler::StartNextTests()
 {
   size_t numToStart = this->ParallelLevel - this->RunningCount;
@@ -183,36 +194,39 @@ void cmCTestMultiProcessHandler::StartNextTests()
     {
     return;
     }
-  TestMap tests = this->ExpensiveTests.size() > 0 ? 
-    this->ExpensiveTests : this->Tests;
 
-  for(TestMap::iterator i = tests.begin();
-      i !=  tests.end(); ++i)
+  for(TestCostMap::reverse_iterator i = this->TestCosts.rbegin();
+      i != this->TestCosts.rend(); ++i)
     {
-    size_t processors = GetProcessorsUsed(i->first);
-    if(processors > numToStart)
-      {
-      return;
-      }
-    // start test should start only one test
-    if(this->StartTest(i->first))
+    TestSet tests = i->second; //copy the test set
+    for(TestSet::iterator test = tests.begin();
+        test != tests.end(); ++test)
       {
-      numToStart -= processors;
-      this->RunningCount += processors;
-      }
-    else
-      {
-      cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, std::endl
-             << "Test did not start waiting on depends to finish: " 
-                 << i->first << "\n");
-      }
-    if(numToStart == 0 )
-      {
-      return;
+      size_t processors = GetProcessorsUsed(*test);
+      if(processors > numToStart)
+        {
+        return;
+        }
+      if(this->StartTest(*test))
+        {
+        numToStart -= processors;
+        this->RunningCount += processors;
+        }
+      else
+        {
+        cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, std::endl
+                   << "Test did not start waiting on depends to finish: "
+                   << *test << "\n");
+        }
+      if(numToStart == 0)
+        {
+        return;
+        }
       }
     }
 }
 
+//---------------------------------------------------------
 bool cmCTestMultiProcessHandler::CheckOutput()
 {
   // no more output we are done
@@ -248,11 +262,6 @@ bool cmCTestMultiProcessHandler::CheckOutput()
       {
       this->Failed->push_back(p->GetTestProperties()->Name);
       }
-    for(TestMap::iterator j = this->ExpensiveTests.begin();
-        j != this->ExpensiveTests.end(); ++j)
-      {
-      j->second.erase(test);
-      }
     for(TestMap::iterator j = this->Tests.begin();
         j != this->Tests.end(); ++j)
       {
@@ -268,6 +277,7 @@ bool cmCTestMultiProcessHandler::CheckOutput()
   return true;
 }
 
+//---------------------------------------------------------
 void cmCTestMultiProcessHandler::WriteCheckpoint(int index)
 {
   std::string fname = this->CTest->GetBinaryDir()
@@ -354,16 +364,17 @@ void cmCTestMultiProcessHandler::CheckResume()
     }
 }
 
+//---------------------------------------------------------
 void cmCTestMultiProcessHandler::RemoveTest(int index)
 {
-  this->Tests.erase(index);
+  this->EraseTest(index);
   this->Properties.erase(index);
-  this->ExpensiveTests.erase(index);
   this->TestRunningMap[index] = false;
   this->TestFinishMap[index] = true;
   this->Completed++;
 }
 
+//---------------------------------------------------------
 int cmCTestMultiProcessHandler::FindMaxIndex()
 {
   int max = 0;
diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h
index 5d0ae46..97af2f3 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.h
+++ b/Source/CTest/cmCTestMultiProcessHandler.h
@@ -31,12 +31,13 @@ class cmCTestMultiProcessHandler
 public:
   struct TestSet : public std::set<int> {};
   struct TestMap : public std::map<int, TestSet> {};
+  struct TestCostMap : public std::map<float, TestSet> {};
   struct PropertiesMap : public 
      std::map<int, cmCTestTestHandler::cmCTestTestProperties*> {};
 
   cmCTestMultiProcessHandler();
   // Set the tests
-  void SetTests(TestMap& tests, TestMap& expensiveTests,
+  void SetTests(TestMap& tests, TestCostMap& testCosts,
                 PropertiesMap& properties);
   // Set the max number of tests that can be run at the same time.
   void SetParallelLevel(size_t);
@@ -51,9 +52,7 @@ public:
     this->Failed = failed;
     }
   void SetTestResults(std::vector<cmCTestTestHandler::cmCTestTestResult>* r)
-    {
-    this->TestResults = r;
-    }
+  { this->TestResults = r; }
 
   void SetCTest(cmCTest* ctest) { this->CTest = ctest;}
 
@@ -73,6 +72,7 @@ protected:
   void WriteCheckpoint(int index);
   // Removes the checkpoint file
   void MarkFinished();
+  void EraseTest(int index);
   // Return true if there are still tests running
   // check all running processes for output and exit case
   bool CheckOutput();
@@ -83,7 +83,7 @@ protected:
   inline size_t GetProcessorsUsed(int index);
   // map from test number to set of depend tests
   TestMap Tests;
-  TestMap ExpensiveTests;
+  TestCostMap TestCosts;
   //Total number of tests we'll be running
   size_t Total;
   //Number of tests that are complete
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index 6ae227b..3cf2d12 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -1010,7 +1010,7 @@ void cmCTestTestHandler::ProcessDirectory(std::vector<cmStdString> &passed,
     << std::endl;
 
   cmCTestMultiProcessHandler::TestMap tests;
-  cmCTestMultiProcessHandler::TestMap expensiveTests;
+  cmCTestMultiProcessHandler::TestCostMap testCosts;
   cmCTestMultiProcessHandler::PropertiesMap properties;
   
   for (ListOfTests::iterator it = this->TestList.begin();
@@ -1037,12 +1037,9 @@ void cmCTestTestHandler::ProcessDirectory(std::vector<cmStdString> &passed,
       }
     tests[it->Index] = depends;
     properties[it->Index] = &*it;
-    if(it->Expensive)
-      {
-      expensiveTests[it->Index] = depends;
-      }
+    testCosts[p.Cost].insert(p.Index);
     }
-  parallel.SetTests(tests, expensiveTests, properties);
+  parallel.SetTests(tests, testCosts, properties);
   parallel.SetPassFailVectors(&passed, &failed);
   this->TestResults.clear();
   parallel.SetTestResults(&this->TestResults);
@@ -1975,9 +1972,9 @@ bool cmCTestTestHandler::SetTestsProperties(
             {
             rtit->Timeout = atof(val.c_str());
             }
-          if ( key == "EXPENSIVE" )
+          if ( key == "COST" )
             {
-            rtit->Expensive = cmSystemTools::IsOn(val.c_str());
+            rtit->Cost = atof(val.c_str());
             }
           if ( key == "RUN_SERIAL" )
             {
@@ -2130,9 +2127,9 @@ bool cmCTestTestHandler::AddTest(const std::vector<std::string>& args)
   
   test.IsInBasedOnREOptions = true;
   test.WillFail = false;
-  test.Expensive = false;
   test.RunSerial = false;
   test.Timeout = 0;
+  test.Cost = 0;
   test.Processors = 1;
   if (this->UseIncludeRegExpFlag &&
     !this->IncludeTestsRegularExpression.find(testname.c_str()))
diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h
index 113c1c9..ac46e57 100644
--- a/Source/CTest/cmCTestTestHandler.h
+++ b/Source/CTest/cmCTestTestHandler.h
@@ -96,7 +96,7 @@ public:
     std::map<cmStdString, cmStdString> Measurements;
     bool IsInBasedOnREOptions;
     bool WillFail;
-    bool Expensive;
+    float Cost;
     bool RunSerial;
     double Timeout;
     int Index;