From 7f0d4aff24cbb7ea9336a484f6ad8f73a63ac77f Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Fri, 20 Dec 2013 22:44:37 +0100 Subject: [PATCH] CTest: fix regressions introduced by the ctest-fix-run-serial topic The first regression resulted in endless looping due to unrun test dependencies. The second regression prioritized all tests with dependencies in serial test runs. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 87 ++++++++++++++++++- Source/CTest/cmCTestMultiProcessHandler.h | 6 ++ Tests/CMakeLists.txt | 9 ++ .../CTestTestMissingDependsExe/CMakeLists.txt | 10 +++ Tests/CTestTestSerialOrder/CMakeLists.txt | 40 +++++++++ Tests/CTestTestSerialOrder/test.cmake | 31 +++++++ 6 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 Tests/CTestTestMissingDependsExe/CMakeLists.txt create mode 100644 Tests/CTestTestSerialOrder/CMakeLists.txt create mode 100644 Tests/CTestTestSerialOrder/test.cmake diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 4c39d105b..729f14a1b 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -139,6 +139,13 @@ void cmCTestMultiProcessHandler::StartTestProcess(int test) } else { + + for(TestMap::iterator j = this->Tests.begin(); + j != this->Tests.end(); ++j) + { + j->second.erase(test); + } + this->UnlockResources(test); this->Completed++; this->TestFinishMap[test] = true; @@ -440,6 +447,19 @@ int cmCTestMultiProcessHandler::SearchByName(std::string name) //--------------------------------------------------------- void cmCTestMultiProcessHandler::CreateTestCostList() +{ + if(this->ParallelLevel > 1) + { + CreateParallelTestCostList(); + } + else + { + CreateSerialTestCostList(); + } +} + +//--------------------------------------------------------- +void cmCTestMultiProcessHandler::CreateParallelTestCostList() { TestSet alreadySortedTests; @@ -452,8 +472,7 @@ void cmCTestMultiProcessHandler::CreateTestCostList() for(TestMap::const_iterator i = this->Tests.begin(); i != this->Tests.end(); ++i) { - if(this->ParallelLevel > 1 && - std::find(this->LastTestsFailed.begin(), this->LastTestsFailed.end(), + 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. @@ -466,8 +485,9 @@ void cmCTestMultiProcessHandler::CreateTestCostList() } } - // Repeatedly move dependencies of the tests on the current dependency level - // to the next level until no further dependencies exist. + // In parallel test runs 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(); @@ -525,6 +545,65 @@ void cmCTestMultiProcessHandler::CreateTestCostList() } } +//--------------------------------------------------------- +void cmCTestMultiProcessHandler::GetAllTestDependencies( + int test, TestList& dependencies) +{ + TestSet const& dependencySet = this->Tests[test]; + for(TestSet::const_iterator i = dependencySet.begin(); + i != dependencySet.end(); ++i) + { + GetAllTestDependencies(*i, dependencies); + dependencies.push_back(*i); + } +} + +//--------------------------------------------------------- +void cmCTestMultiProcessHandler::CreateSerialTestCostList() +{ + TestList presortedList; + + for(TestMap::iterator i = this->Tests.begin(); + i != this->Tests.end(); ++i) + { + presortedList.push_back(i->first); + } + + TestComparator comp(this); + std::stable_sort(presortedList.begin(), presortedList.end(), comp); + + TestSet alreadySortedTests; + + for(TestList::const_iterator i = presortedList.begin(); + i != presortedList.end(); ++i) + { + int test = *i; + + if(alreadySortedTests.find(test) != alreadySortedTests.end()) + { + continue; + } + + TestList dependencies; + GetAllTestDependencies(test, dependencies); + + for(TestList::const_iterator j = dependencies.begin(); + j != dependencies.end(); ++j) + { + int testDependency = *j; + + if(alreadySortedTests.find(testDependency) == alreadySortedTests.end()) + { + alreadySortedTests.insert(testDependency); + this->SortedTests.push_back(testDependency); + } + } + + alreadySortedTests.insert(test); + this->SortedTests.push_back(test); + } +} + //--------------------------------------------------------- void cmCTestMultiProcessHandler::WriteCheckpoint(int index) { diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 439a8f368..1b53ec7c2 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -72,6 +72,12 @@ protected: int SearchByName(std::string name); void CreateTestCostList(); + + void GetAllTestDependencies(int test, TestList& dependencies); + void CreateSerialTestCostList(); + + void CreateParallelTestCostList(); + // Removes the checkpoint file void MarkFinished(); void EraseTest(int index); diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 5ea604ffe..063bd2da2 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -2219,6 +2219,15 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ ADD_TEST_MACRO(CTestTestSerialInDepends ${CMAKE_CTEST_COMMAND} -j 4 --output-on-failure -C "\${CTestTest_CONFIG}") + ADD_TEST_MACRO(CTestTestMissingDependsExe ${CMAKE_CTEST_COMMAND} + --output-on-failure -C "\${CTestTest_CONFIG}") + set_tests_properties(CTestTestMissingDependsExe PROPERTIES + PASS_REGULAR_EXPRESSION "\\*\\*\\*Not Run" + ) + + ADD_TEST_MACRO(CTestTestSerialOrder ${CMAKE_CTEST_COMMAND} + --output-on-failure -C "\${CTestTest_CONFIG}") + if(NOT BORLAND) set(CTestLimitDashJ_CTEST_OPTIONS --force-new-ctest-process) add_test_macro(CTestLimitDashJ ${CMAKE_CTEST_COMMAND} -j 4 diff --git a/Tests/CTestTestMissingDependsExe/CMakeLists.txt b/Tests/CTestTestMissingDependsExe/CMakeLists.txt new file mode 100644 index 000000000..9826da661 --- /dev/null +++ b/Tests/CTestTestMissingDependsExe/CMakeLists.txt @@ -0,0 +1,10 @@ +cmake_minimum_required(VERSION 2.8.12) + +project(CTestTestMissingDependsExe) + +enable_testing() + +add_test(test1 ${CMAKE_COMMAND} -E echo test) +add_test(test2 non-existent-command) + +set_tests_properties(test1 PROPERTIES DEPENDS test2) diff --git a/Tests/CTestTestSerialOrder/CMakeLists.txt b/Tests/CTestTestSerialOrder/CMakeLists.txt new file mode 100644 index 000000000..69c11fcde --- /dev/null +++ b/Tests/CTestTestSerialOrder/CMakeLists.txt @@ -0,0 +1,40 @@ +cmake_minimum_required(VERSION 2.8.12) + +project(CTestTestSerialOrder) + +set(TEST_OUTPUT_FILE "${CMAKE_CURRENT_BINARY_DIR}/test_output.txt") + +enable_testing() + +function(add_serial_order_test TEST_NAME) + add_test(NAME ${TEST_NAME} + COMMAND ${CMAKE_COMMAND} + "-DTEST_OUTPUT_FILE=${TEST_OUTPUT_FILE}" + "-DTEST_NAME=${TEST_NAME}" + -P "${CMAKE_CURRENT_SOURCE_DIR}/test.cmake" + ) + + if(ARGC GREATER 1) + set_tests_properties(${TEST_NAME} PROPERTIES ${ARGN}) + endif() +endfunction() + +add_serial_order_test(initialization COST 1000) +add_serial_order_test(test1) +add_serial_order_test(test2) +add_serial_order_test(test3) +add_serial_order_test(test4 DEPENDS test5) + +add_serial_order_test(test5) +set_tests_properties(test5 PROPERTIES DEPENDS "test6;test7b;test7a") + +add_serial_order_test(test6 COST -2) +add_serial_order_test(test7a COST -1) +add_serial_order_test(test7b COST -1) +add_serial_order_test(test8 COST 10) +add_serial_order_test(test9 COST 20) +add_serial_order_test(test10 COST 0) +add_serial_order_test(test11) +add_serial_order_test(test12 COST 0) + +add_serial_order_test(verification COST -1000) diff --git a/Tests/CTestTestSerialOrder/test.cmake b/Tests/CTestTestSerialOrder/test.cmake new file mode 100644 index 000000000..8479cae95 --- /dev/null +++ b/Tests/CTestTestSerialOrder/test.cmake @@ -0,0 +1,31 @@ +list(APPEND EXPECTED_OUTPUT + initialization + test9 + test8 + test1 + test2 + test3 + test6 + test7a + test7b + test5 + test4 + test10 + test11 + test12 +) + + +if("${TEST_NAME}" STREQUAL "initialization") + file(WRITE ${TEST_OUTPUT_FILE} "${TEST_NAME}") + +elseif("${TEST_NAME}" STREQUAL "verification") + file(READ ${TEST_OUTPUT_FILE} ACTUAL_OUTPUT) + if(NOT "${ACTUAL_OUTPUT}" STREQUAL "${EXPECTED_OUTPUT}") + message(FATAL_ERROR "Actual test order [${ACTUAL_OUTPUT}] differs from expected test order [${EXPECTED_OUTPUT}]") + endif() + +else() + file(APPEND ${TEST_OUTPUT_FILE} ";${TEST_NAME}") + +endif()