[2980] Rationalised the tests a but by extracting a common class

src/lib/hooks/tests/Makefile.am

@@ -71,6 +71,7 @@ TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
 run_unittests_SOURCES += callout_handle_unittest.cc
 run_unittests_SOURCES += callout_manager_unittest.cc
+run_unittests_SOURCES += common_test_class.h
 run_unittests_SOURCES += handles_unittest.cc
 run_unittests_SOURCES += library_manager_collection_unittest.cc
 run_unittests_SOURCES += library_manager_unittest.cc

src/lib/hooks/tests/common_test_class.h

@@ -0,0 +1,138 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef COMMON_HOOKS_TEST_CLASS_H
+#define COMMON_HOOKS_TEST_CLASS_H
+
+#include <hooks/callout_handle.h>
+#include <hooks/callout_manager.h>
+#include <hooks/server_hooks.h>
+#include <hooks/tests/marker_file.h>
+
+#include <boost/shared_ptr.hpp>
+#include <gtest/gtest.h>
+
+/// @brief Common hooks test class
+///
+/// This class is a shared parent of the test fixture class in the tests of the
+/// higher-level hooks classes (LibraryManager, LibraryManagerCollection and
+/// HooksManager).  It
+///
+/// - sets the the ServerHooks object with three hooks and stores their
+///   indexes.
+/// - executes the callouts (which are assumed to perform a calculation)
+///   and checks the results.
+
+class HooksCommonTestClass {
+public:
+    /// @brief Constructor
+    HooksCommonTestClass() {
+
+        // Set up the server hooks.  ServerHooks is a singleton, so we reset it
+        // between each test.
+        isc::hooks::ServerHooks& hooks =
+            isc::hooks::ServerHooks::getServerHooks();
+        hooks.reset();
+        lm_one_index_ = hooks.registerHook("lm_one");
+        lm_two_index_ = hooks.registerHook("lm_two");
+        lm_three_index_ = hooks.registerHook("lm_three");
+    }
+
+    /// @brief Call callouts test
+    ///
+    /// All of the loaded libraries for which callouts are called register four
+    /// callouts: a context_create callout and three callouts that are attached
+    /// to hooks lm_one, lm_two and lm_three.  These four callouts, executed
+    /// in sequence, perform a series of calculations. Data is passed between
+    /// callouts in the argument list, in a variable named "result".
+    ///
+    /// context_create initializes the calculation by setting a seed
+    /// value, called r0 here.  This value is dependent on the library being
+    /// loaded.  Prior to that, the argument "result" is initialized to -1,
+    /// the purpose being to avoid exceptions when running this test with no
+    /// libraries loaded.
+    ///
+    /// Callout lm_one is passed a value d1 and performs a simple arithmetic
+    /// operation on it and r0 yielding a result r1.  Hence we can say that
+    /// @f[ r1 = lm1(r0, d1) @f]
+    ///
+    /// Callout lm_two is passed a value d2 and peforms another simple
+    /// arithmetic operation on it and d2, yielding r2, i.e.
+    /// @f[ r2 = lm2(d1, d2) @f]
+    ///
+    /// lm_three does a similar operation giving @f[ r3 = lm3(r2, d3) @f].
+    ///
+    /// The details of the operations lm1, lm2 and lm3 depend on the library.
+    /// However the sequence of calls needed to do this set of calculations
+    /// is identical regardless of the exact functions. This method performs
+    /// those operations and checks the results of each step.
+    ///
+    /// It is assumed that callout_manager_ has been set up appropriately.
+    ///
+    /// @note The CalloutHandle used in the calls is declared locally here.
+    ///       The advantage of this (apart from scope reduction) is that on
+    ///       exit, it is destroyed.  This removes any references to memory
+    ///       allocated by loaded libraries while they are still loaded.
+    ///
+    /// @param manager CalloutManager to use for the test
+    /// @param r0...r3, d1..d3 Values and intermediate values expected.  They
+    ///        are ordered so that the variables appear in the argument list in
+    ///        the order they are used.
+    void executeCallCallouts(
+            const boost::shared_ptr<isc::hooks::CalloutManager>& manager,
+            int r0, int d1, int r1, int d2, int r2, int d3, int r3) {
+        static const char* COMMON_TEXT = " callout returned the wong value";
+        static const char* RESULT = "result";
+
+        int result;
+
+        // Set up a callout handle for the calls.
+        isc::hooks::CalloutHandle handle(manager);
+
+        // Initialize the argument RESULT.  This simplifies testing by
+        // eliminating the generation of an exception when we try the unload
+        // test.  In that case, RESULT is unchanged.
+        handle.setArgument(RESULT, -1);
+
+        // Seed the calculation.
+        manager->callCallouts(isc::hooks::ServerHooks::CONTEXT_CREATE, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r0, result) << "context_create" << COMMON_TEXT;
+
+        // Perform the first calculation.
+        handle.setArgument("data_1", d1);
+        manager->callCallouts(lm_one_index_, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r1, result) << "lm_one" << COMMON_TEXT;
+
+        // ... the second ...
+        handle.setArgument("data_2", d2);
+        manager->callCallouts(lm_two_index_, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r2, result) << "lm_two" << COMMON_TEXT;
+
+        // ... and the third.
+        handle.setArgument("data_3", d3);
+        manager->callCallouts(lm_three_index_, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r3, result) << "lm_three" << COMMON_TEXT;
+    }
+
+    /// Hook indexes.  These are are made public for ease of reference.
+    int lm_one_index_;
+    int lm_two_index_;
+    int lm_three_index_;
+};
+
+#endif // COMMON_HOOKS_TEST_CLASS_H

