[5556a] Addressed review comments

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
    LeaseMgrDbLostCallbackTest::testNoCallbackOnOpenFailure() {
    - removed before hand checks for socket descriptor, grab
    the most recently opend descriptor after connecting to the db
    and assume its the SQL client's

    - test return value of close() rather then errno

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h
    LeaseMgrDbLostCallbackTest
    - constructor and destructor clear
    DatabaseConnection::db_lost_callback

src/lib/dhcpsrv/tests/host_mgr_unittest.cc
    HostMgrDbLostCallbackTest
    - Setup and Teardown methods clear DatabaseConnection::db_lost_callback

    HostMgrDbLostCallbackTest::testDbLostCallback()
    - removed before hand checks for socket descriptor, grab
    the most recently opend descriptor after connecting to the db
    and assume its the SQL client's

    - test return value of close() rather then errno

src/lib/dhcpsrv/tests/test_utils.cc
    int findLastSocketFd()
    - iterate over all descriptors rather than stopping at first invalid

diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
index c60fb5e1baec454fe28f066e655ebe6259971236..f0a9f0b306bf79c34e33de14980aa7cd35ec9031 100644 (file)
--- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
@@ -2828,19 +2828,18 @@ LeaseMgrDbLostCallbackTest::testNoCallbackOnOpenFailure() {
 
 void
 LeaseMgrDbLostCallbackTest::testDbLostCallback() {
-    // We should not find a socket open
-    int sql_socket = test::findLastSocketFd();
-    ASSERT_EQ(-1, sql_socket);
-
+    // Set the connectivity lost callback.
     DatabaseConnection::db_lost_callback =
         boost::bind(&LeaseMgrDbLostCallbackTest::db_lost_callback, this, _1);
 
+    // Connect to the lease backend.
     ASSERT_NO_THROW(LeaseMgrFactory::create(validConnectString()));
 
-    // We should find a socket open and it should be for MySQL.
-    sql_socket = test::findLastSocketFd();
-    ASSERT_TRUE(sql_socket > 0);
+    // The most recently opened socket should be for our SQL client.
+    int sql_socket = test::findLastSocketFd();
+    ASSERT_TRUE(sql_socket > -1);
 
+    // Clear the callback invocation marker.
     callback_called_ = false;
 
     // Verify we can execute a query.
@@ -2849,10 +2848,7 @@ LeaseMgrDbLostCallbackTest::testDbLostCallback() {
     ASSERT_NO_THROW(version = lm.getVersion());
 
     // Now close the sql socket out from under backend client
-    errno = 0;
-
-    close(sql_socket);
-    ASSERT_EQ(0, errno) << "failed to close socket";
+    ASSERT_EQ(0, close(sql_socket));
 
     // A query should fail with DbOperationError.
     ASSERT_THROW(version = lm.getVersion(), DbOperationError);

diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h
index 5bbe6fe8a5fc7a6b340ef8c9a4fb2a28af151606..cd62825eb34e9aa5d5fbe9d8eaf9123bacd878c4 100644 (file)
--- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h
+++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h
@@ -415,6 +415,14 @@ public:
 
 class LeaseMgrDbLostCallbackTest : public ::testing::Test {
 public:
+    LeaseMgrDbLostCallbackTest() {
+        DatabaseConnection::db_lost_callback = 0;
+    }
+
+    virtual ~LeaseMgrDbLostCallbackTest() {
+        DatabaseConnection::db_lost_callback = 0;
+    }
+
     /// @brief Prepares the class for a test.
     ///
     /// Invoked by gtest prior test entry, we create the

diff --git a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
index 6f6c95fc0db726ffc6e9c4a9937019c42cf14e93..fccec646517abfecdc6af7fa3ae8f36fd8a9cfd5 100644 (file)
--- a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
@@ -548,6 +548,7 @@ public:
     /// appropriate schema and create a basic host manager to
     /// wipe out any prior instance
     virtual void SetUp() {
+        DatabaseConnection::db_lost_callback = 0;  
         destroySchema();
         createSchema();
         // Wipe out any pre-existing mgr
@@ -559,6 +560,7 @@ public:
     /// Invoked by gtest upon test exit, we destroy the schema
     /// we created.
     virtual void TearDown() {
+        DatabaseConnection::db_lost_callback = 0;  
         destroySchema();
     }
 
@@ -596,22 +598,25 @@ public:
 
 void
 HostMgrDbLostCallbackTest::testDbLostCallback() {
-
-    // We should not find a socket open
-    int sql_socket = test::findLastSocketFd();
-    ASSERT_EQ(-1, sql_socket);
-
+    // Create the HostMgr.
     HostMgr::create();
 
+    // Set the connectivity lost callback.
     DatabaseConnection::db_lost_callback =
         boost::bind(&HostMgrDbLostCallbackTest::db_lost_callback, this, _1);
 
+    // Find the most recently opened socket. Our SQL client's socket should
+    // be the next one.
+    int last_open_socket = test::findLastSocketFd();
+
+    // Connect to the host backend.
     ASSERT_NO_THROW(HostMgr::addBackend(validConnectString()));
 
-    // We should find a socket open and it should be for MySQL.
-    sql_socket = test::findLastSocketFd();
-    ASSERT_TRUE(sql_socket > 0);
+    // Find the SQL client socket.
+    int sql_socket = test::findLastSocketFd();
+    ASSERT_TRUE(sql_socket > last_open_socket);
 
+    // Clear the callback invocation marker.
     callback_called_ = false;
 
     // Verify we can execute a query.  We don't care about the answer.
@@ -619,17 +624,14 @@ HostMgrDbLostCallbackTest::testDbLostCallback() {
     ASSERT_NO_THROW(hosts = HostMgr::instance().getAll4(IOAddress("192.0.2.5")));
 
     // Now close the sql socket out from under backend client
-    errno = 0;
-
-    close(sql_socket);
-    ASSERT_EQ(0, errno) << "failed to close socket";
+    ASSERT_FALSE(close(sql_socket)) << "failed to close socket";
 
     // A query should fail with DbOperationError.
     ASSERT_THROW(hosts = HostMgr::instance().getAll4(IOAddress("192.0.2.5")),
                  DbOperationError);
 
     // Our lost connectivity callback should have been invoked.
-    ASSERT_EQ(true, callback_called_);
+    EXPECT_TRUE(callback_called_);
 }
 
 // The following tests require MySQL enabled.

diff --git a/src/lib/dhcpsrv/tests/test_utils.cc b/src/lib/dhcpsrv/tests/test_utils.cc
index 04714a05ea545e0c45f9f7c52cfb528efa195f9c..0d7463280d59e5dd4c89a38986c1528edc6fc341 100644 (file)
--- a/src/lib/dhcpsrv/tests/test_utils.cc
+++ b/src/lib/dhcpsrv/tests/test_utils.cc
@@ -90,7 +90,8 @@ int findLastSocketFd() {
         fstat(fd, &stats);
 
         if (errno == EBADF ) {
-            break;
+            // Skip any that aren't open
+            continue;
         }
 
         // it's a socket, remember it