Improve gmx::test::CommandLine.
authorTeemu Murtola <teemu.murtola@gmail.com>
Thu, 24 May 2012 06:39:42 +0000 (09:39 +0300)
committerTeemu Murtola <teemu.murtola@gmail.com>
Thu, 24 May 2012 15:26:24 +0000 (18:26 +0300)
- Make the class properly copyable for easier use in tests that need to
  manipulate it.
- Make the class manage its memory using std::vector and prevent memory
  leaks if the argv array is modified outside the class (the internal
  state of the class is still not correctly maintained, though).
- Add methods to append arguments.

Needed for #920, but split from the main change to keep it smaller.

Change-Id: I66ebe9d48936d35bce9e0b4fc1d6246d3d5ec07d

src/gromacs/commandline/tests/cmdlinemodulemanager.cpp
src/gromacs/commandline/tests/cmdlineparser.cpp
src/testutils/cmdlinetest.cpp
src/testutils/cmdlinetest.h

index a160996607cf4fb80279a09944d7f5a13f7cca26..08adc34dc112678e10b9054689dc86cda4ca77bf 100644 (file)
@@ -53,6 +53,8 @@
 namespace
 {
 
+using gmx::test::CommandLine;
+
 /********************************************************************
  * MockModule
  */
@@ -90,7 +92,7 @@ MockModule::MockModule(const char *name, const char *description)
 class CommandLineModuleManagerTest : public ::testing::Test
 {
     public:
-        void initManager(const gmx::test::CommandLine &args);
+        void initManager(const CommandLine &args);
         MockModule &addModule(const char *name, const char *description);
 
         gmx::CommandLineModuleManager &manager() { return *manager_; }
@@ -100,7 +102,7 @@ class CommandLineModuleManagerTest : public ::testing::Test
         boost::scoped_ptr<gmx::CommandLineModuleManager> manager_;
 };
 
-void CommandLineModuleManagerTest::initManager(const gmx::test::CommandLine &args)
+void CommandLineModuleManagerTest::initManager(const CommandLine &args)
 {
     manager_.reset();
     programInfo_.reset(new gmx::ProgramInfo("g_test", args.argc(), args.argv()));
@@ -124,7 +126,7 @@ TEST_F(CommandLineModuleManagerTest, RunsModule)
     const char *const cmdline[] = {
         "g_test", "module", "-flag", "yes"
     };
-    gmx::test::CommandLine args(cmdline);
+    CommandLine args(CommandLine::create(cmdline));
     initManager(args);
     MockModule &mod1 = addModule("module", "First module");
     addModule("other", "Second module");
@@ -143,7 +145,7 @@ TEST_F(CommandLineModuleManagerTest, RunsModuleBasedOnBinaryName)
     const char *const cmdline[] = {
         "g_module", "-flag", "yes"
     };
-    gmx::test::CommandLine args(cmdline);
+    CommandLine args(CommandLine::create(cmdline));
     initManager(args);
     MockModule &mod1 = addModule("module", "First module");
     addModule("other", "Second module");
@@ -162,7 +164,7 @@ TEST_F(CommandLineModuleManagerTest, RunsModuleBasedOnBinaryNameWithPathAndSuffi
     const char *const cmdline[] = {
         "/usr/local/gromacs/bin/g_module" GMX_BINARY_SUFFIX ".exe", "-flag", "yes"
     };
-    gmx::test::CommandLine args(cmdline);
+    CommandLine args(CommandLine::create(cmdline));
     initManager(args);
     MockModule &mod1 = addModule("module", "First module");
     addModule("other", "Second module");
@@ -181,7 +183,7 @@ TEST_F(CommandLineModuleManagerTest, HandlesConflictingBinaryAndModuleNames)
     const char *const cmdline[] = {
         "g_test", "test", "-flag", "yes"
     };
-    gmx::test::CommandLine args(cmdline);
+    CommandLine args(CommandLine::create(cmdline));
     initManager(args);
     MockModule &mod1 = addModule("test", "Test module");
     addModule("other", "Second module");
index ff5d5c5749568f5f204f1aec2d9c88b71ae1aeb3..6e6a1074e3bcd7bea416b320f426fae7d11d3a17 100644 (file)
@@ -51,6 +51,8 @@
 namespace
 {
 
+using gmx::test::CommandLine;
+
 class CommandLineParserTest : public ::testing::Test
 {
     public:
@@ -80,7 +82,7 @@ TEST_F(CommandLineParserTest, HandlesSingleValues)
     const char *const cmdline[] = {
         "test", "-flag", "yes", "-mvi", "2", "-mvd", "2.7"
     };
-    gmx::test::CommandLine args(cmdline);
+    CommandLine args(CommandLine::create(cmdline));
     ASSERT_NO_THROW(_parser.parse(&args.argc(), args.argv()));
     ASSERT_NO_THROW(_options.finish());
 
@@ -96,7 +98,7 @@ TEST_F(CommandLineParserTest, HandlesNegativeNumbers)
     const char *const cmdline[] = {
         "test", "-mvi", "1", "-2", "-mvd", "-2.7"
     };
-    gmx::test::CommandLine args(cmdline);
+    CommandLine args(CommandLine::create(cmdline));
     ASSERT_NO_THROW(_parser.parse(&args.argc(), args.argv()));
     ASSERT_NO_THROW(_options.finish());
 
index 2ce3de268bcda3c791fc3dd29435f0bcbe48e60f..e75064987d1fcfcb4a9942bc238a0e447044ebc5 100644 (file)
 #include <cstdlib>
 #include <cstring>
 
+#include <new>
+#include <vector>
+
 namespace gmx
 {
 namespace test
 {
 
+/********************************************************************
+ * CommandLine::Impl
+ */
+
+class CommandLine::Impl
+{
+    public:
+        Impl(const char *const cmdline[], size_t count);
+        ~Impl();
+
+        std::vector<char *>     args_;
+        std::vector<char *>     argv_;
+        int                     argc_;
+};
+
+CommandLine::Impl::Impl(const char *const cmdline[], size_t count)
+{
+    args_.reserve(count);
+    argv_.reserve(count);
+    argc_ = static_cast<int>(count);
+    for (size_t i = 0; i < count; ++i)
+    {
+        char *arg = strdup(cmdline[i]);
+        if (arg == NULL)
+        {
+            throw std::bad_alloc();
+        }
+        args_.push_back(arg);
+        argv_.push_back(arg);
+    }
+}
+
+CommandLine::Impl::~Impl()
+{
+    for (size_t i = 0; i < args_.size(); ++i)
+    {
+        std::free(args_[i]);
+    }
+}
+
+/********************************************************************
+ * CommandLine
+ */
+
+CommandLine::CommandLine()
+    : impl_(new Impl(NULL, 0))
+{
+}
+
+CommandLine::CommandLine(const char *const cmdline[], size_t count)
+    : impl_(new Impl(cmdline, count))
+{
+}
+
+CommandLine::CommandLine(const CommandLine &other)
+    : impl_(new Impl(other.argv(), other.argc()))
+{
+}
+
 CommandLine::~CommandLine()
 {
-    for (int i = 0; i < argc_; ++i)
+}
+
+void CommandLine::append(const char *arg)
+{
+    size_t newSize = impl_->args_.size() + 1;
+    impl_->args_.reserve(newSize);
+    impl_->argv_.reserve(newSize);
+    char *newArg = strdup(arg);
+    if (newArg == NULL)
     {
-        free(argv_[i]);
+        throw std::bad_alloc();
     }
-    delete [] argv_;
+    impl_->args_.push_back(newArg);
+    impl_->argv_.push_back(newArg);
+    impl_->argc_ = static_cast<int>(newSize);
 }
 
-void CommandLine::initCommandLine(const char *const cmdline[], size_t count)
+int &CommandLine::argc() { return impl_->argc_; }
+char **CommandLine::argv() { return &impl_->argv_[0]; }
+int CommandLine::argc() const { return impl_->argc_; }
+const char *const *CommandLine::argv() const { return &impl_->argv_[0]; }
+const char *CommandLine::arg(int i) const { return impl_->argv_[i]; }
+
+std::string CommandLine::toString() const
 {
-    argv_ = new char *[count];
-    for (size_t i = 0; i < count; ++i)
+    std::string result;
+    for (size_t i = 0; i < impl_->args_.size(); ++i)
     {
-        argv_[i] = strdup(cmdline[i]);
-        // TODO: Check for failure;
+        if (i > 0)
+        {
+            result.append(" ");
+        }
+        const char *arg = impl_->args_[i];
+        bool bSpaces = (std::strchr(arg, ' ') != NULL);
+        if (bSpaces)
+        {
+            result.append("'");
+        }
+        result.append(arg);
+        if (bSpaces)
+        {
+            result.append("'");
+        }
     }
-    argc_ = static_cast<int>(count);
+    return result;
 }
 
 } // namespace test
index d6b2184733d7ac89dd8e937ba8af52769de231a5..7b91621005e567837cea700465ad9682fa096486 100644 (file)
@@ -39,7 +39,9 @@
 #ifndef GMX_TESTUTILS_CMDLINETEST_H
 #define GMX_TESTUTILS_CMDLINETEST_H
 
-#include <cstddef>
+#include <string>
+
+#include "gromacs/utility/common.h"
 
 namespace gmx
 {
@@ -50,17 +52,21 @@ namespace test
  * Helper class for tests that check command-line handling.
  *
  * This class helps in writing tests for command-line handling.
- * The constructor takes an array of const char pointers, specifying the
+ * The create() method takes an array of const char pointers, specifying the
  * command-line arguments, each as one array element.
  * The argc() and argv() methods can then be used to obtain the argc and argv
  * (non-const char pointers) arrays for passing into methods that expect these.
  *
  * Note that although the interface allows passing the argc and argv pointers
  * to methods that modify them (typically as \p f(&argc(), argv())), currently
- * memory leaks occur if the parameters are actually modified.
+ * the CommandLine object is not in a consistent state internally if the
+ * parameters are actually modified.
  * Currently, the C++ methods with this signature do not modify their
  * parameters, so this is not yet a problem.
  *
+ * All constructors and methods that modify this class may throw an
+ * std::bad_alloc.  Const methods and accessors do not throw.
+ *
  * \inlibraryapi
  * \ingroup module_testutils
  */
@@ -68,37 +74,68 @@ class CommandLine
 {
     public:
         /*! \brief
-         * Initializes a command-line object.
+         * Initializes a command-line object from a const C array.
          *
          * \param[in] cmdline  Array of command-line arguments.
-         * \tparam    count Deduced number of elements in \p cmdline.
-         * \throws    std::bad_alloc if out of memory.
+         * \tparam    count    Deduced number of elements in \p cmdline.
          *
          * \p cmdline should include the binary name as the first element if
          * that is desired in the output.
+         *
+         * This is not a constructor, because template constructors are not
+         * possible with a private implementation class.
          */
-        template <size_t count>
-        explicit CommandLine(const char *const (&cmdline)[count])
+        template <size_t count> static
+        CommandLine create(const char *const (&cmdline)[count])
         {
-            initCommandLine(cmdline, count);
+            return CommandLine(cmdline, count);
         }
+
+        //! Initializes an empty command-line object.
+        CommandLine();
+        /*! \brief
+         * Initializes a command-line object.
+         *
+         * \param[in] cmdline  Array of command-line arguments.
+         * \param[in] count    Number of elements in \p cmdline.
+         *
+         * \p cmdline should include the binary name as the first element if
+         * that is desired in the output.
+         */
+        CommandLine(const char *const cmdline[], size_t count);
+        //! Creates a deep copy of a command-line object.
+        CommandLine(const CommandLine &other);
         ~CommandLine();
 
+        /*! \brief
+         * Append an argument to the command line.
+         *
+         * \param[in] arg  Argument to append.
+         *
+         * Strong exception safety.
+         */
+        void append(const char *arg);
+        //! Convenience overload taking a std::string.
+        void append(const std::string &arg) { append(arg.c_str()); }
+
         //! Returns argc for passing into C-style command-line handling.
-        int &argc() { return argc_; }
+        int &argc();
         //! Returns argv for passing into C-style command-line handling.
-        char **argv() { return argv_; }
+        char **argv();
         //! Returns argc for passing into C-style command-line handling.
-        int argc() const { return argc_; }
+        int argc() const;
         //! Returns argv for passing into C-style command-line handling.
-        const char *const *argv() const { return argv_; }
+        const char *const *argv() const;
+        //! Returns a single argument.
+        const char *arg(int i) const;
+
+        //! Returns the command line formatted as a single string.
+        std::string toString() const;
 
     private:
-        //! Internal helper method used to implement the constructor.
-        void initCommandLine(const char *const cmdline[], size_t count);
+        class Impl;
 
-        int                     argc_;
-        char                  **argv_;
+        PrivateImplPointer<Impl> impl_;
 };
 
 } // namespace test