Commit 76ddd1ff authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[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
parent 5cf5d9c9
......@@ -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);
......
......@@ -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
......
......@@ -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.
......
......@@ -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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment