Fix valgrind issues with configurable_test (#7424)

Summary:
Valgrind was reporting a problem with the configurable_test in some GTEST code.  This problem was caused by using a std::function as a GTEST parameter.  This change changes the test to use a string as a function parameter (backed by a map) and fixes the valgrind issue.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7424

Reviewed By: ajkr

Differential Revision: D23855540

Pulled By: pdillinger

fbshipit-source-id: 2f2be03f7f92d96644aa9fa6481e4f37f2cfa5f5

options/configurable_test.cc

@@ -111,22 +111,6 @@ class ConfigurableTest : public testing::Test {
   ConfigOptions config_options_;
 };
 
-class ConfigurableParamTest
-    : public ConfigurableTest,
-      virtual public ::testing::WithParamInterface<
-          std::pair<std::string, ConfigTestFactoryFunc>> {
- public:
-  ConfigurableParamTest() {
-    configuration_ = GetParam().first;
-    factory_ = GetParam().second;
-    object_.reset(factory_());
-  }
-  void TestConfigureOptions(const ConfigOptions& opts);
-  ConfigTestFactoryFunc factory_;
-  std::string configuration_;
-  std::unique_ptr<Configurable> object_;
-};
-
 TEST_F(ConfigurableTest, GetOptionsPtrTest) {
   std::string opt_str;
   std::unique_ptr<Configurable> configurable(SimpleConfigurable::Create());
@@ -598,14 +582,101 @@ TEST_F(ConfigurableTest, TestNoCompare) {
 }
 #endif
 
+static std::unordered_map<std::string, ConfigTestFactoryFunc> TestFactories = {
+    {"Simple", []() { return SimpleConfigurable::Create("simple"); }},
+    {"Struct", []() { return SimpleStructFactory(); }},
+    {"Unique",
+     []() {
+       return SimpleConfigurable::Create(
+           "simple", TestConfigMode::kSimpleMode | TestConfigMode::kUniqueMode);
+     }},
+    {"Shared",
+     []() {
+       return SimpleConfigurable::Create(
+           "simple", TestConfigMode::kSimpleMode | TestConfigMode::kSharedMode);
+     }},
+    {"Nested",
+     []() {
+       return SimpleConfigurable::Create(
+           "simple", TestConfigMode::kSimpleMode | TestConfigMode::kNestedMode);
+     }},
+    {"Mutable",
+     []() {
+       return SimpleConfigurable::Create("simple",
+                                         TestConfigMode::kMutableMode |
+                                             TestConfigMode::kSimpleMode |
+                                             TestConfigMode::kNestedMode);
+     }},
+    {"ThreeWay",
+     []() {
+       std::shared_ptr<Configurable> child;
+       child.reset(
+           SimpleConfigurable::Create("child", TestConfigMode::kDefaultMode));
+       std::shared_ptr<Configurable> parent;
+       parent.reset(new WrappedConfigurable(
+           "parent", TestConfigMode::kDefaultMode, child));
+       return new WrappedConfigurable("master", TestConfigMode::kDefaultMode,
+                                      parent);
+     }},
+    {"ThreeDeep",
+     []() {
+       Configurable* simple = SimpleConfigurable::Create(
+           "Simple",
+           TestConfigMode::kUniqueMode | TestConfigMode::kDefaultMode);
+       auto* unique =
+           simple->GetOptions<std::unique_ptr<Configurable>>("SimpleUnique");
+       unique->reset(SimpleConfigurable::Create(
+           "Child",
+           TestConfigMode::kUniqueMode | TestConfigMode::kDefaultMode));
+       unique = unique->get()->GetOptions<std::unique_ptr<Configurable>>(
+           "ChildUnique");
+       unique->reset(
+           SimpleConfigurable::Create("Child", TestConfigMode::kDefaultMode));
+       return simple;
+     }},
+    {"DBOptions",
+     []() {
+       auto config = DBOptionsAsConfigurable(DBOptions());
+       return config.release();
+     }},
+    {"CFOptions",
+     []() {
+       auto config = CFOptionsAsConfigurable(ColumnFamilyOptions());
+       return config.release();
+     }},
+    {"BlockBased", []() { return NewBlockBasedTableFactory(); }},
+};
+
+class ConfigurableParamTest : public ConfigurableTest,
+                              virtual public ::testing::WithParamInterface<
+                                  std::pair<std::string, std::string>> {
+ public:
+  ConfigurableParamTest() {
+    type_ = GetParam().first;
+    configuration_ = GetParam().second;
+    assert(TestFactories.find(type_) != TestFactories.end());
+    object_.reset(CreateConfigurable());
+  }
+
+  Configurable* CreateConfigurable() {
+    const auto& iter = TestFactories.find(type_);
+    return (iter->second)();
+  }
+
+  void TestConfigureOptions(const ConfigOptions& opts);
+  std::string type_;
+  std::string configuration_;
+  std::unique_ptr<Configurable> object_;
+};
+
 void ConfigurableParamTest::TestConfigureOptions(
     const ConfigOptions& config_options) {
   std::unique_ptr<Configurable> base, copy;
   std::unordered_set<std::string> names;
   std::string opt_str, mismatch;
 
-  base.reset(factory_());
+  base.reset(CreateConfigurable());
-  copy.reset(factory_());
+  copy.reset(CreateConfigurable());
 
   ASSERT_OK(base->ConfigureFromString(config_options, configuration_));
   ASSERT_OK(base->GetOptionString(config_options, &opt_str));
@@ -613,7 +684,7 @@ void ConfigurableParamTest::TestConfigureOptions(
   ASSERT_OK(copy->GetOptionString(config_options, &opt_str));
   ASSERT_TRUE(base->AreEquivalent(config_options, copy.get(), &mismatch));
 
-  copy.reset(factory_());
+  copy.reset(CreateConfigurable());
   ASSERT_OK(base->GetOptionNames(config_options, &names));
   std::unordered_map<std::string, std::string> unused;
   bool found_one = false;
@@ -655,7 +726,7 @@ TEST_P(ConfigurableParamTest, GetDefaultOptionsTest) {
 TEST_P(ConfigurableParamTest, ConfigureFromPropsTest) {
   std::string opt_str, mismatch;
   std::unordered_set<std::string> names;
-  std::unique_ptr<Configurable> copy(factory_());
+  std::unique_ptr<Configurable> copy(CreateConfigurable());
 
   ASSERT_OK(object_->ConfigureFromString(config_options_, configuration_));
   config_options_.delimiter = "\n";
@@ -674,110 +745,43 @@ TEST_P(ConfigurableParamTest, ConfigureFromPropsTest) {
   ASSERT_TRUE(object_->AreEquivalent(config_options_, copy.get(), &mismatch));
 }
 
-static Configurable* SimpleFactory() {
-  return SimpleConfigurable::Create("simple");
-}
-
-static Configurable* UniqueFactory() {
-  return SimpleConfigurable::Create(
-      "simple", TestConfigMode::kSimpleMode | TestConfigMode::kUniqueMode);
-}
-static Configurable* SharedFactory() {
-  return SimpleConfigurable::Create(
-      "simple", TestConfigMode::kSimpleMode | TestConfigMode::kSharedMode);
-}
-
-static Configurable* NestedFactory() {
-  return SimpleConfigurable::Create(
-      "simple", TestConfigMode::kSimpleMode | TestConfigMode::kNestedMode);
-}
-
-static Configurable* MutableFactory() {
-  return SimpleConfigurable::Create("simple", TestConfigMode::kMutableMode |
-                                                  TestConfigMode::kSimpleMode |
-                                                  TestConfigMode::kNestedMode);
-}
-
-static Configurable* ThreeWrappedFactory() {
-  std::shared_ptr<Configurable> child;
-  child.reset(
-      SimpleConfigurable::Create("child", TestConfigMode::kDefaultMode));
-  std::shared_ptr<Configurable> parent;
-  parent.reset(
-      new WrappedConfigurable("parent", TestConfigMode::kDefaultMode, child));
-  return new WrappedConfigurable("master", TestConfigMode::kDefaultMode,
-                                 parent);
-}
-
-static Configurable* ThreeDeepFactory() {
-  Configurable* simple = SimpleConfigurable::Create(
-      "Simple", TestConfigMode::kUniqueMode | TestConfigMode::kDefaultMode);
-  auto* unique =
-      simple->GetOptions<std::unique_ptr<Configurable>>("SimpleUnique");
-  unique->reset(SimpleConfigurable::Create(
-      "Child", TestConfigMode::kUniqueMode | TestConfigMode::kDefaultMode));
-  unique =
-      unique->get()->GetOptions<std::unique_ptr<Configurable>>("ChildUnique");
-  unique->reset(
-      SimpleConfigurable::Create("Child", TestConfigMode::kDefaultMode));
-  return simple;
-}
-
-static Configurable* DBOptionsFactory() {
-  auto config = DBOptionsAsConfigurable(DBOptions());
-  return config.release();
-}
-
-static Configurable* CFOptionsFactory() {
-  auto config = CFOptionsAsConfigurable(ColumnFamilyOptions());
-  return config.release();
-}
-
-static Configurable* BlockBasedFactory() { return NewBlockBasedTableFactory(); }
-
 INSTANTIATE_TEST_CASE_P(
     ParamTest, ConfigurableParamTest,
     testing::Values(
-        std::pair<std::string, ConfigTestFactoryFunc>(
+        std::pair<std::string, std::string>("Simple",
-            "int=42;bool=true;string=s", SimpleFactory),
+                                            "int=42;bool=true;string=s"),
-        std::pair<std::string, ConfigTestFactoryFunc>(
+        std::pair<std::string, std::string>(
-            "int=42;unique={int=33;string=unique}", MutableFactory),
+            "Mutable", "int=42;unique={int=33;string=unique}"),
-        std::pair<std::string, ConfigTestFactoryFunc>(
+        std::pair<std::string, std::string>(
-            "struct={int=33;bool=true;string=s;}", SimpleStructFactory),
+            "Struct", "struct={int=33;bool=true;string=s;}"),
-        std::pair<std::string, ConfigTestFactoryFunc>(
+        std::pair<std::string, std::string>("Shared",
-            "int=33;bool=true;string=outer;"
+                                            "int=33;bool=true;string=outer;"
-            "shared={int=42;string=shared}",
+                                            "shared={int=42;string=shared}"),
-            SharedFactory),
+        std::pair<std::string, std::string>("Unique",
-        std::pair<std::string, ConfigTestFactoryFunc>(
+                                            "int=33;bool=true;string=outer;"
-            "int=33;bool=true;string=outer;"
+                                            "unique={int=42;string=unique}"),
-            "unique={int=42;string=unique}",
+        std::pair<std::string, std::string>("Nested",
-            UniqueFactory),
+                                            "int=11;bool=true;string=outer;"
-        std::pair<std::string, ConfigTestFactoryFunc>(
+                                            "pointer={int=22;string=pointer};"
-            "int=11;bool=true;string=outer;"
+                                            "unique={int=33;string=unique};"
-            "pointer={int=22;string=pointer};"
+                                            "shared={int=44;string=shared}"),
-            "unique={int=33;string=unique};"
+        std::pair<std::string, std::string>("ThreeWay",
-            "shared={int=44;string=shared}",
+                                            "int=11;bool=true;string=outer;"
-            NestedFactory),
+                                            "inner={int=22;string=parent;"
-        std::pair<std::string, ConfigTestFactoryFunc>(
+                                            "inner={int=33;string=child}};"),
-            "int=11;bool=true;string=outer;"
+        std::pair<std::string, std::string>("ThreeDeep",
-            "inner={int=22;string=parent;"
+                                            "int=11;bool=true;string=outer;"
-            "inner={int=33;string=child}};",
+                                            "unique={int=22;string=inner;"
-            ThreeWrappedFactory),
+                                            "unique={int=33;string=unique}};"),
-        std::pair<std::string, ConfigTestFactoryFunc>(
+        std::pair<std::string, std::string>("DBOptions",
-            "int=11;bool=true;string=outer;"
+                                            "max_background_jobs=100;"
-            "unique={int=22;string=inner;"
+                                            "max_open_files=200;"),
-            "unique={int=33;string=unique}};",
+        std::pair<std::string, std::string>("CFOptions",
-            ThreeDeepFactory),
+                                            "table_factory=BlockBasedTable;"
-        std::pair<std::string, ConfigTestFactoryFunc>("max_background_jobs=100;"
+                                            "disable_auto_compactions=true;"),
-                                                      "max_open_files=200;",
+        std::pair<std::string, std::string>("BlockBased",
-                                                      DBOptionsFactory),
+                                            "block_size=1024;"
-        std::pair<std::string, ConfigTestFactoryFunc>(
+                                            "no_block_cache=true;")));
-            "table_factory=BlockBasedTable;"
-            "disable_auto_compactions=true;",
-            CFOptionsFactory),
-        std::pair<std::string, ConfigTestFactoryFunc>("block_size=1024;"
-                                                      "no_block_cache=true;",
-                                                      BlockBasedFactory)));
 #endif  // ROCKSDB_LITE
 
 }  // namespace test