BIND 10 master, updated. 1e8fea4a8987ece65543c5f30f09cdc4d5dc5746 [master ] Merge branch 'trac3264'

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Jan 15 13:31:41 UTC 2014


The branch, master has been updated
       via  1e8fea4a8987ece65543c5f30f09cdc4d5dc5746 (commit)
       via  8edba8799ee54f28e6ab8ee9abe0b60e033ca954 (commit)
       via  432acd7ecbdbe741d07c20f8b43be107d9f761b2 (commit)
       via  faf86ad3a04fc7fca0af2b9d19abba0fa1d94387 (commit)
      from  661e1f133e6c3a3f40c2af4ee8be358e114d641e (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 1e8fea4a8987ece65543c5f30f09cdc4d5dc5746
Merge: 661e1f1 8edba87
Author: Thomas Markwalder <tmark at isc.org>
Date:   Wed Jan 15 08:22:49 2014 -0500

    [master ] Merge branch 'trac3264'
    
    Refactored some unit test classes in src/bin/d2/tests

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

Summary of changes:
 src/bin/d2/tests/dns_client_unittests.cc     |    6 +-
 src/bin/d2/tests/nc_add_unittests.cc         |  117 +++--------------
 src/bin/d2/tests/nc_test_utils.cc            |    4 +-
 src/bin/d2/tests/nc_trans_unittests.cc       |  181 +++++++++-----------------
 src/bin/d2/tests/test_data_files_config.h.in |    2 +-
 5 files changed, 82 insertions(+), 228 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/d2/tests/dns_client_unittests.cc b/src/bin/d2/tests/dns_client_unittests.cc
index a24196e..1b4c2bd 100644
--- a/src/bin/d2/tests/dns_client_unittests.cc
+++ b/src/bin/d2/tests/dns_client_unittests.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -128,7 +128,7 @@ public:
                           response_->getRRCount(D2UpdateMessage::SECTION_ZONE));
                 D2ZonePtr zone = response_->getZone();
                 ASSERT_TRUE(zone);
