[3952] Update hooks-libraries parser tests

These tests were updated while investigating the issue described in
the ticket.

diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
index 315467f63ab237324487fde703892bdb789b01d1..b851819ce1467bda533f55d1b22571641e34bb7c 100644 (file)
--- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
+++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
@@ -823,114 +823,180 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) {
 
 };  // Anonymous namespace
 
-/// These tests check basic operation of the HooksLibrariesParser.
+/// The next set of tests check basic operation of the HooksLibrariesParser.
 
-// hooks-libraries that do not contain anything.
-TEST_F(ParseConfigTest, noHooksLibrariesTest) {
 
-    // Configuration with hooks-libraries not present.
-    string config = "{ \"hooks-libraries\": [] }";
+// Utility function for setting up the "hooks-libraries" configuration.
+//
+// Returns a hooks-libraries configuration element that contains zero to
+// three libraries, depending on what arguments are supplied.
+std::string
+setHooksLibrariesConfig(const char* lib1 = NULL, const char* lib2 = NULL,
+                        const char* lib3 = NULL) {
+    const std::string quote("\"");
+    const std::string comma_space(", ");
+
+    std::string config = std::string("{ \"hooks-libraries\": [");
+    if (lib1 != NULL) {
+        config += (quote + std::string(lib1) + quote);
+        if (lib2 != NULL) {
+            config += (comma_space + quote + std::string(lib2) + quote);
+            if (lib3 != NULL) {
+                config += (comma_space + quote + std::string(lib3) + quote);
+            }
+        }
+    }
+    config += std::string("] }");
+
+    return (config);
+}
+
+// hooks-libraries element that does not contain anything.
+TEST_F(ParseConfigTest, noHooksLibraries) {
+
+    // Create an empty hooks-libraries configuration element.
+    const string config = setHooksLibrariesConfig();
 
     // Verify that the configuration string parses.
-    int rcode = parseConfiguration(config);
+    const int rcode = parseConfiguration(config);
     ASSERT_TRUE(rcode == 0) << error_text_;
 
-    // Check that the parser recorded no change to the current state
-    // (as the test starts with no hooks libraries loaded).
+    // Check that the parser recorded no libraries (and no change of state).
     std::vector<std::string> libraries;
     bool changed;
     hooks_libraries_parser_->getLibraries(libraries, changed);
-    EXPECT_TRUE(libraries.empty());
     EXPECT_FALSE(changed);
+    EXPECT_TRUE(libraries.empty());
+}
 
-    // Load a single library and repeat the parse.
-    vector<string> basic_library;
-    basic_library.push_back(string(CALLOUT_LIBRARY_1));
-    HooksManager::loadLibraries(basic_library);
+// hooks-libraries element that contains a single libaray.
+TEST_F(ParseConfigTest, oneHooksLibrary) {
 
-    rcode = parseConfiguration(config);
+    // Configuration with hooks-libraries with a single library.
+    const string config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1);
+
+    // Verify that the configuration string parses.
+    const int rcode = parseConfiguration(config);
     ASSERT_TRUE(rcode == 0) << error_text_;
 
-    // This time the change should have been recorded.
+    // Check that the parser recorded a single library.
+    std::vector<std::string> libraries;
+    bool changed;
     hooks_libraries_parser_->getLibraries(libraries, changed);
-    EXPECT_TRUE(libraries.empty());
     EXPECT_TRUE(changed);
+    EXPECT_EQ(1, libraries.size());
+    EXPECT_EQ(CALLOUT_LIBRARY_1, libraries[0]);
+}
 
-    // But repeating it again and we are back to no change.
-    rcode = parseConfiguration(config);
+// hooks-libraries element that contains two libraries
+TEST_F(ParseConfigTest, twoHooksLibraries) {
+
+    // Configuration with hooks-libraries not present.
+    const string config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1,
+                                                  CALLOUT_LIBRARY_2);
+
+    // Verify that the configuration string parses.
+    const int rcode = parseConfiguration(config);
     ASSERT_TRUE(rcode == 0) << error_text_;
