From 44017a4767e183d93b4c3f8f9c3000bbc4e33d2b Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Thu, 17 Oct 2013 22:10:35 +0200 Subject: CTest: handle dependent and non dependent test requirements equally --- Source/CTest/cmCTestMultiProcessHandler.cxx | 49 ++++++----------------------- 1 file changed, 9 insertions(+), 40 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 76ddeea..794ce7a 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -210,30 +210,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test) // and we don't want to be iterating a list while removing from it TestSet depends = this->Tests[test]; size_t totalDepends = depends.size(); - if(totalDepends) - { - for(TestSet::const_iterator i = depends.begin(); - i != depends.end(); ++i) - { - // if the test is not already running then start it - if(!this->TestRunningMap[*i]) - { - // this test might be finished, but since - // this is a copy of the depend map we might - // still have it - if(!this->TestFinishMap[*i]) - { - // only start one test in this function - return this->StartTest(*i); - } - else - { - // the depend has been and finished - totalDepends--; - } - } - } - } + // if there are no depends left then run this test if(totalDepends == 0) { @@ -262,25 +239,17 @@ void cmCTestMultiProcessHandler::StartNextTests() TestList copy = this->SortedTests; for(TestList::iterator test = copy.begin(); test != copy.end(); ++test) { - //in case this test has already been started due to dependency - if(this->TestRunningMap[*test] || this->TestFinishMap[*test]) - { - continue; - } size_t processors = GetProcessorsUsed(*test); - if(processors > numToStart) - { - return; - } - if(this->StartTest(*test)) + + if(processors <= numToStart && this->StartTest(*test)) { - if(this->StopTimePassed) - { - return; - } - numToStart -= processors; + if(this->StopTimePassed) + { + return; + } + numToStart -= processors; } - if(numToStart == 0) + else if(numToStart == 0) { return; } -- cgit v0.12 From e809d8cfdf080e0f34122ef4aac0b1a93c6a7f6a Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Sat, 19 Oct 2013 00:38:35 +0200 Subject: CTest: prioritize tests by their depth in the dependency graph --- Source/CTest/cmCTestMultiProcessHandler.cxx | 49 ++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 794ce7a..e1f829d 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -441,11 +441,41 @@ int cmCTestMultiProcessHandler::SearchByName(std::string name) //--------------------------------------------------------- void cmCTestMultiProcessHandler::CreateTestCostList() { + std::list priorityStack; + priorityStack.push_back(TestSet()); + TestSet &topLevel = priorityStack.back(); + + for(TestMap::const_iterator i = this->Tests.begin(); + i != this->Tests.end(); ++i) + { + topLevel.insert(i->first); + } + + while(priorityStack.back().size()) + { + TestSet &previousSet = priorityStack.back(); + priorityStack.push_back(TestSet()); + TestSet ¤tSet = priorityStack.back(); + + for(TestSet::iterator i = previousSet.begin(); + i != previousSet.end(); ++i) + { + TestSet const& dependencies = this->Tests[*i]; + currentSet.insert(dependencies.begin(), dependencies.end()); + } + + for(TestSet::iterator i = currentSet.begin(); + i != currentSet.end(); ++i) + { + previousSet.erase(*i); + } + } + + priorityStack.pop_back(); + for(TestMap::iterator i = this->Tests.begin(); i != this->Tests.end(); ++i) { - SortedTests.push_back(i->first); - //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 explicitly set cost. @@ -457,8 +487,19 @@ void cmCTestMultiProcessHandler::CreateTestCostList() } } - TestComparator comp(this); - std::stable_sort(SortedTests.begin(), SortedTests.end(), comp); + for(std::list::reverse_iterator i = priorityStack.rbegin(); + i != priorityStack.rend(); ++i) + { + TestSet ¤tSet = *i; + TestComparator comp(this); + + TestList sortedCopy; + sortedCopy.insert(sortedCopy.end(), currentSet.begin(), currentSet.end()); + std::stable_sort(sortedCopy.begin(), sortedCopy.end(), comp); + + this->SortedTests.insert(this->SortedTests.end(), + sortedCopy.begin(), sortedCopy.end()); + } } //--------------------------------------------------------- -- cgit v0.12 From 6d4d7ca9551e60592003272a3a2039af4dee192e Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Sat, 19 Oct 2013 08:52:28 +0200 Subject: CTest: consider previously failed tests before all others --- Source/CTest/cmCTestMultiProcessHandler.cxx | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index e1f829d..a9aaf0c 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -448,7 +448,16 @@ void cmCTestMultiProcessHandler::CreateTestCostList() for(TestMap::const_iterator i = this->Tests.begin(); i != this->Tests.end(); ++i) { - topLevel.insert(i->first); + 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. + this->SortedTests.push_back(i->first); + } + else + { + topLevel.insert(i->first); + } } while(priorityStack.back().size()) @@ -473,20 +482,6 @@ void cmCTestMultiProcessHandler::CreateTestCostList() priorityStack.pop_back(); - for(TestMap::iterator i = this->Tests.begin(); - i != this->Tests.end(); ++i) - { - //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 explicitly set cost. - if(this->ParallelLevel > 1 && - std::find(this->LastTestsFailed.begin(), this->LastTestsFailed.end(), - this->Properties[i->first]->Name) != this->LastTestsFailed.end()) - { - this->Properties[i->first]->Cost = FLT_MAX; - } - } - for(std::list::reverse_iterator i = priorityStack.rbegin(); i != priorityStack.rend(); ++i) { -- cgit v0.12 From 1b750cbf9a77388b96b8fd3420c602d98a2e0ab9 Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Sat, 19 Oct 2013 09:28:18 +0200 Subject: CTest: perform cycle test early --- Source/CTest/cmCTestMultiProcessHandler.cxx | 8 +++++++- Source/CTest/cmCTestMultiProcessHandler.h | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index a9aaf0c..33e88b2 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -41,6 +41,7 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler() this->Completed = 0; this->RunningCount = 0; this->StopTimePassed = false; + this->HasCycles = false; } cmCTestMultiProcessHandler::~cmCTestMultiProcessHandler() @@ -65,6 +66,11 @@ cmCTestMultiProcessHandler::SetTests(TestMap& tests, if(!this->CTest->GetShowOnly()) { this->ReadCostData(); + this->HasCycles = !this->CheckCycles(); + if(this->HasCycles) + { + return; + } this->CreateTestCostList(); } } @@ -79,7 +85,7 @@ void cmCTestMultiProcessHandler::SetParallelLevel(size_t level) void cmCTestMultiProcessHandler::RunTests() { this->CheckResume(); - if(!this->CheckCycles()) + if(this->HasCycles) { return; } diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index cd21d91..439a8f3 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -111,6 +111,7 @@ protected: std::set RunningTests; // current running tests cmCTestTestHandler * TestHandler; cmCTest* CTest; + bool HasCycles; }; #endif -- cgit v0.12 From adbe00d6e10906466c82ac3ddc8ef2d14a98a417 Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Mon, 21 Oct 2013 19:32:53 +0200 Subject: CTest: removed redundant copy of test dependency set --- Source/CTest/cmCTestMultiProcessHandler.cxx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 33e88b2..510d3c5 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -211,14 +211,8 @@ bool cmCTestMultiProcessHandler::StartTest(int test) } } - // copy the depend tests locally because when - // a test is finished it will be removed from the depend list - // and we don't want to be iterating a list while removing from it - TestSet depends = this->Tests[test]; - size_t totalDepends = depends.size(); - // if there are no depends left then run this test - if(totalDepends == 0) + if(this->Tests[test].empty()) { this->StartTestProcess(test); return true; -- cgit v0.12 From 384beffc39a00c97ff23a8149e62af5b1cffc0ae Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Mon, 21 Oct 2013 19:55:14 +0200 Subject: CTest: added comments that describe the basic test sorting approach --- Source/CTest/cmCTestMultiProcessHandler.cxx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 510d3c5..3dd446b 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -445,6 +445,8 @@ void cmCTestMultiProcessHandler::CreateTestCostList() priorityStack.push_back(TestSet()); TestSet &topLevel = priorityStack.back(); + // Add previously failed tests to the front of the cost list + // and queue other tests for further sorting for(TestMap::const_iterator i = this->Tests.begin(); i != this->Tests.end(); ++i) { @@ -460,6 +462,8 @@ void cmCTestMultiProcessHandler::CreateTestCostList() } } + // Repeatedly move dependencies of the tests on the current dependency level + // to the next level until no further dependencies exist. while(priorityStack.back().size()) { TestSet &previousSet = priorityStack.back(); @@ -480,8 +484,11 @@ void cmCTestMultiProcessHandler::CreateTestCostList() } } + // Remove the empty dependency level priorityStack.pop_back(); + // Reverse iterate over the different dependency levels (deepest first). + // Sort tests within each level by COST and append them to the cost list. for(std::list::reverse_iterator i = priorityStack.rbegin(); i != priorityStack.rend(); ++i) { -- cgit v0.12 From 7a665ae7e375db01e3eb89bb65967d03103cf164 Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Mon, 21 Oct 2013 22:13:22 +0200 Subject: CTest: added test for RUN_SERIAL issue #14484 --- Tests/CMakeLists.txt | 2 ++ Tests/CTestTestSerialInDepends/CMakeLists.txt | 21 +++++++++++++++++++++ Tests/CTestTestSerialInDepends/test.ctest | 16 ++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 Tests/CTestTestSerialInDepends/CMakeLists.txt create mode 100644 Tests/CTestTestSerialInDepends/test.ctest diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 2f6a456..0d35104 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -2074,6 +2074,8 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --output-log "${CMake_BINARY_DIR}/Tests/CTestTestParallel/testOutput.log" ) + ADD_TEST_MACRO(CTestTestSerialInDepends ${CMAKE_CTEST_COMMAND} -j 4) + if(NOT BORLAND) set(CTestLimitDashJ_EXTRA_OPTIONS --force-new-ctest-process) add_test_macro(CTestLimitDashJ ${CMAKE_CTEST_COMMAND} -j 4 diff --git a/Tests/CTestTestSerialInDepends/CMakeLists.txt b/Tests/CTestTestSerialInDepends/CMakeLists.txt new file mode 100644 index 0000000..f99acab --- /dev/null +++ b/Tests/CTestTestSerialInDepends/CMakeLists.txt @@ -0,0 +1,21 @@ +cmake_minimum_required(VERSION 2.8.12) + +enable_testing() + +function(my_add_test NAME COST) + add_test(NAME ${NAME} + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + COMMAND ${CMAKE_CTEST_COMMAND} -DTEST_NAME=${NAME} + -S ${CMAKE_CURRENT_SOURCE_DIR}/test.ctest) + set_tests_properties(${NAME} PROPERTIES COST ${COST}) +endfunction() + +my_add_test(i_like_company 1000) +my_add_test(i_like_company_too 0) + +my_add_test(i_have_dependencies 1000) +set_tests_properties(i_have_dependencies PROPERTIES + DEPENDS "i_want_to_be_alone") + +my_add_test(i_want_to_be_alone 100) +set_tests_properties(i_want_to_be_alone PROPERTIES RUN_SERIAL 1) diff --git a/Tests/CTestTestSerialInDepends/test.ctest b/Tests/CTestTestSerialInDepends/test.ctest new file mode 100644 index 0000000..28ee094 --- /dev/null +++ b/Tests/CTestTestSerialInDepends/test.ctest @@ -0,0 +1,16 @@ +set(CTEST_RUN_CURRENT_SCRIPT 0) + +set(LOCK_FILE "${TEST_NAME}.lock") + +if("${TEST_NAME}" STREQUAL "i_want_to_be_alone") + file(GLOB LOCK_FILES *.lock) + if(LOCK_FILES) + message(FATAL_ERROR "found lock files of other tests even though this test should be running by itself: ${LOCK_FILES}") + endif() +endif() + +file(WRITE "${LOCK_FILE}") +ctest_sleep(3) +file(REMOVE "${LOCK_FILE}") + +return() -- cgit v0.12 From ff59365f8bdcb302f55d4fc882cae057a70acd4b Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Wed, 23 Oct 2013 21:47:32 +0200 Subject: CTest: fix dashboard issues associated with the ctest-fix-run-serial topic --- Source/CTest/cmCTestMultiProcessHandler.cxx | 27 ++++++++++++++++++++------- Tests/CMakeLists.txt | 3 ++- Tests/CTestTestSerialInDepends/CMakeLists.txt | 2 ++ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 3dd446b..2cae179 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -470,14 +470,18 @@ void cmCTestMultiProcessHandler::CreateTestCostList() priorityStack.push_back(TestSet()); TestSet ¤tSet = priorityStack.back(); - for(TestSet::iterator i = previousSet.begin(); + for(TestSet::const_iterator i = previousSet.begin(); i != previousSet.end(); ++i) { TestSet const& dependencies = this->Tests[*i]; - currentSet.insert(dependencies.begin(), dependencies.end()); + for(TestSet::const_iterator j = dependencies.begin(); + j != dependencies.end(); ++j) + { + currentSet.insert(*j); + } } - for(TestSet::iterator i = currentSet.begin(); + for(TestSet::const_iterator i = currentSet.begin(); i != currentSet.end(); ++i) { previousSet.erase(*i); @@ -492,15 +496,24 @@ void cmCTestMultiProcessHandler::CreateTestCostList() for(std::list::reverse_iterator i = priorityStack.rbegin(); i != priorityStack.rend(); ++i) { - TestSet ¤tSet = *i; + TestSet const& currentSet = *i; TestComparator comp(this); TestList sortedCopy; - sortedCopy.insert(sortedCopy.end(), currentSet.begin(), currentSet.end()); + + for(TestSet::const_iterator j = currentSet.begin(); + j != currentSet.end(); ++j) + { + sortedCopy.push_back(*j); + } + std::stable_sort(sortedCopy.begin(), sortedCopy.end(), comp); - this->SortedTests.insert(this->SortedTests.end(), - sortedCopy.begin(), sortedCopy.end()); + for(TestList::const_iterator j = sortedCopy.begin(); + j != sortedCopy.end(); ++j) + { + this->SortedTests.push_back(*j); + } } } diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 0d35104..d5dec90 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -2074,7 +2074,8 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --output-log "${CMake_BINARY_DIR}/Tests/CTestTestParallel/testOutput.log" ) - ADD_TEST_MACRO(CTestTestSerialInDepends ${CMAKE_CTEST_COMMAND} -j 4) + ADD_TEST_MACRO(CTestTestSerialInDepends ${CMAKE_CTEST_COMMAND} -j 4 + --output-on-failure -C "\${CTestTest_CONFIG}") if(NOT BORLAND) set(CTestLimitDashJ_EXTRA_OPTIONS --force-new-ctest-process) diff --git a/Tests/CTestTestSerialInDepends/CMakeLists.txt b/Tests/CTestTestSerialInDepends/CMakeLists.txt index f99acab..90e50f9 100644 --- a/Tests/CTestTestSerialInDepends/CMakeLists.txt +++ b/Tests/CTestTestSerialInDepends/CMakeLists.txt @@ -1,5 +1,7 @@ cmake_minimum_required(VERSION 2.8.12) +project(CTestTestSerialInDepends) + enable_testing() function(my_add_test NAME COST) -- cgit v0.12