<br><br><div class="gmail_quote">On Thu, Dec 9, 2010 at 12:04 PM, Tyler Roscoe <span dir="ltr"><<a href="mailto:tyler@cryptio.net">tyler@cryptio.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Ok I've added a link to this thread and the patch below to the bug:<br>
<a href="http://www.vtk.org/Bug/view.php?id=11561" target="_blank">http://www.vtk.org/Bug/view.php?id=11561</a><br>
<br>
Without feedback from anyone, I will assume that I have done an awesome,<br>
production-ready job and will await the call for patches to add to CMake<br>
2.8.4.<br>
<br>
Thanks,<br>
<font color="#888888">tyler<br>
</font><div><div></div><div class="h5"><br>
On Tue, Dec 07, 2010 at 09:37:09PM -0800, Tyler Roscoe wrote:<br>
> In the process of attempting to fix this, I learned a lot of stuff about<br>
> how COST is handled that I've never encountered in the docs. Am I<br>
> missing something?<br>
><br>
> Here are some notes I made about the behavior of COST in CTest. If<br>
> others find them useful, I'd be happy to put them in the Wiki if someone<br>
> could nominate an appropriate place.<br>
><br>
> - Any COST property you set on a test is only a starting point. CTest<br>
> calculates an average cost value for a test each time that test is<br>
> run. This value is cached in Testing/Temporary/CTestCostData.txt.<br>
><br>
> - Tests that fail are also stored in<br>
> Testing/Temporary/CTestCostData.txt. On the next run, these tests have<br>
> their cost set to the maximum to insure that they are run first. I<br>
> believe this factors into later averaging, so that tests that fail more<br>
> frequently run earlier than tests that faill less frequently.<br>
><br>
><br>
><br>
> So, my solution. I've tried to implement Zach's suggested "middle<br>
> ground".<br>
><br>
> For non-parallel CTest runs:<br>
><br>
> - The "run failed tests first" behavior is disabled to prevent failed<br>
> tests from clobbering their given COST property.<br>
><br>
> - The stored values in CTestCostData.txt are not used.<br>
><br>
> - As long as std::sort() is stable, the COST for each test should remain<br>
> 0 and the tests should run in the order encountered in CMakeLists.txt.<br>
><br>
> For parallel CTest runs, failed tests run first and the moving average<br>
> is calculated and used. I think it makes sense that if you ask for tests<br>
> to run in parallel, you probably don't care so much about the order<br>
> (modulo test dependencies) so it is more reasonable to throw out the<br>
> COST data provided by the CMakeLists.txt.<br>
><br>
> I'm not really a C++ dev so please let me know if I'm way off base.<br>
> This patch appears to solve my immediate problem but if it can be<br>
> included upstream that is better for everyone.<br>
><br>
> The patch is against the 2.8.3 release.<br>
><br>
> I've also included a simple CMakeLists.txt for testing and verifying<br>
> behavior. Unpatched ctest 2.8.3 runs the tests in reverse order. Patched<br>
> ctest runs them according to COST.<br>
><br>
> ######################<br>
> ####### PATCH ########<br>
> ######################<br>
><br>
> --- ./Source/CTest/cmCTestMultiProcessHandler.cxx.orig 2010-12-07 15:31:57.091228582 -0800<br>
> +++ ./Source/CTest/cmCTestMultiProcessHandler.cxx 2010-12-07 19:59:11.115740666 -0800<br>
> @@ -434,9 +434,14 @@<br>
> if(index == -1) continue;<br>
><br>
> this->Properties[index]->PreviousRuns = prev;<br>
> - if(this->Properties[index] && this->Properties[index]->Cost == 0)<br>
> + // When not running in parallel mode, don't clobber test's cost with<br>
> + // running average.<br>
> + if(this->ParallelLevel > 1)<br>
> {<br>
> - this->Properties[index]->Cost = cost;<br>
> + if(this->Properties[index] && this->Properties[index]->Cost == 0)<br>
> + {<br>
> + this->Properties[index]->Cost = cost;<br>
> + }<br>
> }<br>
> }<br>
> // Next part of the file is the failed tests<br>
> @@ -475,20 +480,23 @@<br>
> {<br>
> SortedTests.push_back(i->first);<br>
><br>
> - //If the test failed last time, it should be run first, so max the cost<br>
> - if(std::find(this->LastTestsFailed.begin(),<br>
> - this->LastTestsFailed.end(),<br>
> - this->Properties[i->first]->Name)<br>
> - != this->LastTestsFailed.end())<br>
> + //If the test failed last time, it should be run first, so max the cost.<br>
> + //Only do this for parallel runs; in non-parallel runs, avoid clobbering<br>
> + //the test's original cost.<br>
> + if(this->ParallelLevel > 1)<br>
> {<br>
> - this->Properties[i->first]->Cost = FLT_MAX;<br>
> + if(std::find(this->LastTestsFailed.begin(),<br>
> + this->LastTestsFailed.end(),<br>
> + this->Properties[i->first]->Name)<br>
> + != this->LastTestsFailed.end())<br>
> + {<br>
> + this->Properties[i->first]->Cost = FLT_MAX;<br>
> + }<br>
> }<br>
> }<br>
> - if(this->ParallelLevel > 1)<br>
> - {<br>
> +<br>
> TestComparator comp(this);<br>
> std::sort(SortedTests.begin(), SortedTests.end(), comp);<br>
> - }<br>
> }<br>
><br>
> //---------------------------------------------------------<br>
><br>
> ##########################<br>
> ####### TEST CASE ########<br>
> ##########################<br>
><br>
> cmake_minimum_required(VERSION 2.8)<br>
> project(p)<br>
> enable_testing()<br>
><br>
> # Add in reverse order to make sure COST rather than order of add_test()<br>
> # commands really controls execution order.<br>
> add_test (i_should_run_fifth ${CMAKE_COMMAND} -E echo i should run fifth)<br>
> set_tests_properties (i_should_run_fifth PROPERTIES COST -100)<br>
><br>
> add_test (i_should_run_fourth ${CMAKE_COMMAND} -E echo i should run fourth)<br>
> set_tests_properties (i_should_run_fourth PROPERTIES COST -1)<br>
><br>
> add_test (i_should_run_third ${CMAKE_COMMAND} -E echo i should run third)<br>
> set_tests_properties (i_should_run_third PROPERTIES COST 1)<br>
><br>
> add_test (i_should_run_first ${CMAKE_COMMAND} -E echo i should run first)<br>
> set_tests_properties (i_should_run_first PROPERTIES COST 100)<br>
><br>
> # In serial mode, this test should always run second.<br>
> #<br>
> # In parallel mode (ctest -jN, N>1), this test should run second in a fresh<br>
> # binary directory. Otherwise, since it failed previously, it should run first.<br>
> add_test (i_should_fail_and_run_second_or_first ${CMAKE_COMMAND} -E echo i should fail and run second)<br>
> set_tests_properties (i_should_fail_and_run_second_or_first PROPERTIES<br>
> WILL_FAIL TRUE<br>
> COST 50<br>
> )<br>
><br>
><br>
><br>
> On Thu, Dec 02, 2010 at 08:54:12AM -0800, Tyler Roscoe wrote:<br>
> > I've taken the liberty of adding this bug to the tracker:<br>
> > <a href="http://www.cmake.org/Bug/view.php?id=11561" target="_blank">http://www.cmake.org/Bug/view.php?id=11561</a><br>
</div></div><div><div></div><div class="h5">> _______________________________________________<br>
> Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
><br>
> Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
><br>
> Please keep messages on-topic and check the CMake FAQ at: <a href="http://www.cmake.org/Wiki/CMake_FAQ" target="_blank">http://www.cmake.org/Wiki/CMake_FAQ</a><br>
><br>
> Follow this link to subscribe/unsubscribe:<br>
> <a href="http://www.cmake.org/mailman/listinfo/cmake" target="_blank">http://www.cmake.org/mailman/listinfo/cmake</a><br>
</div></div></blockquote></div>Tyler,<div><br></div><div>Thanks for the patch :)<br><br clear="all"><br>-- <br>Zach Mullen<div>R & D Engineer<br>Kitware Inc.<br>(919) 969-6990 x314</div><br>
</div>