-    hooks_libraries_parser_->getLibraries(libraries, changed);
-    EXPECT_TRUE(libraries.empty());
-    EXPECT_FALSE(changed);
 
+    // Check that the parser recorded two libraries in the expected order.
+    std::vector<std::string> libraries;
+    bool changed;
+    hooks_libraries_parser_->getLibraries(libraries, changed);
+    EXPECT_TRUE(changed);
+    EXPECT_EQ(2, libraries.size());
+    EXPECT_EQ(CALLOUT_LIBRARY_1, libraries[0]);
+    EXPECT_EQ(CALLOUT_LIBRARY_2, libraries[1]);
 }
 
+// Configure with two libraries, then reconfigure with the same libraries.
+TEST_F(ParseConfigTest, reconfigureSameHooksLibraries) {
 
-TEST_F(ParseConfigTest, validHooksLibrariesTest) {
+    const std::string config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1,
+                                                       CALLOUT_LIBRARY_2);
 
-    // Configuration string.  This contains a set of valid libraries.
-    const std::string quote("\"");
-    const std::string comma(", ");
+    // Verify that the configuration string parses. The twoHooksLibraries
+    // test shows that the list will be as expected.
+    int rcode = parseConfiguration(config);
+    ASSERT_TRUE(rcode == 0) << error_text_;
 
-    const std::string config =
-        std::string("{ ") +
-            std::string("\"hooks-libraries\": [") +
-                quote + std::string(CALLOUT_LIBRARY_1) + quote + comma +
-                quote + std::string(CALLOUT_LIBRARY_2)  + quote +
-            std::string("]") +
-        std::string("}");
+    // Parse the string again.
+    rcode = parseConfiguration(config);
+    ASSERT_TRUE(rcode == 0) << error_text_;
 
-    // Verify that the configuration string parses.
+    // The list has not changed, and this is what we should see.
+    std::vector<std::string> libraries;
+    bool changed;
+    hooks_libraries_parser_->getLibraries(libraries, changed);
+    EXPECT_FALSE(changed);
+    EXPECT_EQ(2, libraries.size());
+    EXPECT_EQ(CALLOUT_LIBRARY_1, libraries[0]);
+    EXPECT_EQ(CALLOUT_LIBRARY_2, libraries[1]);
+}
+
+// Configure the hooks with two libraries, then reconfigure with
+// the same libraries, but in reverse order.
+TEST_F(ParseConfigTest, reconfigureReverseHooksLibraries) {
+
+    std::string config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1,
+                                                 CALLOUT_LIBRARY_2);
+
+    // Verify that the configuration string parses. The twoHooksLibraries
+    // test shows that the list will be as expected.
     int rcode = parseConfiguration(config);
     ASSERT_TRUE(rcode == 0) << error_text_;
 
-    // Check that the parser holds two libraries and the configuration is
-    // recorded as having changed.
+    // Parse the reversed set of libraries.
+    config = setHooksLibrariesConfig(CALLOUT_LIBRARY_2, CALLOUT_LIBRARY_1);
+    rcode = parseConfiguration(config);
+    ASSERT_TRUE(rcode == 0) << error_text_;
+
+    // The list has changed, and this is what we should see.
     std::vector<std::string> libraries;
     bool changed;
     hooks_libraries_parser_->getLibraries(libraries, changed);
-    EXPECT_EQ(2, libraries.size());
     EXPECT_TRUE(changed);
+    EXPECT_EQ(2, libraries.size());
+    EXPECT_EQ(CALLOUT_LIBRARY_2, libraries[0]);
+    EXPECT_EQ(CALLOUT_LIBRARY_1, libraries[1]);
+}
 
