BIND 10 trac805, updated. 9d4ae52d559448299b5561bd0d76930c2be2275b [805] Comment updates
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 4 14:55:45 UTC 2012
The branch, trac805 has been updated
via 9d4ae52d559448299b5561bd0d76930c2be2275b (commit)
via 908c1b114ba233fb2c238c7e102ddc0fdb605c9e (commit)
via 5565505e948f85534896a03e7a083c30496638e7 (commit)
from 444f92d580471b1ed9474a373a4797d497fdcea4 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 9d4ae52d559448299b5561bd0d76930c2be2275b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Jan 4 15:55:35 2012 +0100
[805] Comment updates
commit 908c1b114ba233fb2c238c7e102ddc0fdb605c9e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Jan 4 15:55:14 2012 +0100
[805] Make the test exception more concrete
commit 5565505e948f85534896a03e7a083c30496638e7
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed Jan 4 15:06:34 2012 +0100
[805] Get rid of multiple inheritance
By having the TestSocketRequestor as member.
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/tests/auth_srv_unittest.cc | 13 ++-
src/bin/auth/tests/config_unittest.cc | 8 +-
src/bin/resolver/tests/resolver_config_unittest.cc | 13 ++-
src/lib/server_common/portconfig.cc | 9 ++-
src/lib/server_common/tests/portconfig_unittest.cc | 63 ++++++++++------
src/lib/testutils/socket_request.h | 81 +++++++------------
src/lib/testutils/srv_test.h | 5 +-
7 files changed, 99 insertions(+), 93 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index d3d9b0a..ec02241 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -65,13 +65,13 @@ const char* const CONFIG_TESTDB =
const char* const BADCONFIG_TESTDB =
"{ \"database_file\": \"" TEST_DATA_DIR "/nodir/notexist\"}";
-class AuthSrvTest : public SrvTestBase, public TestSocketRequestor {
+class AuthSrvTest : public SrvTestBase {
protected:
AuthSrvTest() :
- TestSocketRequestor(dnss_, address_store_, 53210),
dnss_(ios_, NULL, NULL, NULL),
server(true, xfrout),
- rrclass(RRClass::IN())
+ rrclass(RRClass::IN()),
+ sock_requestor_(dnss_, address_store_, 53210)
{
server.setDNSService(dnss_);
server.setXfrinSession(¬ify_session);
@@ -89,6 +89,7 @@ protected:
const RRClass rrclass;
vector<uint8_t> response_data;
AddressList address_store_;
+ TestSocketRequestor sock_requestor_;
};
// A helper function that builds a response to version.bind/TXT/CH that
@@ -899,10 +900,12 @@ TEST_F(AuthSrvTest, listenAddresses) {
"UDP:::1:53210:4",
NULL
};
- checkTokens(tokens, given_tokens_, "Given tokens");
+ sock_requestor_.checkTokens(tokens, sock_requestor_.given_tokens_,
+ "Given tokens");
// It returns back to empty set of addresses afterwards, so
// they should be released
- checkTokens(tokens, released_tokens_, "Released tokens");
+ sock_requestor_.checkTokens(tokens, sock_requestor_.released_tokens_,
+ "Released tokens");
}
}
diff --git a/src/bin/auth/tests/config_unittest.cc b/src/bin/auth/tests/config_unittest.cc
index 90048ac..18092c1 100644
--- a/src/bin/auth/tests/config_unittest.cc
+++ b/src/bin/auth/tests/config_unittest.cc
@@ -40,13 +40,13 @@ using namespace isc::asiodns;
using namespace isc::asiolink;
namespace {
-class AuthConfigTest : public isc::testutils::TestSocketRequestor {
+class AuthConfigTest : public ::testing::Test {
protected:
AuthConfigTest() :
- TestSocketRequestor(dnss_, address_store_, 53210),
dnss_(ios_, NULL, NULL, NULL),
rrclass(RRClass::IN()),
- server(true, xfrout)
+ server(true, xfrout),
+ sock_requestor_(dnss_, address_store_, 53210)
{
server.setDNSService(dnss_);
}
@@ -56,6 +56,8 @@ protected:
MockXfroutClient xfrout;
AuthSrv server;
isc::server_common::portconfig::AddressList address_store_;
+private:
+ isc::testutils::TestSocketRequestor sock_requestor_;
};
TEST_F(AuthConfigTest, datasourceConfig) {
diff --git a/src/bin/resolver/tests/resolver_config_unittest.cc b/src/bin/resolver/tests/resolver_config_unittest.cc
index 2f71c99..87d4826 100644
--- a/src/bin/resolver/tests/resolver_config_unittest.cc
+++ b/src/bin/resolver/tests/resolver_config_unittest.cc
@@ -53,7 +53,7 @@ using namespace isc::server_common;
using isc::UnitTestUtil;
namespace {
-class ResolverConfig : public TestSocketRequestor {
+class ResolverConfig : public ::testing::Test {
protected:
IOService ios;
DNSService dnss;
@@ -63,8 +63,8 @@ protected:
scoped_ptr<const Client> client;
scoped_ptr<const RequestContext> request;
ResolverConfig() :
- TestSocketRequestor(dnss, address_store_, 53210),
- dnss(ios, NULL, NULL, NULL)
+ dnss(ios, NULL, NULL, NULL),
+ sock_requestor_(dnss, address_store_, 53210)
{
server.setDNSService(dnss);
server.setConfigured();
@@ -82,6 +82,7 @@ protected:
}
void invalidTest(const string &JSON, const string& name);
isc::server_common::portconfig::AddressList address_store_;
+ isc::testutils::TestSocketRequestor sock_requestor_;
};
TEST_F(ResolverConfig, forwardAddresses) {
@@ -201,10 +202,12 @@ TEST_F(ResolverConfig, listenAddresses) {
"UDP:::1:53210:4",
NULL
};
- checkTokens(tokens, given_tokens_, "Given tokens");
+ sock_requestor_.checkTokens(tokens, sock_requestor_.given_tokens_,
+ "Given tokens");
// It returns back to empty set of addresses afterwards, so
// they should be released
- checkTokens(tokens, released_tokens_, "Released tokens");
+ sock_requestor_.checkTokens(tokens, sock_requestor_.released_tokens_,
+ "Released tokens");
}
// Try setting some addresses and a rollback
diff --git a/src/lib/server_common/portconfig.cc b/src/lib/server_common/portconfig.cc
index 4201157..2b6e4ec 100644
--- a/src/lib/server_common/portconfig.cc
+++ b/src/lib/server_common/portconfig.cc
@@ -94,6 +94,11 @@ setAddresses(DNSService& service, const AddressList& addresses) {
BOOST_FOREACH(const AddressPair &address, addresses) {
const int af(IOAddress(address.first).getFamily());
// TODO: Support sharing somehow in future.
+
+ // As for now, we hardcode the application name as dummy_app, because:
+ // * we don't have a name available in our interface, which will change
+ // soon anyway
+ // * we use the DONT_SHARE mode, so the name is irrelevant anyway
const SocketRequestor::SocketID
tcp(socketRequestor().requestSocket(SocketRequestor::TCP,
address.first, address.second,
@@ -151,7 +156,9 @@ installListenAddresses(const AddressList& newAddresses,
}
catch (const exception& e2) {
LOG_FATAL(logger, SRVCOMM_ADDRESS_UNRECOVERABLE).arg(e2.what());
- // Releasing them should really work
+ // If we can't set the new ones, nor the old ones, at last
+ // releasing everything should work. If it doesn't, there isn't
+ // anything else we could do.
AddressList empty;
setAddresses(service, empty);
addressStore.clear();
diff --git a/src/lib/server_common/tests/portconfig_unittest.cc b/src/lib/server_common/tests/portconfig_unittest.cc
index 4507e14..1cf3e20 100644
--- a/src/lib/server_common/tests/portconfig_unittest.cc
+++ b/src/lib/server_common/tests/portconfig_unittest.cc
@@ -130,10 +130,10 @@ TEST_F(ParseAddresses, invalid) {
}
// Test fixture for installListenAddresses
-struct InstallListenAddresses : public testutils::TestSocketRequestor {
+struct InstallListenAddresses : public ::testing::Test {
InstallListenAddresses() :
- // The members aren't initialized yet, but we need to store refs only
- TestSocketRequestor(dnss_, store_, 5288), dnss_(ios_, NULL, NULL, NULL)
+ dnss_(ios_, NULL, NULL, NULL),
+ sock_requestor_(dnss_, store_, 5288)
{
valid_.push_back(AddressPair("127.0.0.1", 5288));
valid_.push_back(AddressPair("::1", 5288));
@@ -143,6 +143,7 @@ struct InstallListenAddresses : public testutils::TestSocketRequestor {
IOService ios_;
DNSService dnss_;
AddressList store_;
+ isc::testutils::TestSocketRequestor sock_requestor_;
// We should be able to bind to these addresses
AddressList valid_;
// But this shouldn't work
@@ -177,15 +178,19 @@ TEST_F(InstallListenAddresses, valid) {
NULL
};
const char* no_tokens[] = { NULL };
- checkTokens(tokens1, given_tokens_, "Valid given tokens 1");
- checkTokens(no_tokens, released_tokens_, "Valid no released tokens 1");
+ sock_requestor_.checkTokens(tokens1, sock_requestor_.given_tokens_,
+ "Valid given tokens 1");
+ sock_requestor_.checkTokens(no_tokens, sock_requestor_.released_tokens_,
+ "Valid no released tokens 1");
// TODO Maybe some test to actually connect to them
// Try setting it back to nothing
- given_tokens_.clear();
+ sock_requestor_.given_tokens_.clear();
EXPECT_NO_THROW(installListenAddresses(AddressList(), store_, dnss_));
checkAddresses(AddressList(), "No addresses");
- checkTokens(no_tokens, given_tokens_, "Valid no given tokens");
- checkTokens(tokens1, released_tokens_, "Valid released tokens");
+ sock_requestor_.checkTokens(no_tokens, sock_requestor_.given_tokens_,
+ "Valid no given tokens");
+ sock_requestor_.checkTokens(tokens1, sock_requestor_.released_tokens_,
+ "Valid released tokens");
// Try switching back again
EXPECT_NO_THROW(installListenAddresses(valid_, store_, dnss_));
checkAddresses(valid_, "Valid addresses");
@@ -196,8 +201,10 @@ TEST_F(InstallListenAddresses, valid) {
"UDP:::1:5288:8",
NULL
};
- checkTokens(tokens2, given_tokens_, "Valid given tokens 2");
- checkTokens(tokens1, released_tokens_, "Valid released tokens");
+ sock_requestor_.checkTokens(tokens2, sock_requestor_.given_tokens_,
+ "Valid given tokens 2");
+ sock_requestor_.checkTokens(tokens1, sock_requestor_.released_tokens_,
+ "Valid released tokens");
}
// Try if rollback works
@@ -213,11 +220,14 @@ TEST_F(InstallListenAddresses, rollback) {
NULL
};
const char* no_tokens[] = { NULL };
- checkTokens(tokens1, given_tokens_, "Given before rollback");
- checkTokens(no_tokens, released_tokens_, "Released before rollback");
- given_tokens_.clear();
+ sock_requestor_.checkTokens(tokens1, sock_requestor_.given_tokens_,
+ "Given before rollback");
+ sock_requestor_.checkTokens(no_tokens, sock_requestor_.released_tokens_,
+ "Released before rollback");
+ sock_requestor_.given_tokens_.clear();
// This should not bind them, but should leave the original addresses
- EXPECT_THROW(installListenAddresses(invalid_, store_, dnss_), exception);
+ EXPECT_THROW(installListenAddresses(invalid_, store_, dnss_),
+ SocketRequestor::SocketError);
checkAddresses(valid_, "After rollback");
// Now, it should have requested first pair of sockets from the invalids
// and, as the second failed, it should have returned them right away.
@@ -241,22 +251,25 @@ TEST_F(InstallListenAddresses, rollback) {
"UDP:::1:5288:10",
NULL
};
- checkTokens(tokens2, given_tokens_, "Given after rollback");
- checkTokens(released1, released_tokens_, "Released after rollback");
+ sock_requestor_.checkTokens(tokens2, sock_requestor_.given_tokens_,
+ "Given after rollback");
+ sock_requestor_.checkTokens(released1, sock_requestor_.released_tokens_,
+ "Released after rollback");
}
-// Try it at least returns everything when even the rollback fails.
+// Try it at least releases everything when even the rollback fails.
TEST_F(InstallListenAddresses, brokenRollback) {
EXPECT_NO_THROW(installListenAddresses(valid_, store_, dnss_));
checkAddresses(valid_, "Before rollback");
// Don't check the tokens now, we already do it in rollback and valid tests
- given_tokens_.clear();
- break_rollback_ = true;
- EXPECT_THROW(installListenAddresses(invalid_, store_, dnss_), exception);
+ sock_requestor_.given_tokens_.clear();
+ sock_requestor_.break_rollback_ = true;
+ EXPECT_THROW(installListenAddresses(invalid_, store_, dnss_),
+ SocketRequestor::SocketError);
// No addresses here
EXPECT_TRUE(store_.empty());
- // These should be requested in the first part of the failure to bind
- // and the second pair in the first part of rollback
+ // The first pair should be requested in the first part of the failure to
+ // bind and the second pair in the first part of rollback
const char* tokens[] = {
"TCP:127.0.0.1:5288:5",
"UDP:127.0.0.1:5288:6",
@@ -276,8 +289,10 @@ TEST_F(InstallListenAddresses, brokenRollback) {
"UDP:127.0.0.1:5288:8",
NULL
};
- checkTokens(tokens, given_tokens_, "given");
- checkTokens(released, released_tokens_, "released");
+ sock_requestor_.checkTokens(tokens, sock_requestor_.given_tokens_,
+ "given");
+ sock_requestor_.checkTokens(released, sock_requestor_.released_tokens_,
+ "released");
}
}
diff --git a/src/lib/testutils/socket_request.h b/src/lib/testutils/socket_request.h
index 8a820f0..f6f0069 100644
--- a/src/lib/testutils/socket_request.h
+++ b/src/lib/testutils/socket_request.h
@@ -37,25 +37,18 @@ namespace testutils {
///
/// It's awkward to request real sockets from the real socket creator
/// during tests (for one, because it would have to be running, for
-/// another, we need to block real ports). If you inherit this class,
-/// the socket requestor will be initialized to a test one which handles
-/// fake socket FDs and stores what was requested, etc. Make sure to
-/// call the inherited SetUp and TearDown if you define your own (eg.
-/// chain them instead of override).
+/// another, we need to block real ports). If you instantiate this class in
+/// a test case, the socket requestor will be initialized to a test one which
+/// handles fake socket FDs and stores what was requested, etc.
///
/// Furthermore, you can check if the code requested or released the correct
-/// list of sockets.
+/// list of sockets using the checkTokens() method.
///
-/// \note This class breaks many recommendations about "clean" object
-/// design. It is mostly because the recommendations are too paranoid
-/// and we better use the powerful tools we have, when we have a reason.
-/// We inherit both the testing::Test and SocketRequestor, so we can
-/// access both the testing macros and store the requests conveniently.
-/// The virtual inheritance is necessary, because some tests need more
-/// than one such "component" to be build from.
-class TestSocketRequestor : public isc::server_common::SocketRequestor,
- virtual public ::testing::Test {
-protected:
+/// Some member variables are intentionally made public so that test cases
+/// can easily check the value of them. We prefer convenience for tests over
+/// class integrity here.
+class TestSocketRequestor : public isc::server_common::SocketRequestor {
+public:
/// \brief Constructor
///
/// \param dnss The DNS service. It is expected this gets initialized
@@ -71,10 +64,28 @@ protected:
uint16_t expect_port) :
last_token_(0), break_rollback_(false), dnss_(dnss), store_(store),
expect_port_(expect_port)
- {}
+ {
+ // Prepare the requestor (us) for the test
+ SocketRequestor::initTest(this);
+ // Don't manipulate the real sockets
+ server_common::portconfig::test_mode = true;
+ }
/// \brief Destructor
- virtual ~ TestSocketRequestor() {}
+ ///
+ /// Removes the addresses (if any) installed by installListenAddresses,
+ /// resets the socket requestor to uninitialized state and turns off
+ /// the portconfig test mode.
+ virtual ~TestSocketRequestor() {
+ // Make sure no sockets are left inside (if installListenAddresses
+ // wasn't used, this is NOP, so it won't hurt).
+ server_common::portconfig::AddressList list;
+ server_common::portconfig::installListenAddresses(list, store_, dnss_);
+ // Don't leave invalid pointers here
+ SocketRequestor::initTest(NULL);
+ // And return the mode
+ server_common::portconfig::test_mode = false;
+ }
/// \brief Tokens released by releaseSocket
///
@@ -88,7 +99,7 @@ protected:
private:
// Last token number and fd given out
size_t last_token_;
-protected:
+public:
/// \brief Support a broken rollback case
///
/// If this is set to true, the requestSocket will throw when the
@@ -149,38 +160,6 @@ protected:
return (SocketID(number, token));
}
- /// \brief Initialize the test
- ///
- /// It installs itself as the socket requestor. It also turns on portconfig
- /// test mode where it doesn't really put the sockets into ASIO (as the FDs
- /// are fake ones and would get closed).
- ///
- /// Make sure you call it in case you need to define your own SetUp.
- void SetUp() {
- // Prepare the requestor (us) for the test
- SocketRequestor::initTest(this);
- // Don't manipulate the real sockets
- server_common::portconfig::test_mode = true;
- }
-
- /// \brief Cleanup after the test
- ///
- /// Removes the addresses (if any) installed by installListenAddresses,
- /// resets the socket requestor to uninitialized state and turns off
- /// the portconfig test mode.
- ///
- /// Make sure you call it in case you need to define your own TearDown.
- void TearDown() {
- // Make sure no sockets are left inside (if installListenAddresses
- // wasn't used, this is NOP, so it won't hurt).
- server_common::portconfig::AddressList list;
- server_common::portconfig::installListenAddresses(list, store_, dnss_);
- // Don't leave invalid pointers here
- SocketRequestor::initTest(NULL);
- // And return the mode
- server_common::portconfig::test_mode = false;
- }
-
/// \brief Check the list of tokens is as expected
///
/// Compares the expected and real tokens.
diff --git a/src/lib/testutils/srv_test.h b/src/lib/testutils/srv_test.h
index c262447..630232c 100644
--- a/src/lib/testutils/srv_test.h
+++ b/src/lib/testutils/srv_test.h
@@ -45,10 +45,7 @@ extern const unsigned int AD_FLAG;
extern const unsigned int CD_FLAG;
/// \brief The base class for Auth and Recurse test case
-///
-/// The Test is inherited virtually because some tests are built of more
-/// test-base-components, all of which need to inherit Test.
-class SrvTestBase : virtual public ::testing::Test {
+class SrvTestBase : public ::testing::Test {
protected:
SrvTestBase();
virtual ~SrvTestBase();
More information about the bind10-changes
mailing list