Improve option support & tests for key-value trees
authorTeemu Murtola <teemu.murtola@gmail.com>
Sat, 11 Mar 2017 13:14:30 +0000 (15:14 +0200)
committerDavid van der Spoel <davidvanderspoel@gmail.com>
Mon, 13 Mar 2017 08:45:27 +0000 (09:45 +0100)
Add basic tests for the behavior of different option types with
key-value tree handling.  For now, the test covers only the "adjust"
function.  Make enum options serialize as strings instead of integers,
which improves behavior of gmx dump and gmx check, and can also be more
robust for backward/forward compatibility.

Improve the way the key-value tree serialization tests are done: use the
in-memory serializer to test roundtrips, instead of relying on the
reference data.  There is less code this way, and the tests behave
better in the absence of reference data.

Change-Id: I5e52265333f397bd6e058636691cbfd21092df0d

14 files changed:
src/gromacs/options/basicoptions.cpp
src/gromacs/options/basicoptionstorage.h
src/gromacs/options/optionstoragetemplate.h
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsBooleanOption.xml [new file with mode: 0644]
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsDoubleOption.xml [new file with mode: 0644]
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsEnumIntOption.xml [new file with mode: 0644]
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsEnumOption.xml [new file with mode: 0644]
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsFloatOption.xml [new file with mode: 0644]
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsInt64Option.xml [new file with mode: 0644]
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsIntegerOption.xml [new file with mode: 0644]
src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsStringOption.xml [new file with mode: 0644]
src/gromacs/options/tests/treesupport.cpp
src/gromacs/utility/tests/keyvaluetreeserializer.cpp
src/testutils/refdata.cpp

index 86022b3d7578b5e74af38884d23c38e6ee6ff225..31f9ceb7b82240e0528f7aa352a30b906395f94b 100644 (file)
@@ -610,6 +610,11 @@ std::string EnumOptionStorage::formatSingleValue(const int &value) const
     return allowed_[value];
 }
 
+Variant EnumOptionStorage::normalizeValue(const int &value) const
+{
+    return Variant::create<std::string>(formatSingleValue(value));
+}
+
 void EnumOptionStorage::initConverter(ConverterType *converter)
 {
     converter->addConverter<std::string>(
index 92aefa31fa6065a0b0693fb59a1ec797a97b412d..f3e4be1fac4e236fa70e922801e41dafe95fad07 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2013,2014,2015,2016, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2013,2014,2015,2016,2017, 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.
@@ -244,6 +244,7 @@ class EnumOptionStorage : public OptionStorageTemplateSimple<int>
         virtual std::string typeString() const { return "enum"; }
         virtual std::string formatExtraDescription() const;
         virtual std::string formatSingleValue(const int &value) const;
+        virtual Variant normalizeValue(const int &value) const;
 
         //! \copydoc EnumOptionInfo::allowedValues()
         const std::vector<std::string> &allowedValues() const { return allowed_; }
index 2dae1aa9f4958be60c129a992b60543792c6e095..d2d9dac15ffbd2b143770456a15d2b81e47e3906 100644 (file)
@@ -334,9 +334,7 @@ class OptionStorageTemplateSimple : public OptionStorageTemplate<T>
             std::vector<Variant> result;
             for (const auto &value : values)
             {
-                result.push_back(
-                        Variant::create<T>(
-                                processValue(converter_.convert(value))));
+                result.push_back(normalizeValue(converter_.convert(value)));
             }
             return result;
         }
@@ -359,6 +357,19 @@ class OptionStorageTemplateSimple : public OptionStorageTemplate<T>
         {
             return value;
         }
+        /*! \brief
+         * Converts a single value to normalized type.
+         *
+         * \param[in] value  Value after conversion.
+         * \returns   Value to store for the option.
+         *
+         * This can be overridden to serialize a different type than `T`
+         * when using the option with KeyValueTreeObject.
+         */
+        virtual Variant normalizeValue(const T &value) const
+        {
+            return Variant::create<T>(processValue(value));
+        }
 
     private:
         virtual void convertValue(const Variant &variant)
@@ -471,7 +482,7 @@ std::vector<Variant> OptionStorageTemplate<T>::defaultValues() const
     {
         result.push_back(Variant::create<T>(value));
     }
-    return result;
+    return normalizeValues(result);
 }
 
 
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsBooleanOption.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsBooleanOption.xml
new file mode 100644 (file)
index 0000000..ae2a0b9
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <Bool Name="a">true</Bool>
+  </Object>
+</ReferenceData>
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsDoubleOption.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsDoubleOption.xml
new file mode 100644 (file)
index 0000000..03e296e
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <Real Name="a">1.5</Real>
+  </Object>
+</ReferenceData>
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsEnumIntOption.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsEnumIntOption.xml
new file mode 100644 (file)
index 0000000..db29313
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <String Name="a">foo</String>
+  </Object>
+</ReferenceData>
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsEnumOption.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsEnumOption.xml
new file mode 100644 (file)
index 0000000..db29313
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <String Name="a">foo</String>
+  </Object>
+</ReferenceData>
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsFloatOption.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsFloatOption.xml
new file mode 100644 (file)
index 0000000..03e296e
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <Real Name="a">1.5</Real>
+  </Object>
+</ReferenceData>
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsInt64Option.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsInt64Option.xml
new file mode 100644 (file)
index 0000000..d8e09c7
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <Int64 Name="a">2</Int64>
+  </Object>
+</ReferenceData>
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsIntegerOption.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsIntegerOption.xml
new file mode 100644 (file)
index 0000000..bed5abe
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <Int Name="a">2</Int>
+  </Object>
+</ReferenceData>
diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsStringOption.xml b/src/gromacs/options/tests/refdata/TreeValueSupportTest_SupportsStringOption.xml
new file mode 100644 (file)
index 0000000..de8351c
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input"></Object>
+  <Object Name="Adjusted">
+    <String Name="a">s</String>
+  </Object>
+</ReferenceData>
index cf1470733382dd62f80c223e442d4c32d0487582..390a375f231b1c52f8b9111c8a4cc15517f0ccd4 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2016, by the GROMACS development team, led by
+ * Copyright (c) 2016,2017, 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.
@@ -218,4 +218,82 @@ TEST_F(TreeValueSupportAdjustTest, OrdersValues)
     runTest();
 }
 
