BIND 10 #2781: Stats.do_polling should have direct tests
BIND 10 Development
do-not-reply at isc.org
Tue Aug 27 08:13:10 UTC 2013
#2781: Stats.do_polling should have direct tests
-------------------------------------+-------------------------------------
Reporter: jinmei | Owner:
Type: defect | vorner
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 naokikambe):
* owner: naokikambe => vorner
Comment:
Hello, thank you for reviewing.
Replying to [comment:7 vorner]:
> 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:
> }}}
I've used for-loop instead. Could you look at this?
{{{
c347349 [2781] use for-loop instead of while-loop for avoiding from
rewriting the iterator
}}}
> 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"""
> }}}
I mistook. I've corrected documentation and the code.
{{{
5207d19 [2781] correct description of
test_get_multi_module_list_initsessiontimeout()
}}}
> 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'),
> ]
> }}}
I've defined it as a normal function. But it is needed because the order
of the array should be preserved at that code.
{{{
3669c95 [2781] define items() in DummDict class instead of using lambda,
update note
}}}
> 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.
As you know, do_polling() had various tasks as asking Init each module
information, asking each module statistics, and collecting statistics. I
think the scope of this ticket has become broad. Then some of changes I
made might become things not directly related to this ticket. If you would
like to have specific changes to be reviewed in separate tickets, please
point out.
Regards,
--
Ticket URL: <http://bind10.isc.org/ticket/2781#comment:8>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list