Clean up CTest test management
authorTeemu Murtola <teemu.murtola@gmail.com>
Mon, 30 May 2016 19:56:22 +0000 (22:56 +0300)
committerTeemu Murtola <teemu.murtola@gmail.com>
Wed, 7 Sep 2016 10:57:52 +0000 (12:57 +0200)
- Add explicit timeouts to CTest tests. These should work better than
  the current Jenkins timeout: it should produce better JUnit test
  reports, and should catch also tests that end up in an infinite loop
  printing stuff.  The timeouts are still quite generous, since at least
  the coverage run can take a while.
- Use a more easily extensible and better encapsulated mechanism for
  producing the messages about missing tests with 'make check'.
- Abort if running a non-MPI test binary with MPI instead of trying to
  run it on the master rank, since it might deadlock.
- Clean up the macros used to register CTest tests and use a clearer set
  of test labels, and move more logic into src/testutils/CMakeLists.txt.
- Resolve various TODOs.

Change-Id: Ia1370d99eabd5839682832e8c3c60668a667cdde

CMakeLists.txt
src/CMakeLists.txt
src/gromacs/gmxana/legacytests/CMakeLists.txt
src/programs/mdrun/tests/CMakeLists.txt
src/testutils/CMakeLists.txt
src/testutils/TestMacros.cmake
src/testutils/testinit.cpp
tests/CMakeLists.txt
tests/CheckTarget.cmake [new file with mode: 0644]

index d888a3b20e76d0705dc8f5120b9ec8e02bfabb14..a3dbf9f4154a43f1a231dee71a10978b1d2a6bb6 100644 (file)
@@ -853,18 +853,7 @@ if (GMX_BUILD_FOR_COVERAGE)
 endif()
 
 if (BUILD_TESTING)
-    # "tests" target builds all the separate test binaries.
-    add_custom_target(tests)
-    # "run-ctest" is an internal target that actually runs the tests.
-    # This is necessary to be able to add separate targets that execute as part
-    # of 'make check', but are ensured to be executed after the actual tests.
-    add_custom_target(run-ctest
-                      COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure
-                      COMMENT "Running all tests"
-                      VERBATIM)
-    add_dependencies(run-ctest tests)
-    # "check" target builds and runs all tests.
-    add_custom_target(check DEPENDS run-ctest)
+    include(tests/CheckTarget.cmake)
 endif()
 
 if (NOT GMX_BUILD_MDRUN_ONLY)
index 11829495889c933dc8b9fa05e0f3af8ff8791f41..f120b01add4bf0da3db4321cc72aeafd86a88513 100644 (file)
@@ -1,7 +1,7 @@
 #
 # This file is part of the GROMACS molecular simulation package.
 #
-# Copyright (c) 2009,2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by
+# Copyright (c) 2009,2010,2011,2012,2013,2014,2015,2016, by the GROMACS development team, led by
 # Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
 # and including many others, as listed in the AUTHORS file in the
 # top-level source directory and at http://www.gromacs.org.
@@ -55,15 +55,7 @@ if (BUILD_TESTING)
         add_subdirectory(external/gmock-1.7.0)
     endif()
     include(testutils/TestMacros.cmake)
-    if (GMX_BUILD_UNITTESTS)
-        add_subdirectory(testutils)
-    else()
-        add_custom_target(unittests-notice
-            ${CMAKE_COMMAND} -E echo "NOTE: Unit tests have not been run. You need to set GMX_BUILD_UNITTESTS=ON if you want to build and run them."
-            DEPENDS run-ctest
-            COMMENT "Unit tests disabled" VERBATIM)
-        add_dependencies(check unittests-notice)
-    endif()
+    add_subdirectory(testutils)
 endif()
 
 add_subdirectory(gromacs)
index af7a8080babb424ed4761fab9c4ca95f2cee4d02..558aa174a28af6bd114942f151dfe315feb45693 100644 (file)
@@ -32,7 +32,6 @@
 # To help us fund GROMACS development, we humbly ask that you cite
 # the research papers on the package. Check out http://www.gromacs.org.
 
