[CMake] test property COST not working in cmake 2.8.3?

[Tyler Roscoe]

In the process of attempting to fix this, I learned a lot of stuff about
how COST is handled that I've never encountered in the docs. Am I
missing something?

Here are some notes I made about the behavior of COST in CTest. If
others find them useful, I'd be happy to put them in the Wiki if someone
could nominate an appropriate place.

- Any COST property you set on a test is only a starting point. CTest
  calculates an average cost value for a test each time that test is
run. This value is cached in Testing/Temporary/CTestCostData.txt.

- Tests that fail are also stored in
  Testing/Temporary/CTestCostData.txt. On the next run, these tests have
their cost set to the maximum to insure that they are run first. I
believe this factors into later averaging, so that tests that fail more
frequently run earlier than tests that faill less frequently. 



So, my solution. I've tried to implement Zach's suggested "middle
ground". 

For non-parallel CTest runs:

- The "run failed tests first" behavior is disabled to prevent failed
  tests from clobbering their given COST property. 

- The stored values in CTestCostData.txt are not used. 

- As long as std::sort() is stable, the COST for each test should remain
  0 and the tests should run in the order encountered in CMakeLists.txt.

For parallel CTest runs, failed tests run first and the moving average
is calculated and used. I think it makes sense that if you ask for tests
to run in parallel, you probably don't care so much about the order
(modulo test dependencies) so it is more reasonable to throw out the
COST data provided by the CMakeLists.txt.

I'm not really a C++ dev so please let me know if I'm way off base.
This patch appears to solve my immediate problem but if it can be
included upstream that is better for everyone.

The patch is against the 2.8.3 release.

I've also included a simple CMakeLists.txt for testing and verifying
behavior. Unpatched ctest 2.8.3 runs the tests in reverse order. Patched
ctest runs them according to COST.

######################
####### PATCH ########
######################

--- ./Source/CTest/cmCTestMultiProcessHandler.cxx.orig  2010-12-07 15:31:57.091228582 -0800
+++ ./Source/CTest/cmCTestMultiProcessHandler.cxx       2010-12-07 19:59:11.115740666 -0800
@@ -434,9 +434,14 @@
       if(index == -1) continue;

       this->Properties[index]->PreviousRuns = prev;
-      if(this->Properties[index] && this->Properties[index]->Cost == 0)
+      // When not running in parallel mode, don't clobber test's cost with
+      // running average.
+      if(this->ParallelLevel > 1)
         {
-        this->Properties[index]->Cost = cost;
+        if(this->Properties[index] && this->Properties[index]->Cost == 0)
+          {
+          this->Properties[index]->Cost = cost;
+          }
         }
       }
     // Next part of the file is the failed tests
@@ -475,20 +480,23 @@
     {
     SortedTests.push_back(i->first);

-    //If the test failed last time, it should be run first, so max the cost
-    if(std::find(this->LastTestsFailed.begin(),
-                 this->LastTestsFailed.end(),
-                 this->Properties[i->first]->Name)
-       != this->LastTestsFailed.end())
+    //If the test failed last time, it should be run first, so max the cost.
+    //Only do this for parallel runs; in non-parallel runs, avoid clobbering
+    //the test's original cost.
+    if(this->ParallelLevel > 1)
       {
-      this->Properties[i->first]->Cost = FLT_MAX;
+      if(std::find(this->LastTestsFailed.begin(),
+                   this->LastTestsFailed.end(),
+                   this->Properties[i->first]->Name)
+         != this->LastTestsFailed.end())
+        {
+        this->Properties[i->first]->Cost = FLT_MAX;
+        }
       }
     }
-  if(this->ParallelLevel > 1)
-    {
+
     TestComparator comp(this);
     std::sort(SortedTests.begin(), SortedTests.end(), comp);
-    }
 }

 //---------------------------------------------------------

##########################
####### TEST CASE ########
##########################

cmake_minimum_required(VERSION 2.8)
project(p)
enable_testing()

# Add in reverse order to make sure COST rather than order of add_test()
# commands really controls execution order.
add_test (i_should_run_fifth ${CMAKE_COMMAND} -E echo i should run fifth)
set_tests_properties (i_should_run_fifth PROPERTIES COST -100)

add_test (i_should_run_fourth ${CMAKE_COMMAND} -E echo i should run fourth)
set_tests_properties (i_should_run_fourth PROPERTIES COST -1)

add_test (i_should_run_third ${CMAKE_COMMAND} -E echo i should run third)
set_tests_properties (i_should_run_third PROPERTIES COST 1)

add_test (i_should_run_first ${CMAKE_COMMAND} -E echo i should run first)
set_tests_properties (i_should_run_first PROPERTIES COST 100)

# In serial mode, this test should always run second.
#
# In parallel mode (ctest -jN, N>1), this test should run second in a fresh
# binary directory. Otherwise, since it failed previously, it should run first.
add_test (i_should_fail_and_run_second_or_first ${CMAKE_COMMAND} -E echo i should fail and run second)
set_tests_properties (i_should_fail_and_run_second_or_first PROPERTIES
    WILL_FAIL TRUE
    COST 50
)



On Thu, Dec 02, 2010 at 08:54:12AM -0800, Tyler Roscoe wrote:
> I've taken the liberty of adding this bug to the tracker:
> http://www.cmake.org/Bug/view.php?id=11561