BIND 10 #3359: DB backends: Share unit-tests between databases (memfile, mysql, postgres)

[BIND 10 Development]

#3359: DB backends: Share unit-tests between databases (memfile, mysql, postgres)
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcpdb        |                    Milestone:  DHCP-
            Keywords:                |  Kea0.9-alpha
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  8             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  2
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * hours:  6 => 2
 * owner:  marcin => tomek
 * totalhours:  6 => 8


Comment:

 Reviewed 032ae89cd0ca9d04703555611aece3d3b791565f

 I removed a couple of trivial errors from the code. Please pull the latest
 commit from the branch.

 Just a couple of remaining nits:

 '''src/lib/dhcpsrv/tests/lease_mgr_unittest.cc'''
 The reopen function in the abstract class is virtual, so I made an
 occurence in this file virtual too. It is not a must, but looks cleaner.

 '''src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc'''
 addGetDelete6: It would be useful if there was a comment before an
 invocation of:
 {{{
 testAddGetDelete6(true);
 }}}
 to explain what the !''true!'' value is?

 What is (2) in getLease4ClientIdSize?

 It should be explained why this test: DISABLED_getLeases6DuidIaid is
 disabled. Same refers to DISABLED_updateLease and DISABLED_updateLease6.

 Question regarding not-applicable test. Do you think it would be
 reasonable to apply some constraints on the length of the variable length
 data fields in the memfile data structures?

 This test getLease6DuidIaidSubnetIdSize lacks description.

 '''src/lib/dhcpsrv/tests/test_utils.cc'''
 I removed brief descriptions from the tests in the .cc file, because they
 already have the documentation in the header file. It would be hard to
 keep both descriptions consistent so there should be just one description
 in the header.

 '''src/lib/dhcpsrv/tests/test_utils.h'''
 I don't really like the fact that we put generic test fixture classes into
 files named !''test_utils.h!''. First, the utility nature of these files
 suggests that it holds convenience functions, like convert one type to the
 other etc. The !''!GenericLeaseMgrTest!'' is a test fixture class
 implementation and it contains actual test cases. I think it deserves
 being in a separate file. It is hard to find this class without greping
 the code, if it is in the test_utils.h.

 It would be more readable if brief comments for the member variables
 weren't inline, but each description preceded the member declaration.

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