-set(testname "LegacyToolsTests")
 set(exename "legacy-tools-test")
 
 gmx_add_gtest_executable(
@@ -40,7 +39,4 @@ gmx_add_gtest_executable(
     # files with code for test fixtures
     gmx_traj_tests.cpp
     )
-gmx_register_integration_test(
-    ${testname}
-    ${exename}
-    )
+gmx_register_gtest_test(LegacyToolsTest ${exename} INTEGRATION_TEST)
index 6c5270d3e919639328fb87b9b83bd90aac04afa4..d4763d2f7b25a3422fe4192b104d1655cca79f27 100644 (file)
@@ -60,10 +60,7 @@ gmx_add_gtest_executable(
     # pseudo-library for code for mdrun
     $<TARGET_OBJECTS:mdrun_objlib>
     )
-gmx_register_integration_test(
-    ${testname}
-    ${exename}
-    )
+gmx_register_gtest_test(${testname} ${exename} INTEGRATION_TEST)
 
 set(testname "MdrunMpiTests")
 set(exename "mdrun-mpi-test")
@@ -80,8 +77,4 @@ gmx_add_gtest_executable(
     # pseudo-library for code for mdrun
     $<TARGET_OBJECTS:mdrun_objlib>
     )
-gmx_register_mpi_integration_test(
-    ${testname}
-    ${exename}
-    2
-    )
+gmx_register_gtest_test(${testname} ${exename} MPI_RANKS 2 INTEGRATION_TEST)
index 3c51e4bc0905d400dad90c05c0e98c52e1669f73..a78e325931fdc2f55d6fa2c7692266c22fbfa202 100644 (file)
 # To help us fund GROMACS development, we humbly ask that you cite
 # the research papers on the package. Check out http://www.gromacs.org.
 
+if (NOT GMX_BUILD_UNITTESTS)
+    gmx_add_missing_tests_notice("Unit tests have not been run. You need to set GMX_BUILD_UNITTESTS=ON if you want to build and run them.")
+    return()
+endif()
+
 include_directories(BEFORE SYSTEM ${GMOCK_INCLUDE_DIRS})
 set(TESTUTILS_SOURCES
     cmdlinetest.cpp
@@ -68,8 +73,35 @@ else()
     include_directories(BEFORE SYSTEM "../external/tinyxml2")
 endif()
 
+# TODO Use gmx_add_missing_tests_notice() instead of the messages below.
+set(GMX_CAN_RUN_MPI_TESTS 1)
+if (GMX_MPI)
+    set(_an_mpi_variable_had_content 0)
+    foreach(VARNAME MPIEXEC MPIEXEC_NUMPROC_FLAG MPIEXEC_PREFLAGS MPIEXEC_POSTFLAGS)
+        # These variables need a valid value for the test to run
+        # and pass, but conceivably any of them might be valid
+        # with arbitrary (including empty) content. They can't be
+        # valid if they've been populated with the CMake
+        # find_package magic suffix/value "NOTFOUND", though.
+        if (${VARNAME} MATCHES ".*NOTFOUND")
+            message(STATUS "CMake variable ${VARNAME} was not detected to be a valid value. To test GROMACS correctly, check the advice in the install guide.")
+            set(GMX_CAN_RUN_MPI_TESTS 0)
+        endif()
+        if (NOT VARNAME STREQUAL MPIEXEC AND ${VARNAME})
+            set(_an_mpi_variable_had_content 1)
+        endif()
+    endforeach()
+    if(_an_mpi_variable_had_content AND NOT MPIEXEC)
+        message(STATUS "CMake variable MPIEXEC must have a valid value if one of the other related MPIEXEC variables does. To test GROMACS correctly, check the advice in the install guide.")
+        set(GMX_CAN_RUN_MPI_TESTS 0)
+    endif()
+elseif (NOT GMX_THREAD_MPI)
+    set(GMX_CAN_RUN_MPI_TESTS 0)
+endif()
+
 set(TESTUTILS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(TESTUTILS_DIR ${TESTUTILS_DIR} PARENT_SCOPE)
 set(TESTUTILS_LIBS ${TESTUTILS_LIBS} PARENT_SCOPE)
+set(GMX_CAN_RUN_MPI_TESTS ${GMX_CAN_RUN_MPI_TESTS} PARENT_SCOPE)
 
 add_subdirectory(tests)
index c0c0f13809bf08a06b62a9ab5bfd44184c7529f1..c351d13cb09c7721e01b193832a78d5e90e72722 100644 (file)
@@ -81,95 +81,61 @@ function (gmx_add_gtest_executable EXENAME)
     endif()
 endfunction()
 
-function (gmx_register_unit_test NAME EXENAME)
-    if (GMX_BUILD_UNITTESTS AND BUILD_TESTING)
-        add_test(NAME ${NAME}
-                 COMMAND ${EXENAME} --gtest_output=xml:${CMAKE_BINARY_DIR}/Testing/Temporary/${NAME}.xml)
-        set_tests_properties(${NAME} PROPERTIES LABELS "GTest;UnitTest")
-        add_dependencies(tests ${EXENAME})
-    endif()
-endfunction ()
-
-# Use this function to register a test binary as an integration test
-function (gmx_register_integration_test NAME EXENAME)
-    if (GMX_BUILD_UNITTESTS AND BUILD_TESTING)
-        add_test(NAME ${NAME}
-                 COMMAND ${EXENAME} --gtest_output=xml:${CMAKE_BINARY_DIR}/Testing/Temporary/${NAME}.xml)
-        set_tests_properties(${testname} PROPERTIES LABELS "IntegrationTest")
-        add_dependencies(tests ${EXENAME})
-
-        # GMX_EXTRA_LIBRARIES might be needed for mdrun integration tests at
-        # some point.
-        # target_link_libraries(${EXENAME} ${GMX_EXTRA_LIBRARIES} ${GMX_STDLIB_LIBRARIES})
-    endif()
-endfunction ()
-
-# Use this function to register a test binary as an integration test
-# that requires MPI. The intended number of MPI ranks is also passed
+# Use this function with MPI_RANKS <N> INTEGRATION_TEST to register a test
+# binary as an integration test that requires MPI. The intended number of MPI
+# ranks is also passed
 #
-# TODO When a test case needs it, generalize the NUMPROC mechanism so
+# TODO When a test case needs it, generalize the MPI_RANKS mechanism so
 # that ctest can run the test binary over a range of numbers of MPI
 # ranks.
-function (gmx_register_mpi_integration_test NAME EXENAME NUMPROC)
+function (gmx_register_gtest_test NAME EXENAME)
     if (GMX_BUILD_UNITTESTS AND BUILD_TESTING)
-        if (GMX_MPI)
-            foreach(VARNAME MPIEXEC MPIEXEC_NUMPROC_FLAG MPIEXEC_PREFLAGS MPIEXEC_POSTFLAGS)
-                # These variables need a valid value for the test to run
-                # and pass, but conceivably any of them might be valid
-                # with arbitrary (including empty) content. They can't be
-                # valid if they've been populated with the CMake
-                # find_package magic suffix/value "NOTFOUND", though.
-                if (${VARNAME} MATCHES ".*NOTFOUND")
-                    message(STATUS "CMake variable ${VARNAME} was not detected to be a valid value. To test GROMACS correctly, check the advice in the install guide.")
-                    set(_cannot_run_mpi_tests 1)
-                endif()
-                if (NOT VARNAME STREQUAL MPIEXEC AND ${VARNAME})
-                    set(_an_mpi_variable_had_content 1)
-                endif()
-            endforeach()
-            if(_an_mpi_variable_had_content AND NOT MPIEXEC)
-                message(STATUS "CMake variable MPIEXEC must have a valid value if one of the other related MPIEXEC variables does. To test GROMACS correctly, check the advice in the install guide.")
-                set(_cannot_run_mpi_tests 1)
-            endif()
-            if(NOT _cannot_run_mpi_tests)
-                add_test(NAME ${NAME}
-                    COMMAND
-                    ${MPIEXEC} ${MPIEXEC_NUMPROC_FLAG} ${NUMPROC}
-                    ${MPIEXEC_PREFLAGS} $<TARGET_FILE:${EXENAME}> ${MPIEXEC_POSTFLAGS}
-                    --gtest_output=xml:${CMAKE_BINARY_DIR}/Testing/Temporary/${NAME}.xml
-                    )
-                set_tests_properties(${testname} PROPERTIES LABELS "MpiIntegrationTest")
-                add_dependencies(tests ${EXENAME})
-            endif()
-
-            # GMX_EXTRA_LIBRARIES might be needed for mdrun integration tests at
-            # some point.
-            # target_link_libraries(${EXENAME} ${GMX_EXTRA_LIBRARIES} ${GMX_STDLIB_LIBRARIES})
-        elseif(GMX_THREAD_MPI)
-            add_test(NAME ${NAME}
-                COMMAND
-                $<TARGET_FILE:${EXENAME}> -ntmpi ${NUMPROC}
-                --gtest_output=xml:${CMAKE_BINARY_DIR}/Testing/Temporary/${NAME}.xml
-                )
-            set_tests_properties(${testname} PROPERTIES LABELS "MpiIntegrationTest")
-            add_dependencies(tests ${EXENAME})
+        set(_options INTEGRATION_TEST)
+        set(_one_value_args MPI_RANKS)
+        cmake_parse_arguments(ARG "${_options}" "${_one_value_args}" "" ${ARGN})
+        set(_xml_path ${CMAKE_BINARY_DIR}/Testing/Temporary/${NAME}.xml)
+        set(_cmd ${EXENAME})
+        set(_labels GTest)
+        set(_timeout 30)
+        if (ARG_INTEGRATION_TEST)
+            list(APPEND _labels IntegrationTest)
+            set(_timeout 120)
 
             # GMX_EXTRA_LIBRARIES might be needed for mdrun integration tests at
             # some point.
             # target_link_libraries(${EXENAME} ${GMX_EXTRA_LIBRARIES} ${GMX_STDLIB_LIBRARIES})
+        else()
+            list(APPEND _labels UnitTest)
+        endif()
+        if (ARG_MPI_RANKS)
+            if (NOT GMX_CAN_RUN_MPI_TESTS)
+                return()
+            endif()
+            list(APPEND _labels MpiTest)
+            if (GMX_MPI)
+                set(_cmd
+                    ${MPIEXEC} ${MPIEXEC_NUMPROC_FLAG} ${ARG_MPI_RANKS}
+                    ${MPIEXEC_PREFLAGS} $<TARGET_FILE:${EXENAME}> ${MPIEXEC_POSTFLAGS})
+            elseif (GMX_THREAD_MPI)
+                list(APPEND _cmd -ntmpi ${ARG_MPI_RANKS})
+            endif()
         endif()
+        add_test(NAME ${NAME}
+                 COMMAND ${_cmd} --gtest_output=xml:${_xml_path})
+        set_tests_properties(${NAME} PROPERTIES LABELS "${_labels}")
+        set_tests_properties(${NAME} PROPERTIES TIMEOUT ${_timeout})
+        add_dependencies(tests ${EXENAME})
     endif()
 endfunction ()
 
 function (gmx_add_unit_test NAME EXENAME)
     gmx_add_gtest_executable(${EXENAME} ${ARGN})
-    gmx_register_unit_test(${NAME} ${EXENAME})
+    gmx_register_gtest_test(${NAME} ${EXENAME})
 endfunction()
 
 function (gmx_add_mpi_unit_test NAME EXENAME RANKS)
     if (GMX_MPI OR (GMX_THREAD_MPI AND GTEST_IS_THREADSAFE))
         gmx_add_gtest_executable(${EXENAME} MPI ${ARGN})
-        # TODO: This function needs a new name.
-        gmx_register_mpi_integration_test(${NAME} ${EXENAME} ${RANKS})
+        gmx_register_gtest_test(${NAME} ${EXENAME} MPI_RANKS ${RANKS})
     endif()
 endfunction()
index 4c2411b7e83681f84995b6a60b1ad40fbdf57b63..5ade1e5a1d373a388a9cd52023682611c1d77c22 100644 (file)
@@ -168,18 +168,18 @@ void initTestUtils(const char *dataPath, const char *tempPath, bool usesMpi,
     {
         if (!usesMpi && gmx_node_num() > 1)
         {
+            // We cannot continue, since some tests might be using
+            // MPI_COMM_WORLD, which could deadlock if we would only
+            // continue with the master rank here.
             if (gmx_node_rank() == 0)
             {
                 fprintf(stderr, "NOTE: You are running %s on %d MPI ranks, "
                         "but it is does not contain MPI-enabled tests. "
-                        "Rank 0 will run the tests, other ranks will exit.",
+                        "The test will now exit.\n",
                         context.programName(), gmx_node_num());
             }
-            else
-            {
-                finalizeForCommandLine();
-                std::exit(0);
-            }
+            finalizeForCommandLine();
+            std::exit(1);
         }
         g_testContext.reset(new TestProgramContext(context));
         setProgramContext(g_testContext.get());
index 97277f84f7e64e6a852cb7dc002a824c89a74700..5a946bf27df69c596a0ff6f8ca2dda3bf4e200ce 100644 (file)
@@ -174,11 +174,8 @@ if(REGRESSIONTEST_PATH)
             ENVIRONMENT "PATH=${PATH}")
     endforeach()
 else()
-    add_custom_target(regressiontests-notice
-        ${CMAKE_COMMAND} -E echo "NOTE: Regression tests have not been run. If you want to run them from the build system, get the correct version of the regression tests package and set REGRESSIONTEST_PATH in CMake to point to it, or set REGRESSIONTEST_DOWNLOAD=ON."
-        DEPENDS run-ctest
-        COMMENT "Regression tests not available" VERBATIM)
-    add_dependencies(check regressiontests-notice)
+    gmx_add_missing_tests_notice("Regression tests have not been run. If you want to run them from the build system, get the correct version of the regression tests package and set REGRESSIONTEST_PATH in CMake to point to it, or set REGRESSIONTEST_DOWNLOAD=ON.")
 endif()
 
+gmx_create_missing_tests_notice_target()
 include(CppCheck.cmake)
diff --git a/tests/CheckTarget.cmake b/tests/CheckTarget.cmake
new file mode 100644 (file)
index 0000000..3cb3837
--- /dev/null
@@ -0,0 +1,67 @@
+#
+# This file is part of the GROMACS molecular simulation package.
+#
+# Copyright (c) 2014,2016, by the GROMACS development team, led by
+# Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
+# and including many others, as listed in the AUTHORS file in the
+# top-level source directory and at http://www.gromacs.org.
+#
+# GROMACS is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public License
+# as published by the Free Software Foundation; either version 2.1
+# of the License, or (at your option) any later version.
+#
+# GROMACS is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with GROMACS; if not, see
+# http://www.gnu.org/licenses, or write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+#
+# If you want to redistribute modifications to GROMACS, please
+# consider that scientific software is very special. Version
+# control is crucial - bugs must be traceable. We will be happy to
+# consider code for inclusion in the official distribution, but
+# derived work must not be called official GROMACS. Details are found
+# in the README & COPYING files - if they are missing, get the
+# official version at http://www.gromacs.org.
+#
+# To help us fund GROMACS development, we humbly ask that you cite
+# the research papers on the package. Check out http://www.gromacs.org.
+
+# "tests" target builds all the separate test binaries.
+add_custom_target(tests)
+# "run-ctest" is an internal target that actually runs the tests.
+# This is necessary to be able to add separate targets that execute as part
+# of 'make check', but are ensured to be executed after the actual tests.
+add_custom_target(run-ctest
+                  COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure
+                  COMMENT "Running all tests"
+                  VERBATIM)
+add_dependencies(run-ctest tests)
+# "check" target builds and runs all tests.
+add_custom_target(check DEPENDS run-ctest)
+
+# Global property for collecting notices to show at the end of the "check"
+# target.
+set_property(GLOBAL PROPERTY GMX_TESTS_NOTICE)
+
+function (gmx_add_missing_tests_notice TEXT)
+    set_property(GLOBAL APPEND PROPERTY GMX_TESTS_NOTICE ${TEXT})
+endfunction()
+
+function (gmx_create_missing_tests_notice_target)
+    get_property(_text GLOBAL PROPERTY GMX_TESTS_NOTICE)
+    set(_cmds)
+    foreach (_line ${_text})
+        list(APPEND _cmds COMMAND ${CMAKE_COMMAND} -E echo "NOTE: ${_line}")
+    endforeach()
+    add_custom_target(missing-tests-notice
+        ${_cmds}
+        DEPENDS run-ctest
+        COMMENT "Some tests not available" VERBATIM)
+    add_dependencies(check missing-tests-notice)
+endfunction()