-    // The expected libraries should be the list of libraries specified
-    // in the given order.
-    std::vector<std::string> expected;
-    expected.push_back(CALLOUT_LIBRARY_1);
-    expected.push_back(CALLOUT_LIBRARY_2);
-    EXPECT_TRUE(expected == libraries);
+// Configure the hooks with two libraries, then reconfigure with
+// no libraries.
+TEST_F(ParseConfigTest, reconfigureZeroHooksLibraries) {
 
-    // Parse the string again.
+    std::string config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1,
+                                                 CALLOUT_LIBRARY_2);
+
+    // Verify that the configuration string parses.
+    int rcode = parseConfiguration(config);
+    ASSERT_TRUE(rcode == 0) << error_text_;
+
+    // Parse the string again, this time without any libraries.
+    config = setHooksLibrariesConfig();
     rcode = parseConfiguration(config);
     ASSERT_TRUE(rcode == 0) << error_text_;
 
-    // The list has not changed, and this is what we should see.
+    // The list has changed, and this is what we should see.
+    std::vector<std::string> libraries;
+    bool changed;
     hooks_libraries_parser_->getLibraries(libraries, changed);
-    EXPECT_EQ(2, libraries.size());
-    EXPECT_FALSE(changed);
+    EXPECT_TRUE(changed);
+    EXPECT_TRUE(libraries.empty());
 }
 
 // Check with a set of libraries, some of which are invalid.
-TEST_F(ParseConfigTest, invalidHooksLibrariesTest) {
-
-    /// @todo Initialize global library context to null
+TEST_F(ParseConfigTest, invalidHooksLibraries) {
 
     // Configuration string.  This contains an invalid library which should
     // trigger an error in the "build" stage.
-    const std::string quote("\"");
-    const std::string comma(", ");
-
-    const std::string config =
-        std::string("{ ") +
-            std::string("\"hooks-libraries\": [") +
-                quote + std::string(CALLOUT_LIBRARY_1) + quote + comma +
-                quote + std::string(NOT_PRESENT_LIBRARY) + quote + comma +
-                quote + std::string(CALLOUT_LIBRARY_2)  + quote +
-            std::string("]") +
-        std::string("}");
+    const std::string config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1,
+                                                       NOT_PRESENT_LIBRARY,
+                                                       CALLOUT_LIBRARY_2);
 
     // Verify that the configuration fails to parse. (Syntactically it's OK,
     // but the library is invalid).
-    int rcode = parseConfiguration(config);
+    const int rcode = parseConfiguration(config);
     ASSERT_FALSE(rcode == 0) << error_text_;
 
     // Check that the message contains the library in error.
@@ -938,6 +1004,29 @@ TEST_F(ParseConfigTest, invalidHooksLibrariesTest) {
         "Error text returned from parse failure is " << error_text_;
 }
 
+// Check that trying to reconfigure with an invalid set of libraries fails.
+TEST_F(ParseConfigTest, reconfigureInvalidHooksLibraries) {
+
+    // Configure with a single library.
+    std::string config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1);
+    int rcode = parseConfiguration(config);
+    ASSERT_TRUE(rcode == 0) << error_text_;
+
+    // Configuration string.  This contains an invalid library which should
+    // trigger an error in the "build" stage.
+    config = setHooksLibrariesConfig(CALLOUT_LIBRARY_1, NOT_PRESENT_LIBRARY,
+                                     CALLOUT_LIBRARY_2);
+
+    // Verify that the configuration fails to parse. (Syntactically it's OK,
+    // but the library is invalid).
+    rcode = parseConfiguration(config);
+    EXPECT_FALSE(rcode == 0) << error_text_;
+
+    // Check that the message contains the library in error.
+    EXPECT_FALSE(error_text_.find(NOT_PRESENT_LIBRARY) == string::npos) <<
+        "Error text returned from parse failure is " << error_text_;
+}
+
 /// @brief Checks that a valid, enabled D2 client configuration works correctly.
 TEST_F(ParseConfigTest, validD2Config) {