Fix parallel testing
authorPaul Bauer <paul.bauer.q@gmail.com>
Fri, 24 Apr 2020 11:27:15 +0000 (11:27 +0000)
committerM. Eric Irrgang <mei2n@virginia.edu>
Fri, 24 Apr 2020 11:27:15 +0000 (11:27 +0000)
Change testing to be more robust in threaded setups.

Also change on merge test to use real MPI to make sure this works
correctly.

Change-Id: Id78b213291bf7cc225fc2564a23f0d85362467fd

admin/gitlab-ci/gromacs.gitlab-ci.yml
src/api/cpp/tests/testingconfiguration.h
src/gromacs/compat/tests/pointers.cpp
src/gromacs/coordinateio/tests/outputadapters.cpp
src/gromacs/coordinateio/tests/outputadapters.h
src/gromacs/gmxpreprocess/tests/readir.cpp
src/gromacs/utility/tests/enumerationhelpers.cpp
src/programs/mdrun/tests/densityfittingmodule.cpp
src/testutils/testasserts.h

index 1285c582c694c1a38a50a3989d856b4cf9bd347e..4f8e644cb91c5dc3bc3b149819908d85ce819866 100644 (file)
@@ -261,15 +261,16 @@ gromacs:clang-8:configure:
 
 gromacs:clang-3.6:configure:
   extends:
-   - .gromacs:base:configure
-   - .use-clang:base
-   - .rules:merge-requests
+    - .gromacs:base:configure
+    - .use-clang:base
+    - .use-mpi
+    - .rules:merge-requests
   image: gromacs/cmake-3.9.6-llvm-3.6-amdopencl-openmpi:2020
   variables:
     COMPILER_MAJOR_VERSION: "3.6"
     CMAKE_PRECISION_OPTIONS: "-DGMX_DOUBLE=ON"
-    CMAKE_MPI_OPTIONS: "-DGMX_OPENMP=OFF"
     CMAKE_SIMD_OPTIONS: "-DGMX_SIMD=SSE4.1"
+    CMAKE_EXTRA_OPTIONS: "-DGMX_OPENMP=OFF"
 
 gromacs:clang-static-analyzer:configure:
   extends:
@@ -329,15 +330,16 @@ gromacs:clang-3.6:release:configure:
   extends:
     - .gromacs:base:release:configure
     - .use-clang:base
+    - .use-mpi
     - .rules:nightly-only-for-release
   image: gromacs/cmake-3.9.6-llvm-3.6-amdopencl-openmpi:2020
   variables:
     COMPILER_MAJOR_VERSION: "3.6"
     RELEASE_BUILD_DIR: release-builds-clang
     CMAKE_PRECISION_OPTIONS: "-DGMX_DOUBLE=ON"
-    CMAKE_MPI_OPTIONS: "-DGMX_OPENMP=OFF"
     CMAKE_SIMD_OPTIONS: "-DGMX_SIMD=SSE4.1"
     CMAKE_BUILD_TYPE_OPTIONS: "-DCMAKE_BUILD_TYPE=RelWithAssert"
+    CMAKE_EXTRA_OPTIONS: "-DGMX_OPENMP=OFF"
   dependencies:
     - archive:package
     - regressiontests:package
@@ -543,11 +545,7 @@ gromacs:clang-8:release:build:
     - ctest -D $CTEST_RUN_MODE --output-on-failure | tee ctestLog.log || true
     - awk '/The following tests FAILED/,/^Errors while running CTest|^$/'
       ctestLog.log | tee ctestErrors.log
