[3087] Additional minor review updates.

Minor corrections based on re-review.

src/bin/d2/nc_add.h

@@ -136,7 +136,7 @@ protected:
     ///
     /// The READY_ST is the state the model transitions into when the inherited
     /// method, startTransaction() is invoked.  This handler, therefore, as the
-    /// is the entry point into the state model execuition.h  Its primary task
+    /// is the entry point into the state model execution.h  Its primary task
     /// is to determine whether to start with a forward DNS change or a
     /// reverse DNS change.
     ///
@@ -148,7 +148,7 @@ protected:
     /// includes only a reverse change.
     ///
     /// @throw NameAddTransactionError if upon entry next event is not
-    /// START_EVT.READY_ST.
+    /// START_EVT.
     void readyHandler();
 
     /// @brief State handler for SELECTING_FWD_SERVER_ST.

src/bin/d2/nc_trans.h

@@ -204,9 +204,12 @@ protected:
     /// currently selected server.  Since the send is asynchronous, the method
     /// posts NOP_EVT as the next event and then returns.
     ///
+    /// @param use_tsig True if the udpate should be include a TSIG key. This
+    /// is not yet implemented.
+    ///
     /// If an exception occurs it will be logged and and the transaction will
     /// be failed.
-    virtual void sendUpdate(bool use_tsig_ = false);
+    virtual void sendUpdate(bool use_tsig = false);
 
     /// @brief Adds events defined by NameChangeTransaction to the event set.
     ///
@@ -354,7 +357,9 @@ protected:
     /// @param value is the new value to assign.
     void setUpdateAttempts(const size_t value);
 
-    /// @todo
+    /// @brief Fetches the IOService the transaction uses for IO processing.
+    ///
+    /// @return returns a const pointer to the IOService.
     const IOServicePtr& getIOService() {
         return (io_service_);
     }

src/bin/d2/tests/nc_add_unittests.cc