+/********************************************************************
+ * Support for different option types
+ */
+
+class TreeValueSupportTest : public ::testing::Test
+{
+    public:
+        void runTest()
+        {
+            gmx::test::TestReferenceData    refdata;
+            gmx::test::TestReferenceChecker checker(refdata.rootChecker());
+            gmx::KeyValueTreeObject         tree(builder_.build());
+            checker.checkKeyValueTreeObject(tree, "Input");
+            ASSERT_NO_THROW_GMX(tree = gmx::adjustKeyValueTreeFromOptions(tree, options_));
+            checker.checkKeyValueTreeObject(tree, "Adjusted");
+        }
+
+        gmx::Options              options_;
+        gmx::KeyValueTreeBuilder  builder_;
+};
+
+TEST_F(TreeValueSupportTest, SupportsBooleanOption)
+{
+    options_.addOption(gmx::BooleanOption("a").defaultValue(true));
+    runTest();
+}
+
+TEST_F(TreeValueSupportTest, SupportsIntegerOption)
+{
+    options_.addOption(gmx::IntegerOption("a").defaultValue(2));
+    runTest();
+}
+
+TEST_F(TreeValueSupportTest, SupportsInt64Option)
+{
+    options_.addOption(gmx::Int64Option("a").defaultValue(2));
+    runTest();
+}
+
+TEST_F(TreeValueSupportTest, SupportsStringOption)
+{
+    options_.addOption(gmx::StringOption("a").defaultValue("s"));
+    runTest();
+}
+
+TEST_F(TreeValueSupportTest, SupportsFloatOption)
+{
+    options_.addOption(gmx::FloatOption("a").defaultValue(1.5));
+    runTest();
+}
+
+TEST_F(TreeValueSupportTest, SupportsDoubleOption)
+{
+    options_.addOption(gmx::DoubleOption("a").defaultValue(1.5));
+    runTest();
+}
+
+TEST_F(TreeValueSupportTest, SupportsEnumIntOption)
+{
+    const char *const values[] = {"foo", "bar"};
+    options_.addOption(gmx::EnumIntOption("a").enumValue(values).defaultValue(0));
+    runTest();
+}
+
+//! Enum for testing EnumOption.
+enum class TestEnum
+{
+    Foo, Bar
+};
+
+TEST_F(TreeValueSupportTest, SupportsEnumOption)
+{
+    const char *const values[] = {"foo", "bar"};
+    options_.addOption(gmx::EnumOption<TestEnum>("a").enumValue(values)
+                           .defaultValue(TestEnum::Foo));
+    runTest();
+}
+
 } // namespace