-    - if [ $CTEST_RUN_MODE == "ExperimentalTest" ] ; then
-      xsltproc scripts/CTest2JUnit.xsl Testing/`head -n 1 < Testing/TAG`/Test.xml > JUnitTestResults.xml;
-      else
-      touch JUnitTestResults.xml;
-      fi
+    - xsltproc scripts/CTest2JUnit.xsl Testing/`head -n 1 < Testing/TAG`/*.xml > JUnitTestResults.xml
     - if [ -s ctestErrors.log ] ; then
       echo "Error during running ctest";
       exit 1;
@@ -587,8 +585,8 @@ gromacs:clang-8:release:build:
     - cd regressiontests
     - perl gmxtest.pl $REGRESSIONTEST_PARALLEL $REGRESSIONTEST_TOTAL_RANK_NUMBER -ntomp $REGRESSIONTEST_OMP_RANK_NUMBER -npme $REGRESSIONTEST_PME_RANK_NUMBER $REGRESSIONTEST_DOUBLE $REGRESSIONTEST_MPI_RUN_COMMAND -xml all
   artifacts:
-    reports:
-      junit: regressiontests/gmxtest.xml
+    paths:
+      - regressiontests/gmxtest.xml
     when: always
     expire_in: 1 week
 
@@ -715,6 +713,7 @@ gromacs:clang-3.6:regressiontest:
   variables:
     REGRESSIONTEST_DOUBLE: "-double"
     REGRESSIONTEST_OMP_RANK_NUMBER: 0
+    REGRESSIONTEST_PARALLEL: "-np"
   needs:
     - job: gromacs:clang-3.6:build
     - job: regressiontests:prepare
@@ -789,6 +788,7 @@ gromacs:clang-3.6:release:regressiontest:
     BUILD_DIR: release-builds-clang
     REGRESSIONTEST_DOUBLE: "-double"
     REGRESSIONTEST_OMP_RANK_NUMBER: 0
+    REGRESSIONTEST_PARALLEL: "-np"
   needs:
     - job: gromacs:clang-3.6:release:build
     - job: regressiontests:package
index f3bef5f6e0dcf33358a7febf0f079305872991aa..2d4cf147f013d884bf1f21b690682fb55e955db4 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2018,2019,2020, 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.
@@ -41,6 +41,8 @@
 #include <string>
 #include <vector>
 
+#include "config.h"
+
 #include "gromacs/gmxpreprocess/grompp.h"
 #include "gromacs/math/vec.h"
 #include "gromacs/utility/stringutil.h"
@@ -56,6 +58,11 @@ namespace gmxapi
 namespace testing
 {
 
+#if GMX_OPENMP || defined(DOXYGEN)
+//! Number of OpenMP threads for child mdrun call.
+static constexpr int g_numOpenMPThreads = 2;
+#endif
+
 /*! \brief Helper function to get step size as floating point number.
  *
  * A step size of 0.002ps is suitable for the simulation.
@@ -127,6 +134,17 @@ public:
         mdArgs.emplace_back(runner_.edrFileName_);
         mdArgs.emplace_back("-cpo");
         mdArgs.emplace_back(runner_.cptFileName_);
+#if GMX_THREAD_MPI
+        /* This should be handled through the actual API we have for getting
+         * ranks, but currently this leads to data races right now */
+        mdArgs.emplace_back("-ntmpi");
+        mdArgs.emplace_back("1");
+#endif
+
+#if GMX_OPENMP
+        mdArgs.emplace_back("-ntomp");
+        mdArgs.emplace_back(std::to_string(g_numOpenMPThreads));
+#endif
 
         return mdArgs;
     }
index 8e7b3f8e4f8f261bf92b1d54009ad1bfc31ede60..991f9a31725e689a28a4897956fec54717f26c3e 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2018,2019,2020, 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.
 
 #include "gromacs/compat/pointers.h"
 
+#include "config.h"
+
 #include <memory>
 #include <vector>
 
 #include <gtest/gtest.h>
 
