On Thu, Dec 9, 2010 at 12:04 PM, Tyler Roscoe <ty...@cryptio.net> wrote:
> 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 > Tyler, Thanks for the patch :) -- Zach Mullen R & D Engineer Kitware Inc. (919) 969-6990 x314
_______________________________________________ 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