BIND 10 #2353: write tests for all methods of Bob

[BIND 10 Development]

#2353: write tests for all methods of Bob
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20121120
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:  Boss   |           Sub-Project:  DNS
  of BIND                            |  Estimated Difficulty:  8
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''test_socket_srv'''

 Ideally we should test some more things:
 - remove_socket_srv really removes the directory and file and closes
   the socket (btw if we test these we do not need to reset
   `_srv_socket` etc in the main code).
 - the local address of `_srv_socket` matches `_socket_path`.
 - nothing happens if `srv_socket` is None
 - after init_socket_srv it really listens on the socket

 '''test_get_processes'''

 - this doesn't confirm what happens if `components` has more than one
   components, and whether they are sorted by PID
 - does runnable have to be set?

 '''test_reap_children'''

 it doesn't cover some cases
 {{{#!python
             except OSError as o:
                 if o.errno == errno.ECHILD:
                     break
             if pid in self.components:
                 if component.is_running() and self.runnable:
                     if not component_restarted:
 }}}

 '''test_kill_started_components'''

 - does runnable have to be set?
 - it doesn't check kill() is called with parameter of `True`
 - isn't it better to check if `components` is an empty dict directory?

 '''test_start_process

 - if not very difficult, I'd like to use some mock `ProcessInfo` and
   check more detailed behavior like whether the expected parameters
   are is passed or spawn() is called.
 - in addition to that, in this case maybe invoking an actual process
   may be a good idea (normally it's controversial for unit tests), but
   /bin/true is not always available.  Also, IMO such a test should
   actually go to tests for `ProcessInfo` because invoking processes is
   expensive and error prone (and it seems we actually already do it).
 - does runnable have to be set?

 '''test_register_process'''

 - does runnable have to be set?
 - isn't it better to examine components directly?

 '''test_start_simple'''
 - does runnable have to be set?
 - same comments as those for started_components apply
 - verbose case isn't tested
 - in this case I actually think it's sufficient if we confirm
   start_process() (we'd fake it for the test) is called with expected
   params...and then noticed test_start_auth2, etc do it.  I believe we
   can do the same for this method.

 '''test_start_auth2/resolver2/cmdctl'''

 - if possible I'd avoid names like XXX2 because it's often ambiguous
   and easily subject to renumbering problems.  same comment applies to
   other similar ones.
 - verbose case isn't tested
 - for cmdctl, port option case isn't tested

 '''Others'''

 - I guess start_msgq() and start_cfgmgr() need similar tests like
   test_start_auth2
 - same for start_ccsession (checking if expected things are called
   with expected params)
 - from a quick look tests for startup() are not sufficient.
 - I was not 100% sure tests for shutdown() are sufficient.  Please
   check.
 - `_socket_data` itself doesn't seem to be tested, at least not
   sufficiently.
 - is run() sufficiently tested?  It was not clear from the existing
   tests.

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