@@ -350,17 +350,18 @@ TEST_F(NameAddTransactionTest, selectingFwdServerHandler) {
     // next event of SELECT_SERVER_EVT.  Thereafter it will be with a next
     // event of SERVER_IO_ERROR_EVT.
     int num_servers = name_add->getForwardDomain()->getServers()->size();
-    for (int i = 0; i < num_servers; i++) {
+    for (int i = 0; i < num_servers; ++i) {
         // Run selectingFwdServerHandler.
-        EXPECT_NO_THROW(name_add->selectingFwdServerHandler());
+        ASSERT_NO_THROW(name_add->selectingFwdServerHandler())
+              << " num_servers: " << num_servers << " selections: " << i;
 
         // Verify that a server was selected.
-        EXPECT_TRUE(name_add->getCurrentServer());
+        ASSERT_TRUE(name_add->getCurrentServer());
 
         // Verify that we transitioned correctly.
-        EXPECT_EQ(NameAddTransaction::ADDING_FWD_ADDRS_ST,
+        ASSERT_EQ(NameAddTransaction::ADDING_FWD_ADDRS_ST,
               name_add->getCurrState());
-        EXPECT_EQ(NameChangeTransaction::SERVER_SELECTED_EVT,
+        ASSERT_EQ(NameChangeTransaction::SERVER_SELECTED_EVT,
               name_add->getNextEvent());
 
         // Post a server IO error event.  This simulates an IO error occuring
@@ -598,7 +599,7 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_Timeout) {
     // Verify that we can make maximum number of update attempts permitted
     // and then transition to selecting a new server.
     int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER;
-    for (int i = 1; i <= max_tries; i++) {
+    for (int i = 1; i <= max_tries; ++i) {
         const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest();
 
         // Run addingFwdAddrsHandler to send the request.
@@ -669,7 +670,7 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_InvalidResponse) {
     // Verify that we can make maximum number of update attempts permitted
     // and then transition to selecting a new server.
     int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER;
-    for (int i = 1; i <= max_tries; i++) {
+    for (int i = 1; i <= max_tries; ++i) {
         // Run addingFwdAddrsHandler to construct send the request.
         EXPECT_NO_THROW(name_add->addingFwdAddrsHandler());
 
@@ -944,7 +945,7 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_Timeout) {
     // Verify that we can make maximum number of update attempts permitted
     // and then transition to selecting a new server.
     int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER;
-    for (int i = 1; i <= max_tries; i++) {
+    for (int i = 1; i <= max_tries; ++i) {
         const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest();
 
         // Run replacingFwdAddrsHandler to send the request.
@@ -1012,7 +1013,7 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_CorruptResponse) {
     // Verify that we can make maximum number of update attempts permitted
     // and then transition to selecting a new server.
     int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER;
-    for (int i = 1; i <= max_tries; i++) {
+    for (int i = 1; i <= max_tries; ++i) {
         const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest();
 
         // Run replacingFwdAddrsHandler to send the request.
@@ -1078,17 +1079,18 @@ TEST_F(NameAddTransactionTest, selectingRevServerHandler) {
     // next event of SELECT_SERVER_EVT.  Thereafter it will be with a next
     // event of SERVER_IO_ERROR_EVT.
     int num_servers = name_add->getReverseDomain()->getServers()->size();
-    for (int i = 0; i < num_servers; i++) {
+    for (int i = 0; i < num_servers; ++i) {
         // Run selectingRevServerHandler.
-        EXPECT_NO_THROW(name_add->selectingRevServerHandler());
+        ASSERT_NO_THROW(name_add->selectingRevServerHandler())
+              << " num_servers: " << num_servers << " selections: " << i;
 
         // Verify that a server was selected.
-        EXPECT_TRUE(name_add->getCurrentServer());
+        ASSERT_TRUE(name_add->getCurrentServer());
 
         // Verify that we transitioned correctly.
-        EXPECT_EQ(NameAddTransaction::REPLACING_REV_PTRS_ST,
+        ASSERT_EQ(NameAddTransaction::REPLACING_REV_PTRS_ST,
                   name_add->getCurrState());
-        EXPECT_EQ(NameChangeTransaction::SERVER_SELECTED_EVT,
+        ASSERT_EQ(NameChangeTransaction::SERVER_SELECTED_EVT,
                   name_add->getNextEvent());
 
         // Post a server IO error event.  This simulates an IO error occuring
@@ -1250,7 +1252,7 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_Timeout) {
     // Verify that we can make maximum number of update attempts permitted
     // and then transition to selecting a new server.
     int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER;
-    for (int i = 1; i <= max_tries; i++) {
+    for (int i = 1; i <= max_tries; ++i) {
         const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest();
 
         // Run replacingRevPtrsHandler to send the request.
@@ -1317,7 +1319,7 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_CorruptResponse) {
     // Verify that we can make maximum number of update attempts permitted
     // and then transition to selecting a new server.
     int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER;
-    for (int i = 1; i <= max_tries; i++) {
+    for (int i = 1; i <= max_tries; ++i) {
         const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest();
 
         // Run replacingRevPtrsHandler to send the request.

src/bin/d2/tests/nc_trans_unittests.cc

@@ -939,8 +939,8 @@ TEST_F(NameChangeTransactionTest, retryTransition) {
               name_change->getNextEvent());
 
     // Verify that we have not exceeded maximum number of attempts.
-    EXPECT_TRUE(name_change->getUpdateAttempts() <
-                NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER);
+    ASSERT_LT(name_change->getUpdateAttempts(),
+              NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER);
 
     // Call retryTransition.
     ASSERT_NO_THROW(name_change->retryTransition(
@@ -998,7 +998,6 @@ TEST_F(NameChangeTransactionTest, sendUpdateDoUpdateFailure) {
 
 /// @brief Tests sendUpdate method when underlying doUpdate times out.
 TEST_F(NameChangeTransactionTest, sendUpdateTimeout) {
-    isc::log::initLogger();
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
     ASSERT_NO_THROW(name_change->initDictionaries());
@@ -1032,10 +1031,9 @@ TEST_F(NameChangeTransactionTest, sendUpdateTimeout) {
     ASSERT_EQ(DNSClient::TIMEOUT, name_change->getDnsUpdateStatus());
 }
 
-/// @brief Tests sendUpdate method when it receives a corrupt respons from
+/// @brief Tests sendUpdate method when it receives a corrupt response from
 /// the server.
 TEST_F(NameChangeTransactionTest, sendUpdateCorruptResponse) {
-    isc::log::initLogger();
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
     ASSERT_NO_THROW(name_change->initDictionaries());
@@ -1075,7 +1073,6 @@ TEST_F(NameChangeTransactionTest, sendUpdateCorruptResponse) {
 
 /// @brief Tests sendUpdate method when the exchange succeeds.
 TEST_F(NameChangeTransactionTest, sendUpdate) {
-    isc::log::initLogger();
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
     ASSERT_NO_THROW(name_change->initDictionaries());