src/lib/hooks/tests/library_manager_collection_unittest.cc

@@ -18,6 +18,7 @@
 #include <hooks/library_manager_collection.h>
 #include <hooks/server_hooks.h>
 
+#include <hooks/tests/common_test_class.h>
 #include <hooks/tests/marker_file.h>
 #include <hooks/tests/test_libraries.h>
 
@@ -25,11 +26,8 @@
 #include <gtest/gtest.h>
 
 #include <algorithm>
-#include <fstream>
 #include <string>
 
-#include <unistd.h>
-
 
 using namespace isc;
 using namespace isc::hooks;
@@ -39,116 +37,8 @@ namespace {
 
 /// @brief Library manager collection test class
 
-class LibraryManagerCollectionTest : public ::testing::Test {
-public:
-    /// @brief Constructor
-    LibraryManagerCollectionTest() {
-
-        // Set up the server hooks.  ServerHooks is a singleton, so we reset it
-        // between each test.
-        ServerHooks& hooks = ServerHooks::getServerHooks();
-        hooks.reset();
-        lm_one_index_ = hooks.registerHook("lm_one");
-        lm_two_index_ = hooks.registerHook("lm_two");
-        lm_three_index_ = hooks.registerHook("lm_three");
-
-        // Ensure the marker file is not present at the start of a test.
-        static_cast<void>(unlink(MARKER_FILE));
-    }
-
-    /// @brief Destructor
-    ///
-    /// Ensures a marker file is removed after each test.
-    ~LibraryManagerCollectionTest() {
-        static_cast<void>(unlink(MARKER_FILE));
-    }
-
-    /// @brief Call callouts test
-    ///
-    /// All of the loaded libraries for which callouts are called register four
-    /// callouts: a context_create callout and three callouts that are attached
-    /// to hooks lm_one, lm_two and lm_three.  These four callouts, executed
-    /// in sequence, perform a series of calculations. Data is passed between
-    /// callouts in the argument list, in a variable named "result".
-    ///
-    /// context_create initializes the calculation by setting a seed
-    /// value, called r0 here.  This value is dependent on the library being
-    /// loaded.  Prior to that, the argument "result" is initialized to -1,
-    /// the purpose being to avoid exceptions when running this test with no
-    /// libraries loaded.
-    ///
-    /// Callout lm_one is passed a value d1 and performs a simple arithmetic
-    /// operation on it and r0 yielding a result r1.  Hence we can say that
-    /// @f[ r1 = lm1(r0, d1) @f]
-    ///
-    /// Callout lm_two is passed a value d2 and peforms another simple
-    /// arithmetic operation on it and d2, yielding r2, i.e.
-    /// @f[ r2 = lm2(d1, d2) @f]
-    ///
-    /// lm_three does a similar operation giving @f[ r3 = lm3(r2, d3) @f].
-    ///
-    /// The details of the operations lm1, lm2 and lm3 depend on the library.
-    /// However the sequence of calls needed to do this set of calculations
-    /// is identical regardless of the exact functions. This method performs
-    /// those operations and checks the results of each step.
-    ///
-    /// It is assumed that callout_manager_ has been set up appropriately.
-    ///
-    /// @note The CalloutHandle used in the calls is declared locally here.
-    ///       The advantage of this (apart from scope reduction) is that on
-    ///       exit, it is destroyed.  This removes any references to memory
-    ///       allocated by loaded libraries while they are still loaded.
-    ///
-    /// @param r0...r3, d1..d3 Values and intermediate values expected.  They
-    ///        are ordered so that the variables appear in the argument list in
-    ///        the order they are used.
-    void executeCallCallouts(int r0, int d1, int r1, int d2, int r2, int d3,
-                             int r3) {
-        static const char* COMMON_TEXT = " callout returned the wong value";
-        static const char* RESULT = "result";
-
-        int result;
-
-        // Set up a callout handle for the calls.
-        CalloutHandle callout_handle(callout_manager_);
-
-        // Initialize the argument RESULT.  This simplifies testing by
-        // eliminating the generation of an exception when we try the unload
-        // test.  In that case, RESULT is unchanged.
-        callout_handle.setArgument(RESULT, -1);
-
-        // Seed the calculation.
-        callout_manager_->callCallouts(ServerHooks::CONTEXT_CREATE,
-                                       callout_handle);
-        callout_handle.getArgument(RESULT, result);
-        EXPECT_EQ(r0, result) << "context_create" << COMMON_TEXT;
-
-        // Perform the first calculation.
-        callout_handle.setArgument("data_1", d1);
-        callout_manager_->callCallouts(lm_one_index_, callout_handle);
-        callout_handle.getArgument(RESULT, result);
-        EXPECT_EQ(r1, result) << "lm_one" << COMMON_TEXT;
-
-        // ... the second ...
-        callout_handle.setArgument("data_2", d2);
-        callout_manager_->callCallouts(lm_two_index_, callout_handle);
-        callout_handle.getArgument(RESULT, result);
-        EXPECT_EQ(r2, result) << "lm_two" << COMMON_TEXT;
-
-        // ... and the third.
-        callout_handle.setArgument("data_3", d3);
-        callout_manager_->callCallouts(lm_three_index_, callout_handle);
-        callout_handle.getArgument(RESULT, result);
-        EXPECT_EQ(r3, result) << "lm_three" << COMMON_TEXT;
-    }
-
-    /// Hook indexes.  These are are made public for ease of reference.
-    int lm_one_index_;
-    int lm_two_index_;
-    int lm_three_index_;
-
-    /// Callout manager used in the executeCallCallouts() call.
-    boost::shared_ptr<CalloutManager> callout_manager_;
+class LibraryManagerCollectionTest : public ::testing::Test,
+                                     public HooksCommonTestClass {
 };
 
 /// @brief Public library manager collection class
@@ -188,7 +78,8 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
 
     // Load the libraries.
     EXPECT_TRUE(lm_collection.loadLibraries());
-    callout_manager_ = lm_collection.getCalloutManager();
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
 
     // Execute the callouts.  The first library implements the calculation.
     //
@@ -204,7 +95,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
     // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
     {
         SCOPED_TRACE("Doing calculation with libraries loaded");
-        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
+        executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
     }
 
     // Try unloading the libraries.
@@ -214,7 +105,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
     // happens, the result should always be -1.
     {
         SCOPED_TRACE("Doing calculation with libraries not loaded");
-        executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
+        executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
     }
 }
 
@@ -237,10 +128,11 @@ TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
     // Load the libraries.  We expect a failure status to be returned as
     // one of the libraries failed to load.
     EXPECT_FALSE(lm_collection.loadLibraries());
-    callout_manager_ = lm_collection.getCalloutManager();
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
 
     // Expect only two libraries were loaded.
-    EXPECT_EQ(2, callout_manager_->getNumLibraries());
+    EXPECT_EQ(2, manager->getNumLibraries());
 
     // Execute the callouts.  The first library implements the calculation.
     //
@@ -256,7 +148,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
     // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
     {
         SCOPED_TRACE("Doing calculation with libraries loaded");
-        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
+        executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
     }
 
     // Try unloading the libraries.
@@ -266,7 +158,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
     // happens, the result should always be -1.
     {
         SCOPED_TRACE("Doing calculation with libraries not loaded");
-        executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
+        executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
     }
 }
 