-                EXPECT_EQ("response.example.com.", zone->getName().toText());
+                EXPECT_EQ("example.com.", zone->getName().toText());
                 EXPECT_EQ(RRClass::IN().getCode(), zone->getClass().getCode());
 
             } else {
@@ -288,7 +288,7 @@ public:
         // Create a request DNS Update message.
         D2UpdateMessage message(D2UpdateMessage::OUTBOUND);
         ASSERT_NO_THROW(message.setRcode(Rcode(Rcode::NOERROR_CODE)));
-        ASSERT_NO_THROW(message.setZone(Name("response.example.com"), RRClass::IN()));
+        ASSERT_NO_THROW(message.setZone(Name("example.com"), RRClass::IN()));
 
         // In order to perform the full test, when the client sends the request
         // and receives a response from the server, we have to emulate the
diff --git a/src/bin/d2/tests/nc_add_unittests.cc b/src/bin/d2/tests/nc_add_unittests.cc
index 9aecda4..05e8a1a 100644
--- a/src/bin/d2/tests/nc_add_unittests.cc
+++ b/src/bin/d2/tests/nc_add_unittests.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -187,19 +187,12 @@ typedef boost::shared_ptr<NameAddStub> NameAddStubPtr;
 ///
 /// Note this class uses NameAddStub class to exercise non-public
 /// aspects of NameAddTransaction.
-class NameAddTransactionTest : public ::testing::Test {
+class NameAddTransactionTest : public TransactionTest {
 public:
-    IOServicePtr io_service_;
-    DdnsDomainPtr forward_domain_;
-    DdnsDomainPtr reverse_domain_;
 
-    NameAddTransactionTest() : io_service_(new isc::asiolink::IOService()) {
+    NameAddTransactionTest() {
     }
 
-    static const unsigned int FORWARD_CHG = 0x01;
-    static const unsigned int REVERSE_CHG = 0x02;
-    static const unsigned int FWD_AND_REV_CHG = REVERSE_CHG | FORWARD_CHG;
-
     virtual ~NameAddTransactionTest() {
     }
 
@@ -211,53 +204,12 @@ public:
     /// will have either the forward, reverse, or both domains populated.
     ///
     /// @param change_mask determines which change directions are requested
-    NameAddStubPtr makeTransaction4(int change_mask=FWD_AND_REV_CHG) {
-        const char* msg_str =
-            "{"
-            " \"change_type\" : 0 , "
-            " \"forward_change\" : true , "
-            " \"reverse_change\" : true , "
-            " \"fqdn\" : \"my.forward.example.com.\" , "
-            " \"ip_address\" : \"192.168.2.1\" , "
-            " \"dhcid\" : \"0102030405060708\" , "
-            " \"lease_expires_on\" : \"20130121132405\" , "
-            " \"lease_length\" : 1300 "
-            "}";
-
-        // Create NameChangeRequest from JSON string.
-        dhcp_ddns::NameChangeRequestPtr ncr = dhcp_ddns::NameChangeRequest::
-                                              fromJSON(msg_str);
-
-        // If the change mask does not include a forward change clear the
-        // forward domain; otherwise create the domain and its servers.
-        if (!(change_mask & FORWARD_CHG)) {
-            ncr->setForwardChange(false);
-            forward_domain_.reset();
-        } else {
-            // Create the forward domain and then its servers.
-            forward_domain_ = makeDomain("example.com.");
-            addDomainServer(forward_domain_, "forward.example.com",
-                            "1.1.1.1");
-            addDomainServer(forward_domain_, "forward2.example.com",
-                            "1.1.1.2");
-        }
-
-        // If the change mask does not include a reverse change clear the
-        // reverse domain; otherwise create the domain and its servers.
-        if (!(change_mask & REVERSE_CHG)) {
-            ncr->setReverseChange(false);
-            reverse_domain_.reset();
-        } else {
-            // Create the reverse domain and its server.
-            reverse_domain_ = makeDomain("2.168.192.in.addr.arpa.");
-            addDomainServer(reverse_domain_, "reverse.example.com",
-                            "2.2.2.2");
-            addDomainServer(reverse_domain_, "reverse2.example.com",
-                            "2.2.2.3");
-        }
+    NameAddStubPtr makeTransaction4(int change_mask = FWD_AND_REV_CHG) {
+        // Creates IPv4 remove request, forward, and reverse domains.
+        setupForIPv4Transaction(dhcp_ddns::CHG_ADD, change_mask);
 
         // Now create the test transaction as would occur in update manager.
-        return (NameAddStubPtr(new NameAddStub(io_service_, ncr,
+        return (NameAddStubPtr(new NameAddStub(io_service_, ncr_,
                                                forward_domain_,
                                                reverse_domain_)));
     }
@@ -270,53 +222,16 @@ public:
     /// will have either the forward, reverse, or both domains populated.
     ///
     /// @param change_mask determines which change directions are requested
-    NameAddStubPtr makeTransaction6(int change_mask=FWD_AND_REV_CHG) {
-        const char* msg_str =
-            "{"
-            " \"change_type\" : 0 , "
-            " \"forward_change\" : true , "
-            " \"reverse_change\" : true , "
-            " \"fqdn\" : \"my6.forward.example.com.\" , "
-            " \"ip_address\" : \"2001:1::100\" , "
-            " \"dhcid\" : \"0102030405060708\" , "
-            " \"lease_expires_on\" : \"20130121132405\" , "
-            " \"lease_length\" : 1300 "
-            "}";
-
-        // Create NameChangeRequest from JSON string.
-        dhcp_ddns::NameChangeRequestPtr ncr = makeNcrFromString(msg_str);
-
-        // If the change mask does not include a forward change clear the
-        // forward domain; otherwise create the domain and its servers.
-        if (!(change_mask & FORWARD_CHG)) {
-            ncr->setForwardChange(false);
-            forward_domain_.reset();
-        } else {
-            // Create the forward domain and then its servers.
-            forward_domain_ = makeDomain("example.com.");
-            addDomainServer(forward_domain_, "fwd6-server.example.com",
-                            "2001:1::5");
-        }
-
-        // If the change mask does not include a reverse change clear the
-        // reverse domain; otherwise create the domain and its servers.
-        if (!(change_mask & REVERSE_CHG)) {
-            ncr->setReverseChange(false);
-            reverse_domain_.reset();
-        } else {
-            // Create the reverse domain and its server.
-            reverse_domain_ = makeDomain("1.2001.ip6.arpa.");
-            addDomainServer(reverse_domain_, "rev6-server.example.com",
-                            "2001:1::6");
-        }
+    NameAddStubPtr makeTransaction6(int change_mask = FWD_AND_REV_CHG) {
+        // Creates IPv6 remove request, forward, and reverse domains.
+        setupForIPv6Transaction(dhcp_ddns::CHG_ADD, change_mask);
 
         // Now create the test transaction as would occur in update manager.
-        return (NameAddStubPtr(new NameAddStub(io_service_, ncr,
+        return (NameAddStubPtr(new NameAddStub(io_service_, ncr_,
                                                forward_domain_,
                                                reverse_domain_)));
     }
 
-
     /// @brief Create a test transaction at a known point in the state model.
     ///
     /// Method prepares a new test transaction and sets its state and next
@@ -329,15 +244,19 @@ public:
     /// @param state value to set as the current state
     /// @param event value to post as the next event
     /// @param change_mask determines which change directions are requested
+    /// @param family selects between an IPv4 (AF_INET) and IPv6 (AF_INET6)
+    /// transaction.
     NameAddStubPtr prepHandlerTest(unsigned int state, unsigned int event,
-                                   unsigned int change_mask = FWD_AND_REV_CHG) {
-        NameAddStubPtr name_add = makeTransaction4(change_mask);
+                                   unsigned int change_mask = FWD_AND_REV_CHG,
+                                   short family = AF_INET) {
+        NameAddStubPtr name_add =  (family == AF_INET ?
+                                    makeTransaction4(change_mask) :
+                                    makeTransaction4(change_mask));
         name_add->initDictionaries();
         name_add->postNextEvent(event);
         name_add->setState(state);
         return (name_add);
     }
-
 };
 
 /// @brief Tests NameAddTransaction construction.
diff --git a/src/bin/d2/tests/nc_test_utils.cc b/src/bin/d2/tests/nc_test_utils.cc
index 5ec178d..b65b9e4 100644
--- a/src/bin/d2/tests/nc_test_utils.cc
+++ b/src/bin/d2/tests/nc_test_utils.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -172,7 +172,7 @@ TimedIO::runTimedIO(int run_time) {
     run_time_ = run_time;
     int cnt = io_service_->get_io_service().poll();
     if (cnt == 0) {
-        timer_.setup(boost::bind(&TransactionTest::timesUp, this), run_time_);
+        timer_.setup(boost::bind(&TimedIO::timesUp, this), run_time_);
         cnt = io_service_->get_io_service().run_one();
         timer_.cancel();
     }
diff --git a/src/bin/d2/tests/nc_trans_unittests.cc b/src/bin/d2/tests/nc_trans_unittests.cc
index 0829ff8..010d197 100644
--- a/src/bin/d2/tests/nc_trans_unittests.cc
+++ b/src/bin/d2/tests/nc_trans_unittests.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -275,83 +275,65 @@ typedef boost::shared_ptr<NameChangeStub> NameChangeStubPtr;
 ///
 /// Note this class uses NameChangeStub class to exercise non-public
 /// aspects of NameChangeTransaction.
-class NameChangeTransactionTest : public ::testing::Test {
+class NameChangeTransactionTest : public TransactionTest {
 public:
-    IOServicePtr io_service_;
-    DdnsDomainPtr forward_domain_;
-    DdnsDomainPtr reverse_domain_;
-    asiolink::IntervalTimer timer_;
-    int run_time_;
-
-    NameChangeTransactionTest()
-        : io_service_(new isc::asiolink::IOService()), timer_(*io_service_),
-         run_time_(0) {
+    NameChangeTransactionTest() {
     }
 
     virtual ~NameChangeTransactionTest() {
     }
 
-    /// @brief Run the IO service for no more than a given amount of time.
-    ///
-    /// Uses an IntervalTimer to interrupt the invocation of IOService run(),
-    /// after the given number of milliseconds elapse.  The timer executes
-    /// the timesUp() method if it expires.
-    ///
-    /// @param run_time amount of time in milliseconds to allow run to execute.
-    void runTimedIO(int run_time) {
-        run_time_ = run_time;
-        timer_.setup(boost::bind(&NameChangeTransactionTest::timesUp, this),
-                     run_time_);
-        io_service_->run();
-    }
-
-    /// @brief IO Timer expiration handler
-    ///
-    /// Stops the IOSerivce and fails the current test.
-    void timesUp() {
-        io_service_->stop();
-        FAIL() << "Test Time: " << run_time_ << " expired";
-    }
-
     /// @brief  Instantiates a NameChangeStub test transaction
     /// The transaction is constructed around a predefined (i.e "canned")
     /// NameChangeRequest. The request has both forward and reverse DNS
     /// changes requested, and both forward and reverse domains are populated.
     NameChangeStubPtr makeCannedTransaction() {
-        // NCR in JSON form.
-        const char* msg_str =
-            "{"
-            " \"change_type\" : 0 , "
-            " \"forward_change\" : true , "
-            " \"reverse_change\" : true , "
-            " \"fqdn\" : \"my.example.com.\" , "
-            " \"ip_address\" : \"192.168.2.1\" , "
-            " \"dhcid\" : \"0102030405060708\" , "
-            " \"lease_expires_on\" : \"20130121132405\" , "
-            " \"lease_length\" : 1300 "
-            "}";
-
-        // Create the request from JSON.
-        dhcp_ddns::NameChangeRequestPtr ncr = dhcp_ddns::NameChangeRequest::
-                                              fromJSON(msg_str);
-
-        // Make forward DdnsDomain with 2 forward servers.
-        forward_domain_ = makeDomain("example.com.");
-        addDomainServer(forward_domain_, "forward.example.com",
-                        "127.0.0.1", 5301);
-        addDomainServer(forward_domain_, "forward2.example.com",
-                        "127.0.0.1", 5302);
-
-        // Make reverse DdnsDomain with one reverse server.
-        reverse_domain_ = makeDomain("2.168.192.in.addr.arpa.");
-        addDomainServer(reverse_domain_, "reverse.example.com",
-                        "127.0.0.1", 5301);
+        // Creates IPv4 remove request, forward, and reverse domains.
+        setupForIPv4Transaction(dhcp_ddns::CHG_ADD, FWD_AND_REV_CHG);
 
+        // Now create the test transaction as would occur in update manager.
         // Instantiate the transaction as would be done by update manager.
-        return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr,
+        return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr_,
                                   forward_domain_, reverse_domain_)));
     }
 
+    /// @brief Builds and then sends an update request
+    ///
+    /// This method is used to build and send and update request. It is used
+    /// in conjuction with FauxServer to test various message response
+    /// scenarios.
+    /// @param name_change Transaction under test
+    /// @param run_time Maximum time to permit IO processing to run before
+    /// timing out (in milliseconds)
+    void doOneExchange(NameChangeStubPtr name_change,
+                       unsigned int run_time = 500) {
+        // Create a valid request for the transaction.
+        D2UpdateMessagePtr req;
+        ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::
+                                                      OUTBOUND)));
+        ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
+        req->setZone(dns::Name("request.example.com"), dns::RRClass::ANY());
+        req->setRcode(dns::Rcode(dns::Rcode::NOERROR_CODE));
+
+        // Set the flag to use the NameChangeStub's DNSClient callback.
+        name_change->use_stub_callback_ = true;
+
+        // Invoke sendUpdate.
+        ASSERT_NO_THROW(name_change->sendUpdate());
+
+        // Update attempt count should be 1, next event should be NOP_EVT.
+        ASSERT_EQ(1, name_change->getUpdateAttempts());
+        ASSERT_EQ(NameChangeTransaction::NOP_EVT,
+                  name_change->getNextEvent());
+
+        int cnt = 0;
+        while (name_change->getNextEvent() == NameChangeTransaction::NOP_EVT) {
+            ASSERT_NO_THROW(cnt = runTimedIO(run_time));
+            if (cnt == 0) {
+                FAIL() << "IO Service stopped unexpectedly";
+            }
+        }
+    }
 };
 
 /// @brief Tests NameChangeTransaction construction.
@@ -874,27 +856,16 @@ TEST_F(NameChangeTransactionTest, sendUpdateTimeout) {
     ASSERT_NO_THROW(name_change->initDictionaries());
     ASSERT_TRUE(name_change->selectFwdServer());
 
-    // Create a valid request.
-    D2UpdateMessagePtr req;
-    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
-    ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
-    req->setZone(dns::Name("request.example.com"), dns::RRClass::ANY());
-    req->setRcode(dns::Rcode(dns::Rcode::NOERROR_CODE));
-
-    // Set the flag to use the NameChangeStub's DNSClient callback.
-    name_change->use_stub_callback_ = true;
-
-    // Invoke sendUpdate.
-    ASSERT_NO_THROW(name_change->sendUpdate());
-
-    // Update attempt count should be 1, next event should be NOP_EVT.
-    EXPECT_EQ(1, name_change->getUpdateAttempts());
-    ASSERT_EQ(NameChangeTransaction::NOP_EVT,
-              name_change->getNextEvent());
-
-    // Run IO a bit longer than maximum allowed to permit timeout logic to
-    // execute.
-    runTimedIO(NameChangeTransaction::DNS_UPDATE_DEFAULT_TIMEOUT + 100);
+    // Build a valid request, call sendUpdate and process the response.
+    // Note we have to wait for DNSClient timeout plus a bit more to allow
+    // DNSClient to timeout.
+    // The method, doOneExchange, can suffer fatal assertions which invalidate
+    // not only it but the invoking test as well. In other words, if the
+    // doOneExchange blows up the rest of test is pointless. I use
+    // ASSERT_NO_FATAL_FAILURE to abort the test immediately.
+    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change,
+                                          NameChangeTransaction::
+                                          DNS_UPDATE_DEFAULT_TIMEOUT + 100));
 
     // Verify that next event is IO_COMPLETED_EVT and DNS status is TIMEOUT.
     ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
@@ -914,26 +885,8 @@ TEST_F(NameChangeTransactionTest, sendUpdateCorruptResponse) {
     FauxServer server(*io_service_, *(name_change->getCurrentServer()));
     server.receive(FauxServer::CORRUPT_RESP);
 
-    // Create a valid request for the transaction.
-    D2UpdateMessagePtr req;
-    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
-    ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
-    req->setZone(dns::Name("request.example.com"), dns::RRClass::ANY());
-    req->setRcode(dns::Rcode(dns::Rcode::NOERROR_CODE));
-
-    // Set the flag to use the NameChangeStub's DNSClient callback.
-    name_change->use_stub_callback_ = true;
-
-    // Invoke sendUpdate.
-    ASSERT_NO_THROW(name_change->sendUpdate());
-
-    // Update attempt count should be 1, next event should be NOP_EVT.
-    EXPECT_EQ(1, name_change->getUpdateAttempts());
-    ASSERT_EQ(NameChangeTransaction::NOP_EVT,
-              name_change->getNextEvent());
-
-    // Run the IO for 500 ms.  This should be more than enough time.
-    runTimedIO(500);
+    // Build a valid request, call sendUpdate and process the response.
+    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
 
     // Verify that next event is IO_COMPLETED_EVT and DNS status is INVALID.
     ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
@@ -952,26 +905,8 @@ TEST_F(NameChangeTransactionTest, sendUpdate) {
     FauxServer server(*io_service_, *(name_change->getCurrentServer()));
     server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR());
 
-    // Create a valid request for the transaction.
-    D2UpdateMessagePtr req;
-    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
-    ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
-    req->setZone(dns::Name("request.example.com"), dns::RRClass::ANY());
-    req->setRcode(dns::Rcode(dns::Rcode::NOERROR_CODE));
-
-    // Set the flag to use the NameChangeStub's DNSClient callback.
-    name_change->use_stub_callback_ = true;
-
-    // Invoke sendUpdate.
-    ASSERT_NO_THROW(name_change->sendUpdate());
-
-    // Update attempt count should be 1, next event should be NOP_EVT.
-    EXPECT_EQ(1, name_change->getUpdateAttempts());
-    ASSERT_EQ(NameChangeTransaction::NOP_EVT,
-              name_change->getNextEvent());
-
-    // Run the IO for 500 ms.  This should be more than enough time.
-    runTimedIO(500);
+    // Build a valid request, call sendUpdate and process the response.
+    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
 
     // Verify that next event is IO_COMPLETED_EVT and DNS status is SUCCESS.
     ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
diff --git a/src/bin/d2/tests/test_data_files_config.h.in b/src/bin/d2/tests/test_data_files_config.h.in
index 6064d3d..b351f88 100644
--- a/src/bin/d2/tests/test_data_files_config.h.in
+++ b/src/bin/d2/tests/test_data_files_config.h.in
@@ -1,4 +1,4 @@
-// Copyright (C) 2009  Internet Systems Consortium, Inc. ("ISC")   
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above



More information about the bind10-changes mailing list