index f94866c8a6766a42086265227a2df0e284ac7c5b..6f2e092c07133d1f67c0f7e7e7d1249aa2d6aa75 100644 (file)
@@ -38,6 +38,7 @@
 
 #include <gtest/gtest.h>
 
+#include "gromacs/utility/inmemoryserializer.h"
 #include "gromacs/utility/iserializer.h"
 #include "gromacs/utility/keyvaluetreebuilder.h"
 
 namespace
 {
 
-class RefDataWriteSerializer : public gmx::ISerializer
+class RefDataSerializer : public gmx::ISerializer
 {
     public:
-        RefDataWriteSerializer(gmx::test::TestReferenceChecker *parentChecker,
-                               const char                      *id)
+        RefDataSerializer(gmx::test::TestReferenceChecker *parentChecker,
+                          const char                      *id)
             : checker_(parentChecker->checkCompound("SerializedData", id))
         {
         }
@@ -86,46 +87,6 @@ class RefDataWriteSerializer : public gmx::ISerializer
         gmx::test::TestReferenceChecker checker_;
 };
 
-class RefDataReadSerializer : public gmx::ISerializer
-{
-    public:
-        RefDataReadSerializer(gmx::test::TestReferenceChecker *parentChecker,
-                              const char                      *id)
-            : checker_(parentChecker->checkCompound("SerializedData", id))
-        {
-        }
-
-        virtual bool reading() const { return true; }
-
-        virtual void doUChar(unsigned char *value)
-        {
-            *value = checker_.readUChar(nullptr);
-        }
-        virtual void doInt(int *value)
-        {
-            *value = checker_.readInteger(nullptr);
-        }
-        virtual void doInt64(gmx_int64_t *value)
-        {
-            *value = checker_.readInt64(nullptr);
-        }
-        virtual void doFloat(float *value)
-        {
-            *value = checker_.readFloat(nullptr);
-        }
-        virtual void doDouble(double *value)
-        {
-            *value = checker_.readDouble(nullptr);
-        }
-        virtual void doString(std::string *value)
-        {
-            *value = checker_.readString(nullptr);
-        }
-
-    private:
-        gmx::test::TestReferenceChecker checker_;
-};
-
 class KeyValueTreeSerializerTest : public ::testing::Test
 {
     public:
@@ -136,18 +97,27 @@ class KeyValueTreeSerializerTest : public ::testing::Test
             gmx::test::TestReferenceChecker   checker(data.rootChecker());
             checker.checkKeyValueTreeObject(input, "Input");
             {
-                RefDataWriteSerializer        serializer(&checker, "Stream");
+                RefDataSerializer             serializer(&checker, "Stream");
                 gmx::serializeKeyValueTree(input, &serializer);
             }
+            std::vector<char>                 buffer = serializeTree(input);
             {
-                RefDataReadSerializer         serializer(&checker, "Stream");
+                gmx::InMemoryDeserializer     deserializer(buffer);
                 gmx::KeyValueTreeObject       output
-                    = gmx::deserializeKeyValueTree(&serializer);
+                    = gmx::deserializeKeyValueTree(&deserializer);
                 checker.checkKeyValueTreeObject(output, "Input");
             }
         }
 
         gmx::KeyValueTreeBuilder builder_;
+
+    private:
+        std::vector<char> serializeTree(const gmx::KeyValueTreeObject &tree)
+        {
+            gmx::InMemorySerializer serializer;
+            gmx::serializeKeyValueTree(tree, &serializer);
+            return serializer.finishAndGetBuffer();
+        }
 };
 
 TEST_F(KeyValueTreeSerializerTest, EmptyTree)
index 474feff22fa371a26666ae47807bbfd9d7111bb8..2d80f779f7582aeb5dd30a862e5249de8d05b600 100644 (file)
@@ -982,6 +982,10 @@ void TestReferenceChecker::checkVariant(const Variant &variant, const char *id)
     {
         checkInteger(variant.cast<int>(), id);
     }
+    else if (variant.isType<gmx_int64_t>())
+    {
+        checkInt64(variant.cast<gmx_int64_t>(), id);
+    }
     else if (variant.isType<float>())
     {
         checkFloat(variant.cast<float>(), id);