From: Mark Abraham Date: Mon, 10 Mar 2014 12:29:26 +0000 (+0100) Subject: Refactor mdrun integration tests X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?p=alexxy%2Fgromacs.git;a=commitdiff_plain;h=293630c584fc91d87438538030a612fce4ed31f3 Refactor mdrun integration tests Future work will need the ability for such a test to call mdrun more than once and compare the results. This means that a test fixture object needs to be able to keep track of the separate files in an orderly way. Created SimulationRunner to manage the setup and calls to grompp and mdrun, somewhat like an implementation class. MdrunTestFixture now owns a single such object. Created MdrunTestFixtureBase to be the common point for managing issues of controlling the execution environment of mdrun, so that future work can derive classes from it that use more than one SimulationRunner. Change-Id: Ic2ab82b3bacb01f9bf77de67909567352fc2d306 --- diff --git a/src/programs/mdrun/tests/compressed_x_output.cpp b/src/programs/mdrun/tests/compressed_x_output.cpp index 26a65475bf..4cc1b106f2 100644 --- a/src/programs/mdrun/tests/compressed_x_output.cpp +++ b/src/programs/mdrun/tests/compressed_x_output.cpp @@ -69,16 +69,16 @@ TEST_P(CompressedXOutputTest, ExitsNormally) "nsteps = 1\n" "nstxout-compressed = 1\n"); mdpFile += GetParam(); - useStringAsMdpFile(mdpFile.c_str()); - useTopGroAndNdxFromDatabase("spc2"); - ASSERT_EQ(0, callGrompp()); + runner_.useStringAsMdpFile(mdpFile.c_str()); + runner_.useTopGroAndNdxFromDatabase("spc2"); + ASSERT_EQ(0, runner_.callGrompp()); - reducedPrecisionTrajectoryFileName = fileManager_.getTemporaryFilePath(".xtc"); - ASSERT_EQ(0, callMdrun()); + runner_.reducedPrecisionTrajectoryFileName_ = fileManager_.getTemporaryFilePath(".xtc"); + ASSERT_EQ(0, runner_.callMdrun()); ::gmx::test::CommandLine checkCaller; checkCaller.append("check"); - checkCaller.addOption("-f", reducedPrecisionTrajectoryFileName); + checkCaller.addOption("-f", runner_.reducedPrecisionTrajectoryFileName_); ASSERT_EQ(0, gmx_check(checkCaller.argc(), checkCaller.argv())); } diff --git a/src/programs/mdrun/tests/interactiveMD.cpp b/src/programs/mdrun/tests/interactiveMD.cpp index ecd31972b9..a4b7b3dfe6 100644 --- a/src/programs/mdrun/tests/interactiveMD.cpp +++ b/src/programs/mdrun/tests/interactiveMD.cpp @@ -77,10 +77,10 @@ typedef gmx::test::ImdTestFixture ImdTest; TEST_F(ImdTest, ImdCanRun) { std::string name = "spc2"; - useTopGroAndNdxFromDatabase(name.c_str()); - mdpInputFileName = fileManager_.getInputFilePath((name + "-IMD.mdp").c_str()); + runner_.useTopGroAndNdxFromDatabase(name.c_str()); + runner_.mdpInputFileName_ = fileManager_.getInputFilePath((name + "-IMD.mdp").c_str()); - EXPECT_EQ(0, callGrompp()); + EXPECT_EQ(0, runner_.callGrompp()); ::gmx::test::CommandLine imdCaller; imdCaller.append("mdrun"); @@ -91,7 +91,7 @@ TEST_F(ImdTest, ImdCanRun) imdCaller.append("-noimdterm"); // Do an mdrun with IMD enabled - ASSERT_EQ(0, callMdrun(imdCaller)); + ASSERT_EQ(0, runner_.callMdrun(imdCaller)); } diff --git a/src/programs/mdrun/tests/moduletest.cpp b/src/programs/mdrun/tests/moduletest.cpp index b9cb26f354..1e8a6373c5 100644 --- a/src/programs/mdrun/tests/moduletest.cpp +++ b/src/programs/mdrun/tests/moduletest.cpp @@ -83,92 +83,89 @@ GMX_TEST_OPTIONS(MdrunTestOptions, options) GMX_UNUSED_VALUE(options); #ifdef GMX_THREAD_MPI options->addOption(IntegerOption("nt").store(&g_numThreads) - .description("Number of thread-MPI threads/ranks for child mdrun call")); + .description("Number of thread-MPI threads/ranks for child mdrun calls")); #endif #ifdef GMX_OPENMP options->addOption(IntegerOption("nt_omp").store(&g_numOpenMPThreads) - .description("Number of OpenMP threads for child mdrun call")); + .description("Number of OpenMP threads for child mdrun calls")); #endif } //! \endcond } -MdrunTestFixture::MdrunTestFixture() : - topFileName(), - groFileName(), - fullPrecisionTrajectoryFileName(), - ndxFileName(), - mdpInputFileName(fileManager_.getTemporaryFilePath("input.mdp")), - mdpOutputFileName(fileManager_.getTemporaryFilePath("output.mdp")), - tprFileName(fileManager_.getTemporaryFilePath(".tpr")), - logFileName(fileManager_.getTemporaryFilePath(".log")), - edrFileName(fileManager_.getTemporaryFilePath(".edr")), - nsteps(-2) +SimulationRunner::SimulationRunner(IntegrationTestFixture *fixture) : + fixture_(fixture), + topFileName_(), + groFileName_(), + fullPrecisionTrajectoryFileName_(), + ndxFileName_(), + mdpInputFileName_(fixture_->fileManager_.getTemporaryFilePath("input.mdp")), + mdpOutputFileName_(fixture_->fileManager_.getTemporaryFilePath("output.mdp")), + tprFileName_(fixture_->fileManager_.getTemporaryFilePath(".tpr")), + logFileName_(fixture_->fileManager_.getTemporaryFilePath(".log")), + edrFileName_(fixture_->fileManager_.getTemporaryFilePath(".edr")), + nsteps_(-2) { #ifdef GMX_LIB_MPI GMX_RELEASE_ASSERT(gmx_mpi_initialized(), "MPI system not initialized for mdrun tests"); #endif } -MdrunTestFixture::~MdrunTestFixture() -{ -} - // TODO The combination of defaulting to Verlet cut-off scheme, NVE, // and verlet-buffer-tolerance = -1 gives a grompp error. If we keep // things that way, this function should be renamed. For now, // force the use of the group scheme. void -MdrunTestFixture::useEmptyMdpFile() +SimulationRunner::useEmptyMdpFile() { useStringAsMdpFile("cutoff-scheme = Group\n"); } void -MdrunTestFixture::useStringAsMdpFile(const char *mdpString) +SimulationRunner::useStringAsMdpFile(const char *mdpString) { useStringAsMdpFile(std::string(mdpString)); } void -MdrunTestFixture::useStringAsMdpFile(const std::string &mdpString) +SimulationRunner::useStringAsMdpFile(const std::string &mdpString) { - gmx::File::writeFileFromString(mdpInputFileName, mdpString); + gmx::File::writeFileFromString(mdpInputFileName_, mdpString); } void -MdrunTestFixture::useStringAsNdxFile(const char *ndxString) +SimulationRunner::useStringAsNdxFile(const char *ndxString) { - gmx::File::writeFileFromString(ndxFileName, ndxString); + gmx::File::writeFileFromString(ndxFileName_, ndxString); } void -MdrunTestFixture::useTopGroAndNdxFromDatabase(const char *name) +SimulationRunner::useTopGroAndNdxFromDatabase(const char *name) { - topFileName = fileManager_.getInputFilePath((std::string(name) + ".top").c_str()); - groFileName = fileManager_.getInputFilePath((std::string(name) + ".gro").c_str()); - ndxFileName = fileManager_.getInputFilePath((std::string(name) + ".ndx").c_str()); + topFileName_ = fixture_->fileManager_.getInputFilePath((std::string(name) + ".top").c_str()); + groFileName_ = fixture_->fileManager_.getInputFilePath((std::string(name) + ".gro").c_str()); + ndxFileName_ = fixture_->fileManager_.getInputFilePath((std::string(name) + ".ndx").c_str()); } int -MdrunTestFixture::callGromppOnThisRank() +SimulationRunner::callGromppOnThisRank() { CommandLine caller; caller.append("grompp"); - caller.addOption("-f", mdpInputFileName); - caller.addOption("-n", ndxFileName); - caller.addOption("-p", topFileName); - caller.addOption("-c", groFileName); + caller.addOption("-f", mdpInputFileName_); + caller.addOption("-n", ndxFileName_); + caller.addOption("-p", topFileName_); + caller.addOption("-c", groFileName_); - caller.addOption("-po", mdpOutputFileName); - caller.addOption("-o", tprFileName); + caller.addOption("-po", mdpOutputFileName_); + caller.addOption("-o", tprFileName_); return gmx_grompp(caller.argc(), caller.argv()); } int -MdrunTestFixture::callGrompp() +SimulationRunner::callGrompp() { #ifdef GMX_LIB_MPI // When compiled with external MPI, only call one instance of the @@ -182,25 +179,25 @@ MdrunTestFixture::callGrompp() } int -MdrunTestFixture::callMdrun(const CommandLine &callerRef) +SimulationRunner::callMdrun(const CommandLine &callerRef) { /* Conforming to style guide by not passing a non-const reference to this function. Passing a non-const reference might make it easier to write code that incorrectly re-uses callerRef after the call to this function. */ CommandLine caller(callerRef); - caller.addOption("-s", tprFileName); + caller.addOption("-s", tprFileName_); - caller.addOption("-g", logFileName); - caller.addOption("-e", edrFileName); - caller.addOption("-o", fullPrecisionTrajectoryFileName); - caller.addOption("-x", reducedPrecisionTrajectoryFileName); + caller.addOption("-g", logFileName_); + caller.addOption("-e", edrFileName_); + caller.addOption("-o", fullPrecisionTrajectoryFileName_); + caller.addOption("-x", reducedPrecisionTrajectoryFileName_); - caller.addOption("-deffnm", fileManager_.getTemporaryFilePath("state")); + caller.addOption("-deffnm", fixture_->fileManager_.getTemporaryFilePath("state")); - if (nsteps > -2) + if (nsteps_ > -2) { - caller.addOption("-nsteps", nsteps); + caller.addOption("-nsteps", nsteps_); } #ifdef GMX_THREAD_MPI @@ -214,12 +211,35 @@ MdrunTestFixture::callMdrun(const CommandLine &callerRef) } int -MdrunTestFixture::callMdrun() +SimulationRunner::callMdrun() { CommandLine caller; caller.append("mdrun"); return callMdrun(caller); } +// ==== + +MdrunTestFixtureBase::MdrunTestFixtureBase() +{ +#ifdef GMX_LIB_MPI + GMX_RELEASE_ASSERT(gmx_mpi_initialized(), "MPI system not initialized for mdrun tests"); +#endif +} + +MdrunTestFixtureBase::~MdrunTestFixtureBase() +{ +} + +// ==== + +MdrunTestFixture::MdrunTestFixture() : runner_(this) +{ +} + +MdrunTestFixture::~MdrunTestFixture() +{ +} + } // namespace test } // namespace gmx diff --git a/src/programs/mdrun/tests/moduletest.h b/src/programs/mdrun/tests/moduletest.h index 3e9a7d0b2f..d021dc8621 100644 --- a/src/programs/mdrun/tests/moduletest.h +++ b/src/programs/mdrun/tests/moduletest.h @@ -52,41 +52,30 @@ namespace gmx namespace test { -/*! \libinternal \brief Declares test fixture for integration tests of - * mdrun functionality +/*! \libinternal \brief Helper object for running grompp and mdrun in + * integration tests of mdrun functionality * - * Derived fixture classes (or individual test cases) that might have - * specific requirements should assert that behaviour, rather than - * hard-code the requirements. A test that (for example) can't run - * with more than one thread should report that as a diagnostic, so the - * person running the test (or designing the test harness) can get - * feedback on what tests need what conditions without having to read - * the code of lots of tests. + * Objects of this class are intended to be owned by + * IntegrationTestFixture objects, and an IntegrationTestFixture + * object might own more than one SimulationRunner. * * The setup phase creates various temporary files for input and * output that are common for mdrun tests, using the file manager - * object of the parent class. Individual tests should create any - * extra filenames similarly, so that the test users's current working - * directory does not get littered with files left over from all - * manner of tests. - * - * Specifying the execution context (such as numbers of threads and - * processors) is normally sensible to specify from the test harness - * (i.e. when CMake/CTest/the user runs a test executable), because - * only there is information about the hardware available. The default - * values for such context provided in test fixtures for mdrun should - * mirror the defaults for mdrun, but currently mdrun.c hard-codes - * those in a gmx_hw_opt_t. + * object of the fixture that owns this object. Individual tests + * should create any extra filenames similarly, so that the test + * users's current working directory does not get littered with files + * left over from tests. * * Any method in this class may throw std::bad_alloc if out of memory. * * \ingroup module_mdrun_integration_tests */ -class MdrunTestFixture : public IntegrationTestFixture +class SimulationRunner { - protected: - MdrunTestFixture(); - virtual ~MdrunTestFixture(); + public: + /*! \brief Constructor, which establishes the fixture that + * will own each object */ + explicit SimulationRunner(IntegrationTestFixture *fixture_); //! Use an empty .mdp file as input to grompp void useEmptyMdpFile(); @@ -108,6 +97,10 @@ class MdrunTestFixture : public IntegrationTestFixture * with default command line */ int callMdrun(); + private: + //! Provides access to the test fixture, e.g. for the TestFileManager + IntegrationTestFixture *fixture_; + public: //@{ /*! \name Names for frequently used grompp and mdrun output files * @@ -119,23 +112,70 @@ class MdrunTestFixture : public IntegrationTestFixture * in a temporary directory and (by default behaviour of * TestFileManager) deleted when the test is complete. */ - std::string topFileName; - std::string groFileName; - std::string fullPrecisionTrajectoryFileName; - std::string reducedPrecisionTrajectoryFileName; - std::string groOutputFileName; - std::string ndxFileName; - std::string mdpInputFileName; - std::string mdpOutputFileName; - std::string tprFileName; - std::string logFileName; - std::string edrFileName; - std::string cptFileName; - std::string swapFileName; - int nsteps; + std::string topFileName_; + std::string groFileName_; + std::string fullPrecisionTrajectoryFileName_; + std::string reducedPrecisionTrajectoryFileName_; + std::string groOutputFileName_; + std::string ndxFileName_; + std::string mdpInputFileName_; + std::string mdpOutputFileName_; + std::string tprFileName_; + std::string logFileName_; + std::string edrFileName_; + std::string cptFileName_; + std::string swapFileName_; + int nsteps_; //@} }; +/*! \libinternal \brief Declares test fixture base class for + * integration tests of mdrun functionality + * + * Derived fixture classes (or individual test cases) that might have + * specific requirements should assert that behaviour, rather than + * hard-code the requirements. A test that (for example) can't run + * with more than one thread should report that as a diagnostic, so the + * person running the test (or designing the test harness) can get + * feedback on what tests need what conditions without having to read + * the code of lots of tests. + * + * Specifying the execution context (such as numbers of threads and + * processors) is normally sensible to specify from the test harness + * (i.e. when CMake/CTest/the user runs a test executable), because + * only there is information about the hardware available. The default + * values for such context provided in test fixtures for mdrun should + * mirror the defaults for mdrun, but currently mdrun.c hard-codes + * those in a gmx_hw_opt_t. + * + * Any method in this class may throw std::bad_alloc if out of memory. + * + * \ingroup module_mdrun_integration_tests + */ +class MdrunTestFixtureBase : public IntegrationTestFixture +{ + public: + MdrunTestFixtureBase(); + virtual ~MdrunTestFixtureBase(); +}; + +/*! \libinternal \brief Declares test fixture class for integration + * tests of mdrun functionality that use a single call of mdrun + * + * Any method in this class may throw std::bad_alloc if out of memory. + * + * \ingroup module_mdrun_integration_tests + */ +class MdrunTestFixture : public IntegrationTestFixture +{ + public: + MdrunTestFixture(); + virtual ~MdrunTestFixture(); + + //! Helper object to manage the preparation for and call of mdrun + SimulationRunner runner_; +}; + /*! \libinternal \brief * Parameterized test fixture for mdrun integration tests */ diff --git a/src/programs/mdrun/tests/replicaexchange.cpp b/src/programs/mdrun/tests/replicaexchange.cpp index e28f5f3016..37d9e02c12 100644 --- a/src/programs/mdrun/tests/replicaexchange.cpp +++ b/src/programs/mdrun/tests/replicaexchange.cpp @@ -105,9 +105,9 @@ class ReplicaExchangeTest : public gmx::test::ParameterizedMdrunTestFixture /* Prepare grompp output filenames inside the new temporary directory */ - mdpInputFileName = fileManager_.getTemporaryFilePath("input.mdp"); - mdpOutputFileName = fileManager_.getTemporaryFilePath("output.mdp"); - tprFileName = fileManager_.getTemporaryFilePath(".tpr"); + runner_.mdpInputFileName_ = fileManager_.getTemporaryFilePath("input.mdp"); + runner_.mdpOutputFileName_ = fileManager_.getTemporaryFilePath("output.mdp"); + runner_.tprFileName_ = fileManager_.getTemporaryFilePath(".tpr"); mdrunCaller.addOption("-deffnm", fileManager_.getTestSpecificFileNameRoot()); } @@ -151,7 +151,7 @@ class ReplicaExchangeTest : public gmx::test::ParameterizedMdrunTestFixture occur. */ std::max(baseTemperature - 10 * rank, real(0)), controlVariable); - useStringAsMdpFile(mdpFileContents); + runner_.useStringAsMdpFile(mdpFileContents); } //! MPI process set size @@ -183,10 +183,10 @@ TEST_P(ReplicaExchangeTest, ExitsNormally) organizeMultidir(); const char *pcoupl = GetParam(); organizeMdpFile(pcoupl); - useTopGroAndNdxFromDatabase("spc2"); + runner_.useTopGroAndNdxFromDatabase("spc2"); /* Call grompp on every rank - the standard callGrompp() only runs grompp on rank 0. */ - EXPECT_EQ(0, callGromppOnThisRank()); + EXPECT_EQ(0, runner_.callGromppOnThisRank()); mdrunCaller.addOption("-replex", 1); ASSERT_EQ(0, gmx_mdrun(mdrunCaller.argc(), mdrunCaller.argv())); diff --git a/src/programs/mdrun/tests/rerun.cpp b/src/programs/mdrun/tests/rerun.cpp index a8e2b87bc6..a73f2623be 100644 --- a/src/programs/mdrun/tests/rerun.cpp +++ b/src/programs/mdrun/tests/rerun.cpp @@ -64,16 +64,16 @@ class MdrunRerun : public gmx::test::MdrunTestFixture, /* Among other things, this test ensures mdrun can read a trajectory. */ TEST_P(MdrunRerun, WithDifferentInputFormats) { - useEmptyMdpFile(); - useTopGroAndNdxFromDatabase("spc2"); - EXPECT_EQ(0, callGrompp()); + runner_.useEmptyMdpFile(); + runner_.useTopGroAndNdxFromDatabase("spc2"); + EXPECT_EQ(0, runner_.callGrompp()); std::string rerunFileName = fileManager_.getInputFilePath(GetParam()); ::gmx::test::CommandLine rerunCaller; rerunCaller.append("mdrun"); rerunCaller.addOption("-rerun", rerunFileName); - ASSERT_EQ(0, callMdrun(rerunCaller)); + ASSERT_EQ(0, runner_.callMdrun(rerunCaller)); } /*! \brief Helper array of input files present in the source repo diff --git a/src/programs/mdrun/tests/swapcoords.cpp b/src/programs/mdrun/tests/swapcoords.cpp index 7f12d33b1b..ac43152206 100644 --- a/src/programs/mdrun/tests/swapcoords.cpp +++ b/src/programs/mdrun/tests/swapcoords.cpp @@ -77,29 +77,29 @@ typedef gmx::test::SwapTestFixture CompelTest; TEST_F(CompelTest, SwapCanRun) { std::string name = "OctaneSandwich"; - useTopGroAndNdxFromDatabase(name.c_str()); - mdpInputFileName = fileManager_.getInputFilePath((name + ".mdp").c_str()); + runner_.useTopGroAndNdxFromDatabase(name.c_str()); + runner_.mdpInputFileName_ = fileManager_.getInputFilePath((name + ".mdp").c_str()); - EXPECT_EQ(0, callGrompp()); + EXPECT_EQ(0, runner_.callGrompp()); - cptFileName = fileManager_.getTemporaryFilePath(".cpt"); - groOutputFileName = fileManager_.getTemporaryFilePath(".gro"); - swapFileName = fileManager_.getTemporaryFilePath("swap.xvg"); + runner_.cptFileName_ = fileManager_.getTemporaryFilePath(".cpt"); + runner_.groOutputFileName_ = fileManager_.getTemporaryFilePath(".gro"); + runner_.swapFileName_ = fileManager_.getTemporaryFilePath("swap.xvg"); ::gmx::test::CommandLine swapCaller; swapCaller.append("mdrun"); - swapCaller.addOption("-c", groOutputFileName); - swapCaller.addOption("-swap", swapFileName); + swapCaller.addOption("-c", runner_.groOutputFileName_); + swapCaller.addOption("-swap", runner_.swapFileName_); // Do an initial mdrun that writes a checkpoint file ::gmx::test::CommandLine firstCaller(swapCaller); - firstCaller.addOption("-cpo", cptFileName); - ASSERT_EQ(0, callMdrun(firstCaller)); + firstCaller.addOption("-cpo", runner_.cptFileName_); + ASSERT_EQ(0, runner_.callMdrun(firstCaller)); // Continue mdrun from that checkpoint file ::gmx::test::CommandLine secondCaller(swapCaller); - secondCaller.addOption("-cpi", cptFileName); - nsteps = 2; - ASSERT_EQ(0, callMdrun(secondCaller)); + secondCaller.addOption("-cpi", runner_.cptFileName_); + runner_.nsteps_ = 2; + ASSERT_EQ(0, runner_.callMdrun(secondCaller)); } diff --git a/src/programs/mdrun/tests/trajectory_writing.cpp b/src/programs/mdrun/tests/trajectory_writing.cpp index 6ab2425320..fafb1c3f92 100644 --- a/src/programs/mdrun/tests/trajectory_writing.cpp +++ b/src/programs/mdrun/tests/trajectory_writing.cpp @@ -69,13 +69,13 @@ class TrajectoryWritingTest : //! Execute the trajectory writing test void runTest() { - useStringAsMdpFile(theMdpFile); - useTopGroAndNdxFromDatabase("spc-and-methanol"); - EXPECT_EQ(0, callGrompp()); + runner_.useStringAsMdpFile(theMdpFile); + runner_.useTopGroAndNdxFromDatabase("spc-and-methanol"); + EXPECT_EQ(0, runner_.callGrompp()); - fullPrecisionTrajectoryFileName = fileManager_.getTemporaryFilePath("spc-and-methanol.tng"); - reducedPrecisionTrajectoryFileName = fileManager_.getTemporaryFilePath("spc-and-methanol-reduced.tng"); - ASSERT_EQ(0, callMdrun()); + runner_.fullPrecisionTrajectoryFileName_ = fileManager_.getTemporaryFilePath("spc-and-methanol.tng"); + runner_.reducedPrecisionTrajectoryFileName_ = fileManager_.getTemporaryFilePath("spc-and-methanol-reduced.tng"); + ASSERT_EQ(0, runner_.callMdrun()); // TODO When there is a way to sense something like the // output of gmx check, compare the result with that from // writing .trr and .xtc and assert the behaviour is diff --git a/src/testutils/integrationtests.h b/src/testutils/integrationtests.h index 86d4395426..f62da51767 100644 --- a/src/testutils/integrationtests.h +++ b/src/testutils/integrationtests.h @@ -86,17 +86,17 @@ class IntegrationTestFixture : public ::testing::Test * sufficiently modular to permit it to work. */ void redirectStderrToDevNull(); - /* TEST_F() constructs derived classes, and those classes - * might need to access implementation details, so we - * cannot use the private access specifer here. */ - protected: + /* TEST_F() constructs derived classes, and those classes and + * their member objects might need to access implementation + * details, so we cannot use the private access specifer + * here. */ + public: /*! \brief Object that manages finding input files, writing * temporary output files and cleaning up files. */ ::gmx::test::TestFileManager fileManager_; - public: //! Number of backup files to write static std::string s_maxBackup; };