+#include "testutils/testasserts.h"
 
 namespace gmx
 {
@@ -65,10 +68,12 @@ TEST(NotNullConstruction, Works)
  * will not trigger the assert when using the pointer without
  * a valid object. This was needed due to an internal error
  * being triggered instead with the compiler under this condition.
+ *
+ * Death tests can also not be used safely in a parallel environment.
  */
 #    if !defined(__INTEL_COMPILER) || !(__INTEL_COMPILER == 1800 && __INTEL_COMPILER_UPDATE == 0)
     int* nullPointer = nullptr;
-    EXPECT_DEATH_IF_SUPPORTED(not_null<int*> invalidNullPointer(nullPointer), "");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(not_null<int*> invalidNullPointer(nullPointer), "");
 #    endif
 #endif
 
index ec4023bd95812bab23698a3ca1b2998f26af55f5..d71abf1d0e5bcca2736b2bf351b5a37733a7cf99 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, 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.
index 5e0437da9ec360392d4eb4c688d66e9399d2fbd9..37f24b6eb9870266c1032c3c31e8a7aef4178542 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, 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.
@@ -160,9 +160,9 @@ public:
         addOptionForSelection(&sel, false);
         setSelectionOptionValues(getOption(), &sel, false);
 
-        ASSERT_DEATH_IF_SUPPORTED(adapters.addAdapter(std::make_unique<OutputSelector>(sel),
-                                                      CoordinateFileFlags::RequireCoordinateSelection),
-                                  "Need a valid selection out of simple atom indices");
+        GMX_ASSERT_DEATH_IF_SUPPORTED(adapters.addAdapter(std::make_unique<OutputSelector>(sel),
+                                                          CoordinateFileFlags::RequireCoordinateSelection),
+                                      "Need a valid selection out of simple atom indices");
     }
 };
 
index b9d06c00b601cf1bf7914604cf4ab7a2c3c748dc..80e407451b48b9619c57df2afe6942f6606a6d1c 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2017,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2017,2018,2019,2020, 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.
@@ -47,6 +47,8 @@
 
 #include "gromacs/gmxpreprocess/readir.h"
 
+#include "config.h"
+
 #include <string>
 
 #include <gtest/gtest.h>
@@ -61,8 +63,8 @@
 #include "gromacs/utility/unique_cptr.h"
 
 #include "testutils/refdata.h"
+#include "testutils/testasserts.h"
 #include "testutils/testfilemanager.h"
-
 namespace gmx
 {
 namespace test
@@ -139,7 +141,7 @@ TEST_F(GetIrTest, HandlesDifferentKindsOfMdpLines)
 TEST_F(GetIrTest, RejectsNonCommentLineWithNoEquals)
 {
     const char* inputMdpFile = "title simulation";
-    EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile), "No '=' to separate");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile), "No '=' to separate");
 }
 
 TEST_F(GetIrTest, AcceptsKeyWithoutValue)
@@ -152,13 +154,14 @@ TEST_F(GetIrTest, AcceptsKeyWithoutValue)
 TEST_F(GetIrTest, RejectsValueWithoutKey)
 {
     const char* inputMdpFile = "= -I/home/me/stuff";
-    EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile), "No .mdp parameter name was found");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile), "No .mdp parameter name was found");
 }
 
 TEST_F(GetIrTest, RejectsEmptyKeyAndEmptyValue)
 {
     const char* inputMdpFile = " = ";
-    EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile), "No .mdp parameter name or value was found");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile),
+                                  "No .mdp parameter name or value was found");
 }
 
 TEST_F(GetIrTest, AcceptsDefineParametersWithValuesIncludingAssignment)
@@ -198,8 +201,8 @@ TEST_F(GetIrTest, AcceptsElectricFieldOscillating)
 TEST_F(GetIrTest, RejectsDuplicateOldAndNewKeys)
 {
     const char* inputMdpFile[] = { "verlet-buffer-drift = 1.3", "verlet-buffer-tolerance = 2.7" };
-    EXPECT_DEATH_IF_SUPPORTED(runTest(joinStrings(inputMdpFile, "\n")),
-                              "A parameter is present with both");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(runTest(joinStrings(inputMdpFile, "\n")),
+                                  "A parameter is present with both");
 }
 
 TEST_F(GetIrTest, AcceptsImplicitSolventNo)
@@ -211,7 +214,7 @@ TEST_F(GetIrTest, AcceptsImplicitSolventNo)
 TEST_F(GetIrTest, RejectsImplicitSolventYes)
 {
     const char* inputMdpFile = "implicit-solvent = yes";
-    EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile), "Invalid enum");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(runTest(inputMdpFile), "Invalid enum");
 }
 
 TEST_F(GetIrTest, AcceptsMimic)
index b5f84a86bf9734a4558efe0940b0256b10ded984..9d27cc96ac576972f4c90dba2c293503ae9d22d4 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, 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.
 
 #include "gromacs/utility/enumerationhelpers.h"
 
