[asterisk-dev] [Code Review]: speedup of cdrtestcase by not waiting for reactor timeout

[wdoekes]

> On Jan. 16, 2012, 9:13 a.m., Matt Jordan wrote:
> > asterisk/trunk/lib/python/asterisk/CDRTestCase.py, line 125
> > <https://reviewboard.asterisk.org/r/1671/diff/2/?file=23174#file23174line125>
> >
> >     Granted, ami_connect doesn't do anything in TestCase right now - but its probably not a bad idea to keep this, in case any behavior is added to the base class that all derived classes need... such as your hangup checking code

Mm.. you're probably right. Best practice is to: either replace 'pass' there with 'raise NotImplementedError' or calling the parent method like you suggest. I'll take a quick peek later to see which is best.


> On Jan. 16, 2012, 9:13 a.m., Matt Jordan wrote:
> > asterisk/trunk/lib/python/asterisk/CDRTestCase.py, lines 131-132
> > <https://reviewboard.asterisk.org/r/1671/diff/2/?file=23174#file23174line131>
> >
> >     Speaking of which - why don't we put this in TestCase?  I can't see how this will hurt any of the current tests, as it doesn't stop the test if any channels are still up and running and doing work.  I know we have a few other tests that simply wait for their reactors to time out - this has the potential to speed up a bunch of tests.

I was reluctant to do that immediately. I was planning on looking at the other tests once these were out of the way. (Moving the no_active_channels method to TestCase is a likely next step, but I planned on doing that whenever it's really needed in a non-CDR-test.)

In the CDRTestCase there is always one call (series of channels), so this should be fine.

For the TestCase there might very well be no call at all OR possibly a series of sequential calls (in which case this could break), so this is probably *not* a good default.


- wdoekes


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1671/#review5175
-----------------------------------------------------------


On Jan. 15, 2012, 3:16 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1671/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2012, 3:16 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Most CDR test cases simply waited for the reactor timeout to fire. Only the tests that had a failing call were terminated in a timely fashion.
> 
> Add a check for the Hangup and count 0 active channels.
> 
> 
> Diffs
> -----
> 
>   asterisk/trunk/lib/python/asterisk/CDRTestCase.py 2991 
>   asterisk/trunk/lib/python/asterisk/asterisk.py 2991 
>   asterisk/trunk/tests/cdr/console_dial_sip_answer/run-test 2991 
>   asterisk/trunk/tests/cdr/console_dial_sip_busy/run-test 2991 
> 
> Diff: https://reviewboard.asterisk.org/r/1671/diff
> 
> 
> Testing
> -------
> 
> 5 out of 8 CDR tests run faster now.
> 
> Before:
> 
> --> Running test 'tests/cdr/console_dial_sip_busy' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="3.40">
> --> Cannot run test 'tests/cdr/console_dial_sip_transfer'
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="0" time="0.00"/>
> --> Running test 'tests/cdr/console_dial_sip_answer' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="33.28">
> --> Running test 'tests/cdr/cdr_unanswered_yes' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="3.60">
> --> Running test 'tests/cdr/console_dial_sip_congestion' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="3.44">
> --> Running test 'tests/cdr/console_fork_after_busy_forward' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="33.25">
> --> Cannot run test 'tests/cdr/cdr_originate_sip_congestion_log'
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="0" time="0.00"/>
> --> Running test 'tests/cdr/console_fork_before_dial' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="33.21">
> --> Running test 'tests/cdr/cdr_userfield' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="33.21">
> --> Running test 'tests/cdr/nocdr' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="33.21">
> 
> After:
> 
> --> Running test 'tests/cdr/console_dial_sip_busy' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="4.55">
> --> Cannot run test 'tests/cdr/console_dial_sip_transfer'
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="0" time="0.00"/>
> --> Running test 'tests/cdr/console_dial_sip_answer' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="4.26">
> --> Running test 'tests/cdr/cdr_unanswered_yes' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="3.62">
> --> Running test 'tests/cdr/console_dial_sip_congestion' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="5.10">
> --> Running test 'tests/cdr/console_fork_after_busy_forward' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="5.05">
> --> Cannot run test 'tests/cdr/cdr_originate_sip_congestion_log'
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="0" time="0.00"/>
> --> Running test 'tests/cdr/console_fork_before_dial' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="7.78">
> --> Running test 'tests/cdr/cdr_userfield' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="4.06">
> --> Running test 'tests/cdr/nocdr' ...
> <testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="3.95">
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120116/a220079e/attachment.htm>