[asterisk-dev] [Code Review]: Testsuite: CDRTestCase and tests/cdr/originate_sip_congestion_log

Tue Aug 9 14:34:28 CDT 2011

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/run-test, line 39
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17813#file17813line39>
> >
> >     You don't need to pass any arguments, these should be the defaults.

Alrighty, removing those arguments.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/manager/event-monitor/configs/ast1/manager.conf, line 1
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17817#file17817line1>
> >
> >     same comment as before, don't override this file and use the #include statement.

This file shouldn't even be touched at all.  Purely a mistake on my part.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/test-output.txt, lines 12-26
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17815#file17815line12>
> >
> >     Do you still get this traceback?

Yes, I get this traceback because the originate event returns an error and this doesn't get handled by starpy.  It doesn't have any impact on the failure or success of the test though.  Don't currently know if this means a change to starpy is needed, but if it's something that needs to be fixed, it probably needs to be handled within starpy.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/test-config.yaml, line 9
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17814#file17814line9>
> >
> >     Why 1.12?

It's trunk only. I guess that means I should put "11.0" in this field.  Back when I first started working on this, the whole 1.10/2/10 thing hadn't been fully resolved yet.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/test-config.yaml, line 13
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17814#file17814line13>
> >
> >     remove

Oh, right.  Oversight.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/configs/ast2/sip.conf, line 3
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17812#file17812line3>
> >
> >     same, 127.0.0.2

Got it.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/configs/ast1/sip.conf, line 3
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17809#file17809line3>
> >
> >     loopback adapter (127.0.0.1)

Got it.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/lib/python/asterisk/CDRTestCase.py, line 40
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17805#file17805line40>
> >
> >     blob

Got the blob.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/lib/python/asterisk/CDRTestCase.py, line 91
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17805#file17805line91>
> >
> >     blob

Got the other blob.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/configs/ast2/cdr.conf, line 1
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17810#file17810line1>
> >
> >     This file is broken, no [general] header.

Got it.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/lib/python/asterisk/CDRTestCase.py, line 78
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17805#file17805line78>
> >
> >     Hard-coded paths should be avoided.  You should be able to access the astlogdir directory from:
> >     
> >     ast1.directories['astlogdir']

Neat.

> On Aug. 9, 2011, 11:48 a.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/cdr/originate_sip_congestion_log/configs/ast1/manager.conf, line 1
> > <https://reviewboard.asterisk.org/r/1357/diff/1/?file=17808#file17808line1>
> >
> >     We should not be over writing the base manager.conf file, you should create a manager.users.conf.inc file for theses settings.

Got it.  Had to use a manager.general.conf.inc as well though.

- jrose

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

On Aug. 9, 2011, 2:34 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1357/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2011, 2:34 p.m.)
> 
> 
> Review request for Asterisk Developers and Paul Belanger.
> 
> 
> Summary
> -------
> 
> This is a test suite addition for performing tests involving CDRs.  It should be possible now to perform most of the CDR tests simply by using AMI commands to control the call and have some CDR info supplied before hand and the whole test should just work without much input beyond that.
> 
> The test included which uses the CDRTestCase requires a patch that is still in review from:
> https://reviewboard.asterisk.org/r/1331/
> 
> If this change is OK'd, I'll go ahead and start working on a major conversion project to change most of the existing python CDR tests into CDRTestCase equivalents.
> 
> Note about CDRTestCase - It is a subclass of TestCase, so it has most of the functionality from that.  In addition, it holds CDRLines specific to individual instances of Asterisk that are running and makes adding expected lines and checking them as simple as a single function call containing the CDRLine and an index.  It should also be pretty easy to adapt the behavior for more elaborate needs.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/cdr/tests.yaml 1822 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/run-test PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast2/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast2/cdr.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast2/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast1/manager.users.conf.inc PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast1/manager.general.conf.inc PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/lib/python/asterisk/CDRTestCase.py PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/configs/ast1/cdr.conf PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/1357/diff
> 
> 
> Testing
> -------
> 
> The bulk of the testing was just making sure the add method and the match methods worked properly.  There was also getting the test itself working.  I'm pretty confident that this is fine as is.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110809/01c4d807/attachment-0001.htm>