@@ -280,14 +172,15 @@ TEST_F(LibraryManagerCollectionTest, NoLibrariesLoaded) {
     // be using.
     LibraryManagerCollection lm_collection(library_names);
     EXPECT_TRUE(lm_collection.loadLibraries());
-    callout_manager_ = lm_collection.getCalloutManager();
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
 
     // Load the libraries.
     EXPECT_TRUE(lm_collection.loadLibraries());
 
     // Eecute the calculation - callouts can be called but as nothing
     // happens, the result should always be -1.
-    executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
+    executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
 }
 
 } // Anonymous namespace

src/lib/hooks/tests/library_manager_unittest.cc

@@ -17,6 +17,7 @@
 #include <hooks/library_manager.h>
 #include <hooks/server_hooks.h>
 
+#include <hooks/tests/common_test_class.h>
 #include <hooks/tests/marker_file.h>
 #include <hooks/tests/test_libraries.h>
 
@@ -37,23 +38,15 @@ namespace {
 
 /// @brief Library manager test class
 
-class LibraryManagerTest : public ::testing::Test {
+class LibraryManagerTest : public ::testing::Test,
+                           public HooksCommonTestClass {
 public:
     /// @brief Constructor
     ///
-    /// Sets up a collection of three LibraryHandle objects to use in the test.
+    /// Initializes the CalloutManager object used in the tests.  It sets it
+    /// up with the hooks initialized in the HooksCommonTestClass object and
+    /// with four libraries.
     LibraryManagerTest() {
-
-        // Set up the server hooks.  ServerHooks is a singleton, so we reset it
-        // between each test.
-        ServerHooks& hooks = ServerHooks::getServerHooks();
-        hooks.reset();
-        lm_one_index_ = hooks.registerHook("lm_one");
-        lm_two_index_ = hooks.registerHook("lm_two");
-        lm_three_index_ = hooks.registerHook("lm_three");
-
-        // Set up the callout manager with these hooks.  Assume a maximum of
-        // four libraries.
         callout_manager_.reset(new CalloutManager(4));
 
         // Ensure the marker file is not present at the start of a test.
@@ -67,80 +60,42 @@ public:
         static_cast<void>(unlink(MARKER_FILE));
     }
 
-    /// @brief Call callouts test
-    ///
-    /// All of the loaded libraries for which callouts are called register four
-    /// callouts: a context_create callout and three callouts that are attached
-    /// to hooks lm_one, lm_two and lm_three.  These four callouts, executed
-    /// in sequence, perform a series of calculations. Data is passed between
-    /// callouts in the argument list, in a variable named "result".
-    ///
-    /// context_create initializes the calculation by setting a seed
-    /// value, called r0 here.
+    /// @brief Marker file present
     ///
-    /// Callout lm_one is passed a value d1 and performs a simple arithmetic
-    /// operation on it and r0 yielding a result r1.  Hence we can say that
-    /// @f[ r1 = lm1(r0, d1) @f]
+    /// Convenience function to check whether a marker file is present.  It
+    /// does this by opening the file.
     ///
-    /// Callout lm_two is passed a value d2 and peforms another simple
-    /// arithmetic operation on it and d2, yielding r2, i.e.
-    /// @f[ r2 = lm2(d1, d2) @f]
-    ///
-    /// lm_three does a similar operation giving @f[ r3 = lm3(r2, d3) @f].
-    ///
-    /// The details of the operations lm1, lm2 and lm3 depend on the library.
-    /// However the sequence of calls needed to do this set of calculations
-    /// is identical regardless of the exact functions. This method performs
-    /// those operations and checks the results of each step.
-    ///
-    /// It is assumed that callout_manager_ has been set up appropriately.
+    /// @return true if the marker file is present.
+    bool markerFilePresent() const {
+
+        // Try to open it.
+        std::fstream marker;
+        marker.open(MARKER_FILE, std::fstream::in);
+
+        // Check if it is open and close it if so.
+        bool exists = marker.is_open();
+        if (exists) {
+            marker.close();
+        }
+
+        return (exists);
+    }
+
+    /// @brief Call callouts test
     ///
-    /// @note The CalloutHandle used in the calls is declared locally here.
-    ///       The advantage of this (apart from scope reduction) is that on
-    ///       exit, it is destroyed.  This removes any references to memory
-    ///       allocated by loaded libraries while they are still loaded.
+    /// A wrapper around the method of the same name in the HooksCommonTestClass
+    /// object, this passes this class's CalloutManager to that method.
     ///
     /// @param r0...r3, d1..d3 Values and intermediate values expected.  They
     ///        are ordered so that the variables appear in the argument list in
-    ///        the order they are used.
+    ///        the order they are used.  See HooksCommonTestClass::execute for
+    ///        a full description.
     void executeCallCallouts(int r0, int d1, int r1, int d2, int r2, int d3,
                              int r3) {
-        static const char* COMMON_TEXT = " callout returned the wong value";
-        int result;
-
-        // Set up a callout handle for the calls.
-        CalloutHandle callout_handle(callout_manager_);
-
-        // Seed the calculation.
-        callout_manager_->callCallouts(ServerHooks::CONTEXT_CREATE,
-                                       callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r0, result) << "context_create" << COMMON_TEXT;
-
-        // Perform the first calculation.
-        callout_handle.setArgument("data_1", d1);
-        callout_manager_->callCallouts(lm_one_index_, callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r1, result) << "lm_one" << COMMON_TEXT;
-
-        // ... the second ...
-        callout_handle.setArgument("data_2", d2);
-        callout_manager_->callCallouts(lm_two_index_, callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r2, result) << "lm_two" << COMMON_TEXT;
-
-        // ... and the third.
-        callout_handle.setArgument("data_3", d3);
-        callout_manager_->callCallouts(lm_three_index_, callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r3, result) << "lm_three" << COMMON_TEXT;
+        HooksCommonTestClass::executeCallCallouts(callout_manager_, r0, d1,
+                                                  r1, d2, r2, d3, r3);
     }
 
-    /// Hook indexes.  These are are made public for ease of reference.
-    int lm_one_index_;
-    int lm_two_index_;
-    int lm_three_index_;
-
     /// Callout manager used for the test.
     boost::shared_ptr<CalloutManager> callout_manager_;
 };
@@ -380,21 +335,16 @@ TEST_F(LibraryManagerTest, CheckUnload) {
                                      0, callout_manager_);
     EXPECT_TRUE(lib_manager.openLibrary());
 
+
     // Check that the marker file is not present (at least that the file
     // open fails).
-    fstream marker;
-    marker.open(MARKER_FILE, fstream::in);
-    EXPECT_TRUE(marker.fail());
+    EXPECT_FALSE(markerFilePresent());
 
     // Check that unload function runs and returns a success
     EXPECT_TRUE(lib_manager.runUnload());
 
-    // Check that the open succeeded
-    marker.open(MARKER_FILE, fstream::in);
-    EXPECT_TRUE(marker.is_open());
-
-    // Tidy up
-    marker.close();
+    // Check that the marker file was created.
+    EXPECT_TRUE(markerFilePresent());
 
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());