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
-------------------------------------+-------------------------------------

Comment (by tmark):

 Replying to [comment:5 marcin]:
 >
 > '''checkNakResponse()'''
 >
 > Please add a short description of the function
 >

 Done.

 > 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) {
 > }}}
 >

 Done.

 > !''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.

 This line is already followed by a call to ASSERT_TRU(ia).


 >
 > '''!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.


 Done.


 >
 > '''!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

 I reverted the original commit to ChangeLog as it was made prematurely.
 The following is submitted for review as the new ChangeLog entry:


 5XX.    [func]      tmark
     b10-dhcp6: Added unit tests for handling requests when no
     v6 subnets are configured/defined.  Testing these conditions
     was overlooked during implementation of Trac #2719.
     (Trac #2721, git 03b6bd8461d7aee84ac34da1c737fed4a3c16c22)
     (Trac #2721, git c533897b58690ab30d12dd9837de04e08d840ee3)

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