BIND 10 trac805, updated. b42300ae4b958bef9ff55eec929a227c4db4b5e3 [805] Documentation of the fake socket requestor

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Dec 23 14:51:05 UTC 2011


The branch, trac805 has been updated
       via  b42300ae4b958bef9ff55eec929a227c4db4b5e3 (commit)
       via  0524cba1473680ecf3fb6d1f7031622047472f83 (commit)
       via  6d541b2acec75ea2ab8811619f8ccda8c05eec69 (commit)
      from  33a396ecf7eddfa82054f4056d471ce9ff0616e4 (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 b42300ae4b958bef9ff55eec929a227c4db4b5e3
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Fri Dec 23 15:45:05 2011 +0100

    [805] Documentation of the fake socket requestor

commit 0524cba1473680ecf3fb6d1f7031622047472f83
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Fri Dec 23 15:03:10 2011 +0100

    [805] Make the resolver tests pass
    
    The same problem as with the auth ones - the socket requestor needs to
    be faked.

commit 6d541b2acec75ea2ab8811619f8ccda8c05eec69
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Fri Dec 23 14:46:02 2011 +0100

    [805] Make the auth tests pass
    
    We need to fake the SocketRequestor in them. This also asked for little
    update of the general base for socket requesting tests.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/auth/tests/auth_srv_unittest.cc            |   17 +++-
 src/bin/auth/tests/config_unittest.cc              |    5 +-
 src/bin/resolver/tests/resolver_config_unittest.cc |   21 ++++-
 src/lib/server_common/tests/portconfig_unittest.cc |    2 +-
 src/lib/testutils/portconfig.h                     |    5 +-
 src/lib/testutils/socket_request.h                 |  107 ++++++++++++++++++--
 src/lib/testutils/srv_test.h                       |    7 +-
 7 files changed, 145 insertions(+), 19 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index ac25cd6..d3d9b0a 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -41,6 +41,7 @@
 #include <testutils/dnsmessage_test.h>
 #include <testutils/srv_test.h>
 #include <testutils/portconfig.h>
+#include <testutils/socket_request.h>
 
 using namespace std;
 using namespace isc::cc;
@@ -64,9 +65,10 @@ const char* const CONFIG_TESTDB =
 const char* const BADCONFIG_TESTDB =
     "{ \"database_file\": \"" TEST_DATA_DIR "/nodir/notexist\"}";
 
-class AuthSrvTest : public SrvTestBase {
+class AuthSrvTest : public SrvTestBase, public TestSocketRequestor {
 protected:
     AuthSrvTest() :
+        TestSocketRequestor(dnss_, address_store_, 53210),
         dnss_(ios_, NULL, NULL, NULL),
         server(true, xfrout),
         rrclass(RRClass::IN())
@@ -86,6 +88,7 @@ protected:
     AuthSrv server;
     const RRClass rrclass;
     vector<uint8_t> response_data;
+    AddressList address_store_;
 };
 
 // A helper function that builds a response to version.bind/TXT/CH that
@@ -888,6 +891,18 @@ TEST_F(AuthSrvTest, stop) {
 
 TEST_F(AuthSrvTest, listenAddresses) {
     isc::testutils::portconfig::listenAddresses(server);
+    // Check it requests the correct addresses
+    const char* tokens[] = {
+        "TCP:127.0.0.1:53210:1",
+        "UDP:127.0.0.1:53210:2",
+        "TCP:::1:53210:3",
+        "UDP:::1:53210:4",
+        NULL
+    };
+    checkTokens(tokens, given_tokens_, "Given tokens");
+    // It returns back to empty set of addresses afterwards, so
+    // they should be released
+    checkTokens(tokens, released_tokens_, "Released tokens");
 }
 
 }
diff --git a/src/bin/auth/tests/config_unittest.cc b/src/bin/auth/tests/config_unittest.cc
index dadb0ee..90048ac 100644
--- a/src/bin/auth/tests/config_unittest.cc
+++ b/src/bin/auth/tests/config_unittest.cc
@@ -31,6 +31,7 @@
 
 #include <testutils/mockups.h>
 #include <testutils/portconfig.h>
+#include <testutils/socket_request.h>
 
 using namespace isc::dns;
 using namespace isc::data;
@@ -39,9 +40,10 @@ using namespace isc::asiodns;
 using namespace isc::asiolink;
 
 namespace {
-class AuthConfigTest : public ::testing::Test {
+class AuthConfigTest : public isc::testutils::TestSocketRequestor {
 protected:
     AuthConfigTest() :
+        TestSocketRequestor(dnss_, address_store_, 53210),
         dnss_(ios_, NULL, NULL, NULL),
         rrclass(RRClass::IN()),
         server(true, xfrout)
@@ -53,6 +55,7 @@ protected:
     const RRClass rrclass;
     MockXfroutClient xfrout;
     AuthSrv server;
+    isc::server_common::portconfig::AddressList address_store_;
 };
 
 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 c089041..2f71c99 100644
--- a/src/bin/resolver/tests/resolver_config_unittest.cc
+++ b/src/bin/resolver/tests/resolver_config_unittest.cc
@@ -39,6 +39,7 @@
 #include <dns/tests/unittest_util.h>
 #include <testutils/srv_test.h>
 #include <testutils/portconfig.h>
+#include <testutils/socket_request.h>
 
 using namespace std;
 using boost::scoped_ptr;
@@ -52,7 +53,7 @@ using namespace isc::server_common;
 using isc::UnitTestUtil;
 
 namespace {
-class ResolverConfig : public ::testing::Test {
+class ResolverConfig : public TestSocketRequestor {
 protected:
     IOService ios;
     DNSService dnss;
@@ -61,7 +62,10 @@ protected:
     scoped_ptr<const IOMessage> query_message;
     scoped_ptr<const Client> client;
     scoped_ptr<const RequestContext> request;
-    ResolverConfig() : dnss(ios, NULL, NULL, NULL) {
+    ResolverConfig() :
+        TestSocketRequestor(dnss, address_store_, 53210),
+        dnss(ios, NULL, NULL, NULL)
+    {
         server.setDNSService(dnss);
         server.setConfigured();
     }
@@ -77,6 +81,7 @@ protected:
         return (*request);
     }
     void invalidTest(const string &JSON, const string& name);
+    isc::server_common::portconfig::AddressList address_store_;
 };
 
 TEST_F(ResolverConfig, forwardAddresses) {
@@ -188,6 +193,18 @@ TEST_F(ResolverConfig, invalidForwardAddresses) {
 // Try setting the addresses directly
 TEST_F(ResolverConfig, listenAddresses) {
     isc::testutils::portconfig::listenAddresses(server);
+    // Check it requests the correct addresses
+    const char* tokens[] = {
+        "TCP:127.0.0.1:53210:1",
+        "UDP:127.0.0.1:53210:2",
+        "TCP:::1:53210:3",
+        "UDP:::1:53210:4",
+        NULL
+    };
+    checkTokens(tokens, given_tokens_, "Given tokens");
+    // It returns back to empty set of addresses afterwards, so
+    // they should be released
+    checkTokens(tokens, released_tokens_, "Released tokens");
 }
 
 // Try setting some addresses and a rollback
diff --git a/src/lib/server_common/tests/portconfig_unittest.cc b/src/lib/server_common/tests/portconfig_unittest.cc
index 7e3ec1f..9e6ad46 100644
--- a/src/lib/server_common/tests/portconfig_unittest.cc
+++ b/src/lib/server_common/tests/portconfig_unittest.cc
@@ -133,7 +133,7 @@ TEST_F(ParseAddresses, invalid) {
 struct InstallListenAddresses : public testutils::TestSocketRequestor {
     InstallListenAddresses() :
         // The members aren't initialized yet, but we need to store refs only
-        TestSocketRequestor(dnss_, store_), dnss_(ios_, NULL, NULL, NULL)
+        TestSocketRequestor(dnss_, store_, 5288), dnss_(ios_, NULL, NULL, NULL)
     {
         valid_.push_back(AddressPair("127.0.0.1", 5288));
         valid_.push_back(AddressPair("::1", 5288));
diff --git a/src/lib/testutils/portconfig.h b/src/lib/testutils/portconfig.h
index 8e61ffc..a4dc2bd 100644
--- a/src/lib/testutils/portconfig.h
+++ b/src/lib/testutils/portconfig.h
@@ -95,12 +95,11 @@ listenAddressConfig(Server& server) {
     EXPECT_EQ("127.0.0.1", server.getListenAddresses()[0].first);
     EXPECT_EQ(53210, server.getListenAddresses()[0].second);
 
-    // As this is example address, the machine should not have it on
-    // any interface
+    // This address is rejected by the test socket requestor
     config = Element::fromJSON("{"
                                "\"listen_on\": ["
                                "   {"
-                               "       \"address\": \"192.0.2.0\","
+                               "       \"address\": \"192.0.2.2\","
                                "       \"port\": 53210"
                                "   }"
                                "]"
diff --git a/src/lib/testutils/socket_request.h b/src/lib/testutils/socket_request.h
index aea5667..0ca741f 100644
--- a/src/lib/testutils/socket_request.h
+++ b/src/lib/testutils/socket_request.h
@@ -33,25 +33,89 @@ extern bool test_mode;
 
 namespace testutils {
 
-// TODO Docs
-
+/// \brief A testcase part for faking the SocketRequestor in tests
+///
+/// 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).
+///
+/// Furthermore, you can check if the code requested or released the correct
+/// list of sockets.
+///
+/// \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,
-    public ::testing::Test {
+    virtual public ::testing::Test {
 protected:
+    /// \brief Constructor
+    ///
+    /// \param dnss The DNS service. It is expected this gets initialized
+    ///     after the TestSocketRequestor constructor is called, as the
+    ///     TestSocketRequestor should be a base class and the service only
+    ///     a member.
+    /// \param store Address store used when cleaning up.
+    /// \param expect_port The port which is expected to be requested. If
+    ///     the application requests a different port, it is considered
+    ///     a failure.
     TestSocketRequestor(asiodns::DNSService& dnss,
-                        server_common::portconfig::AddressList& store) :
-        last_token_(0), break_rollback_(false), dnss_(dnss), store_(store)
+                        server_common::portconfig::AddressList& store,
+                        uint16_t expect_port) :
+        last_token_(0), break_rollback_(false), dnss_(dnss), store_(store),
+        expect_port_(expect_port)
     {}
-    // Store the tokens as they go in and out
+    /// \brief Destructor
+    virtual ~ TestSocketRequestor() {}
+    /// \brief Tokens released by releaseSocket
+    ///
+    /// They are stored here by this class and you can examine them.
     std::vector<std::string> released_tokens_;
+    /// \brief Tokens returned from requestSocket
+    ///
+    /// They are stored here by this class and you can examine them.
     std::vector<std::string> given_tokens_;
+private:
     // Last token number and fd given out
     size_t last_token_;
-    // Should we break the rollback?
+protected:
+    /// \brief Support a broken rollback case
+    ///
+    /// If this is set to true, the requestSocket will throw when the
+    /// ::1 address is requested.
     bool break_rollback_;
+    /// \brief Release a socket
+    ///
+    /// This only stores the token passed.
+    /// \param token The socket to release
     void releaseSocket(const std::string& token) {
         released_tokens_.push_back(token);
     }
+    /// \brief Request a socket
+    ///
+    /// This creates a new token and fakes a new socket and returs it.
+    /// The token is stored.
+    ///
+    /// In case the address is 192.0.2.2 or if the break_rollback_ is true
+    /// and address is ::1, it throws.
+    ///
+    /// The tokens produced are in form of protocol:address:port:fd. The fds
+    /// start at 1 and increase by each successfull call.
+    ///
+    /// \param protocol The protocol to request
+    /// \param address to bind to
+    /// \param port to bind to
+    /// \param mode checked to be DONT_SHARE for now
+    /// \param name checked to be dummy_app for now
+    /// \return The token and FD
+    /// \throw SocketError as described above, to test error handling
     SocketID requestSocket(Protocol protocol, const std::string& address,
                            uint16_t port, ShareMode mode,
                            const std::string& name)
@@ -70,7 +134,7 @@ protected:
         }
         const std::string proto(protocol == TCP ? "TCP" : "UDP");
         size_t number = ++ last_token_;
-        EXPECT_EQ(5288, port);
+        EXPECT_EQ(expect_port_, port);
         EXPECT_EQ(DONT_SHARE, mode);
         EXPECT_EQ("dummy_app", name);
         const std::string token(proto + ":" + address + ":" +
@@ -79,12 +143,26 @@ protected:
         given_tokens_.push_back(token);
         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).
@@ -95,7 +173,17 @@ protected:
         // And return the mode
         server_common::portconfig::test_mode = false;
     }
-    // This checks the set of tokens is the same
+    /// \brief Check the list of tokens is as expected
+    ///
+    /// Compares the expected and real tokens.
+    ///
+    /// \param expected List of the expected tokens, as NULL-terminated array
+    ///     of C strings (it is more convenient to type as a constant than to
+    ///     manually push_back all the strings to a vector).
+    /// \param real The token list that was produced by this class (usually
+    ///     either given_tokens_ or released_tokens_).
+    /// \param scope Human readable identifier of which checkTokens call it is.
+    ///     It is printed as a part of failure message.
     void checkTokens(const char** expected,
                      const std::vector<std::string>& real,
                      const char* scope)
@@ -112,6 +200,7 @@ protected:
 private:
     asiodns::DNSService& dnss_;
     server_common::portconfig::AddressList& store_;
+    uint16_t expect_port_;
 };
 
 }
diff --git a/src/lib/testutils/srv_test.h b/src/lib/testutils/srv_test.h
index c92e876..c262447 100644
--- a/src/lib/testutils/srv_test.h
+++ b/src/lib/testutils/srv_test.h
@@ -44,8 +44,11 @@ extern const unsigned int RA_FLAG;
 extern const unsigned int AD_FLAG;
 extern const unsigned int CD_FLAG;
 
-// The base class for Auth and Recurse test case
-class SrvTestBase : public ::testing::Test {
+/// \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 {
 protected:
     SrvTestBase();
     virtual ~SrvTestBase();




More information about the bind10-changes mailing list