BIND 10 #2823: complete removing threads from stats tests

BIND 10 Development do-not-reply at isc.org
Mon May 6 23:25:50 UTC 2013


#2823: complete removing threads from stats tests
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  statistics    |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130514
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:9 vorner]:

 > The tests fail for me:
 > {{{
 ...
 > AssertionError: '1308759248.0' != '1308726848.0'
 > - 1308759248.0
 > ?      --
 > + 1308726848.0
 > ?       ++
 > }}}

 Oops, it's because specific local time was hardcoded.  I believe it
 was fixed in the latest version.

 > This testcase is somewhat useless, with the return at the start of it:
 > {{{#!python
 > class TestOSEnv(unittest.TestCase):
 >     def test_osenv(self):
 >         """
 >         test for the environ variable "B10_FROM_SOURCE"
 >         "B10_FROM_SOURCE" is set in Makefile
 >         """
 >         return
 ...
 > }}}

 My bad, it was a leftover.  Removed, and the test should still pass.

 > Do statistics really do such a strange thing as here? Once the queries
 per zone are a list with names inside the items, once it is named set:
 > {{{#!python
 >             'Auth': {'queries.udp': 4, 'queries.tcp': 6,
 >                      'queries.perzone': [
 >                     {'queries.udp': 8, 'queries.tcp': 10,
 >                      'zonename': 'test1.example'},
 >                     {'queries.udp': 6, 'queries.tcp': 8,
 >                      'zonename': 'test2.example'}],
 >                      'nds_queries.perzone': {
 >                     'test10.example': {'queries.udp': 8, 'queries.tcp':
 10},
 >                     'test20.example': {'queries.udp': 6, 'queries.tcp':
 8}}}}
 > }}}

 This is basically not specific to this branch (so I don't think I'm
 the right person to answer), but my guess is these are artificial
 test-only spec and data.  In fact, the real Auth module doesn't (yet)
 even support per zone statistics at all.  It's probably better to
 avoid reusing the existing module for tests like this, but that would
 be beyond the scope of this task.  I at least clarified the (seeming)
 intent of the unrealistic spec and data in comments.

 > What is the purpose of this test? Does it really test that the test code
 throws an exception?
 > {{{#!python
 >      def test_init_hterr(self):
 >         class FailingStatsHttpd(SimpleStatsHttpd):
 >             def open_httpd(self):
 >                 raise stats_httpd.HttpServerError
 >         self.assertRaises(stats_httpd.HttpServerError,
 FailingStatsHttpd)
 > }}}

 Hmm, on a closer look, the original test (implicitly) checks one more
 thing (in an expensive way): stats-httpd should tell !ConfigMgr of its
 departure (by send_stopping()).  I believe it's sufficient to check
 if close_mccs() is called, so I revised the test so it checks this
 condition, too.

 > Also, there's lot of code snippets like:
 > {{{#!python
 > self.assertEqual(None, something)
 > }}}
 >
 > Would it be better to use
 > {{{#!python
 > self.assertIsNone(something)
 > }}}
 >
 > The same with `assertIsNotNone`.

 Changed them as many as I can find.

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


More information about the bind10-tickets mailing list