+#include "config.h"
+
 #include <iostream>
 
 #include <gtest/gtest.h>
 
 #include "gromacs/utility/arrayref.h"
 
+#include "testutils/testasserts.h"
+
 namespace gmx
 {
 namespace
@@ -135,6 +139,21 @@ TEST(EnumerationHelpersTest, EnumerationArrayWorks)
     EXPECT_EQ(fooStrings[Foo::Fooz], "Fooz");
 }
 
+TEST(EnumerationHelpersTest, EnumerationArrayCountIsSafe)
+{
+    using FooArray = EnumerationArray<Foo, std::string>;
+    const FooArray fooStrings{ { "Bar", "Baz", "Fooz" } };
+
+    // Ensures that the assertions in EnumerationArray::operator[]
+    // would fire if an out-range value (including Count) was used.
+    EXPECT_LE(fooStrings.size(), size_t(Foo::Count));
+#ifndef NDEBUG
+    // Tests (where possible) that those assertions do fire in a build
+    // with debug behavior.
+    GMX_EXPECT_DEATH_IF_SUPPORTED(fooStrings[Foo::Count], "index out of range");
+#endif
+}
+
 //! Helper function
 void func(ArrayRef<const int> a)
 {
index 5a2e068f8bafb3153e7e1f542a18d1c17c8a9557..7149eee08b3d87a291fdd7143e6900c34bcc08a2 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, 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.
@@ -42,6 +42,8 @@
  */
 #include "gmxpre.h"
 
+#include "config.h"
+
 #include <string>
 
 #include <gmock/gmock.h>
@@ -171,8 +173,8 @@ TEST_F(DensityFittingTest, GromppErrorWhenEnergyEvaluationFrequencyMismatch)
 {
     runner_.useStringAsMdpFile(mdpMdDensfitYesUnsetValues + mdpEnergyAndDensityfittingIntervalMismatch_);
 
-    EXPECT_DEATH_IF_SUPPORTED(runner_.callGrompp(),
-                              ".*is not a multiple of density-guided-simulation-nst.*");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(runner_.callGrompp(),
+                                  ".*is not a multiple of density-guided-simulation-nst.*");
 }
 
 /* Fit a subset of three of twelve argon atoms into a reference density
index 1a243802cdc8544a6e42d620bbece806c3601942..67141294df81c6e842b5e4c71d011872e8116036 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2013,2014,2015,2016,2017,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2016,2017,2018,2019,2020, 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.
@@ -41,6 +41,7 @@
  *  - exceptions
  *  - floating-point comparison
  *  - comparison against NULL
+ *  - death tests
  *
  * \if internal
  * \todo
@@ -206,6 +207,34 @@ void processExpectedException(const std::exception& ex);
  */
 #define ASSERT_NO_THROW_GMX(statement) GMX_TEST_NO_THROW_(statement, GTEST_FATAL_FAILURE_)
 
+/*! \brief
+ * Wrapper around EXPECT_DEATH_IF_SUPPORTED gtest macro for thread safe execution.
+ *
+ * Makes sure that tests that are supposed to trigger an assertion are only executed
+ * in a threadsafe environment, and not when when running under e.g. MPI.
+ *
+ * \hideinitializer
+ */
+#define GMX_EXPECT_DEATH_IF_SUPPORTED(expr, msg) \
+    if (!GMX_LIB_MPI)                            \
+    {                                            \
+        EXPECT_DEATH_IF_SUPPORTED(expr, msg);    \
+    }
+
+/*! \brief
+ * Wrapper around ASSERT_DEATH_IF_SUPPORTED gtest macro for thread safe execution.
+ *
+ * Makes sure that tests that are supposed to trigger an assertion are only executed
+ * in a threadsafe environment when running under e.g. MPI.
+ *
+ * \hideinitializer
+ */
+#define GMX_ASSERT_DEATH_IF_SUPPORTED(expr, msg) \
+    if (!GMX_LIB_MPI)                            \
+    {                                            \
+        ASSERT_DEATH_IF_SUPPORTED(expr, msg);    \
+    }
+
 //! \}
 
 /*! \libinternal \brief