From: Thomas Markwalder Date: Mon, 9 Apr 2018 18:42:42 +0000 (-0400) Subject: [5556a] Addressed review comments X-Git-Tag: trac5458a_base~17^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=76ddd1ffdd5ee858b3dfdad3e58fed8e3e105b26;p=thirdparty%2Fkea.git [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 c60fb5e1ba..f0a9f0b306 100644 --- 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 5bbe6fe8a5..cd62825eb3 100644 --- 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 6f6c95fc0d..fccec64651 100644 --- 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 04714a05ea..0d7463280d 100644 --- 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