BIND 10 #2721: Write unit test for Ticket 2719

[BIND 10 Development]

#2721: Write unit test for Ticket 2719
-------------------------------------+-------------------------------------
            Reporter:  jwright       |                        Owner:  tmark
                Type:  task          |                       Status:
            Priority:  medium        |  reviewing
           Component:  Unclassified  |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130328
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  UnAssigned => tmark


Comment:

 '''checkNakResponse()'''

 Please add a short description of the function

 The list of function arguments in the second line should be aligned with
 arguments in the first line like this
 {{{
     void checkNakResponse(const Pkt6Ptr& rsp, uint8_t
 expected_message_type,
                           uint32_t expected_transid, uint16_t
 expected_status_code) {
 }}}

 !''tmp!'' is not the best name for the object holding an option. I can see
 that !''tmp!'' is used everywhere in this file but it is wrong.
 Assume, that in the following code:
 {{{
 // Check that IA_NA was returned
 OptionPtr tmp = rsp->getOption(D6O_IA_NA);
 ASSERT_TRUE(tmp);
 }}}

 the ''getOption()'' function returns NULL pointer. The ASSERT will report
 an error, visible in a log file. This log will tell me that some temporary
 value is NULL. If you called this variable !''option_ia_na!'' I would know
 from the log that it is IA option which is NULL - that gives me some more
 information.


 '''checkIA_NA''':
 This line the code:
 {{{
 boost::shared_ptr<Option6IA> ia =
 boost::dynamic_pointer_cast<Option6IA>(tmp);
 }}}

 may result in !''ia!'' being NULL pointer. Please consider adding
 !''ASSERT_TRUE(ia)!'' before using it further in the code.

 '''!SolicitNoSubnet''': I suggest that the function description is
 rephrased: !''Test verifies that incoming SOLICIT can be handled properly
 when there are no subnets configured!''. The word !''even!'' suggests that
 test may be also doing other checks and the no-subnet check is only
 special case. This refers for other tests as well.

 '''!RequestNoSubnet''': The test description seems to be copied from the
 previous test. Please replace SOLICIT with REQUEST.

 '''!ChangeLog!'''
 - Spurious !'']!'' in !''func!''.
 - There is a white-space aty the end of the second line in the entry.
 - (See Trac #2719): perhaps it would be better to mention that unit-tests
 being added have been missed during implementation of #2719.
 - There should be a blank line between your entry and the previous entry
 - s/v6/IPv6 as the latter is the actual protocol name

-- 
Ticket URL: <https://bind10.isc.org/ticket/2721#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development