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

[Tyler Roscoe]

Ok I've added a link to this thread and the patch below to the bug:
http://www.vtk.org/Bug/view.php?id=11561

Without feedback from anyone, I will assume that I have done an awesome,
production-ready job and will await the call for patches to add to CMake
2.8.4.

Thanks,
tyler

On Tue, Dec 07, 2010 at 09:37:09PM -0800, Tyler Roscoe wrote:
> 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
> _______________________________________________
> Powered by www.kitware.com
> 
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
> 
> Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ
> 
> Follow this link to subscribe/unsubscribe:
> http://www.cmake.org/mailman/listinfo/cmake