BIND 10 #2781: Stats.do_polling should have direct tests

BIND 10 Development do-not-reply at isc.org
Thu Aug 22 12:23:52 UTC 2013


#2781: Stats.do_polling should have direct tests
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  naokikambe
            Priority:  medium        |                       Status:
           Component:  statistics    |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130903
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => naokikambe


Comment:

 Hello.

 Why does this code use while and removing items from the list instead of
 for? And, even if while would be needed, the len is not needed, you can
 use a list in a boolean expression and it is true if non-empty.

 {{{#!python
         _sequences = sequences[:]
         while len(_sequences) > 0:
             (module_name, seq) = (None, None)
             try:
 }}}

 This description doesn't match what is actually being done, because
 there's `assertRaises` at the end, not `assertEqual([], …)`.

 {{{#!python
     def test_get_multi_module_list_initsessiontimeout(self):
         """Test _get_multi_module_list() returns an empty list if
 rcp_call()
         raise a InitSeeionTimeout exception"""
 }}}

 This class seems odd. Is there a reason why the method is created by
 lambda and not by `def`? And, why it isn't actually just a dict with data
 and a complete class is needed?

 {{{#!python
         class DummyDict:
             items = lambda x: [
                 ('Init', 'dummy'), ('Stats', 'dummy'), ('Auth', 'dummy'),
                 ]
 }}}

 Also, it seems there's quite a lot of changes not directly related to the
 ticket. Not that I'd think the changes are for bad, but it might have been
 better to create a separate ticket for it. But I don't insist on doing
 that ex-post.

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


More information about the bind10-tickets mailing list