Fix parallel testing
authorPaul Bauer <paul.bauer.q@gmail.com>
Thu, 23 Apr 2020 12:07:38 +0000 (12:07 +0000)
committerPaul Bauer <paul.bauer.q@gmail.com>
Thu, 23 Apr 2020 12:07:38 +0000 (12:07 +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 afffc9cfb4cdfd43d2c43802f4a550ff7665cd24..418a646cedc2b52434cd587b8579460b6d12624c 100644 (file)
@@ -263,6 +263,7 @@ gromacs:clang-9:configure:
   extends:
    - .gromacs:base:configure
    - .use-clang:base
+   - .use-mpi
    - .rules:merge-requests
   image: gromacs/cmake-3.15.7-llvm-9-openmpi:master
   variables:
@@ -708,6 +709,7 @@ gromacs:clang-9:regressiontest:
   image: gromacs/cmake-3.15.7-llvm-9-openmpi:master
   variables:
     REGRESSIONTEST_DOUBLE: "-double"
+    REGRESSIONTEST_PARALLEL: "-np"
   needs:
     - job: gromacs:clang-9:build
     - job: regressiontests:prepare
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 3e6c504355e86181638dddaf9f7985f07f01762c..9d27cc96ac576972f4c90dba2c293503ae9d22d4 100644 (file)
 
 #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
@@ -146,7 +150,7 @@ TEST(EnumerationHelpersTest, EnumerationArrayCountIsSafe)
 #ifndef NDEBUG
     // Tests (where possible) that those assertions do fire in a build
     // with debug behavior.
-    EXPECT_DEATH_IF_SUPPORTED(fooStrings[Foo::Count], "index out of range");
+    GMX_EXPECT_DEATH_IF_SUPPORTED(fooStrings[Foo::Count], "index out of range");
 #endif
 }
 
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 f1baab8be797c28ccd514d6807bcf0e532466bd0..3e377280f87cba4a313d77852cf983c97f4a17ee 100644 (file)
@@ -42,6 +42,7 @@
  *  - exceptions
  *  - floating-point comparison
  *  - comparison against NULL
+ *  - death tests
  *
  * \if internal
  * \todo
@@ -207,6 +208,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