[asterisk-dev] [Code Review]: Add CEL testing to the testsuite

Terry Wilson reviewboard at asterisk.org
Mon May 7 16:39:50 CDT 2012 


> On May 6, 2012, 9:46 p.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/asterisk/CDRTestCase.py, line 24
> > <https://reviewboard.asterisk.org/r/1901/diff/1/?file=27682#file27682line24>
> >
> >     Rename this to not be CDR specific - same with its methods.
> >     
> >     CallRecordTestCase?

I've decided to just completely leave out "CELTestCase"-like support for now. CDRTestCase is really only good for testing cdr-specific features like setting unanswered=yes, using NoCDR, etc. Much better, IMHO, is to add a few lines to existing tests to make sure that we get the right CDR/CEL records. Those existing tests have the complex call scenarios that really should be tested. CDRTestCase does Local channel calls into the dialplan and is just not very flexible. CEL doesn't really have a lot of options like that to test so there really isn't any benefit to adding support for it.


> On May 6, 2012, 9:46 p.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/asterisk/CDRTestCase.py, lines 83-84
> > <https://reviewboard.asterisk.org/r/1901/diff/1/?file=27682#file27682line83>
> >
> >     Rather then treating CDR/CEL as separate record types, have AsteriskCSVCDRLine/AsteriskCSVCELLine and AsteriskCSVCDR/AsteriskCSVCEL provide the same method signatures.
> >     
> >     This eliminates the need for instanceof, multiple record lists, and the other type based code you've had to introduce here.  You'll need to change how they're created a bit later on.  One approach could be to use factory objects set by the derived classes.

See above.


> On May 6, 2012, 9:46 p.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/asterisk/CDRTestCase.py, lines 108-109
> > <https://reviewboard.asterisk.org/r/1901/diff/1/?file=27682#file27682line108>
> >
> >     For the creation of the CDR/CEL expectations and the CDR/CEL file, you could use a factory object to create the appropriate types of expectations/files.  These could, again, provide the same method signature.  Each test case class can be responsible for setting the factory to the correct type.  So you'd have something like:
> >     
> >     CDRConcreteTestCase(CallRecordTestCase)
> >     def __init__(self):
> >       CallRecordTestCase.__init__(self)
> >       self.expect_factory = AsteriskCDRExpectFactory()
> >       self.file_factory = AsteriskCDRFileFactory()
> >     
> >       # initialization and what not ...
> >     
> >     CallRecordTestCase(TestCase)
> >     def __init__(self):
> >       TestCase.__init__(self)
> >       self.expect_factory = None
> >       self.file_factory = None
> >       # Rest of existing init...
> >     
> >     def match_records(self):
> >       """ Replacement name for match_cdrs """
> >     
> >       # <snip>
> >     
> >       if records and self.expect_factory and self.file_factory:
> >         expect = self.expect_factory.create_expectations(records=records)
> >         file = file_factory.create_record_file(fn = filepath)
> >     
> >         # etc.

And above.


> On May 6, 2012, 9:46 p.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/asterisk/cel.py, line 24
> > <https://reviewboard.asterisk.org/r/1901/diff/1/?file=27685#file27685line24>
> >
> >     I hope this isn't a CDR

Fixed.


> On May 6, 2012, 9:46 p.m., Matt Jordan wrote:
> > /asterisk/trunk/tests/cdr/cdr_accountcode/run-test, line 19
> > <https://reviewboard.asterisk.org/r/1901/diff/1/?file=27691#file27691line19>
> >
> >     I'd rename this test to reflect that it covers both CDR and CEL record matching.

I'm no longer even adding CEL testing to this test. (although it should be renamed since it is a test of accountcode and not userfield).


> On May 6, 2012, 9:46 p.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/asterisk/cel.py, lines 66-73
> > <https://reviewboard.asterisk.org/r/1901/diff/1/?file=27685#file27685line66>
> >
> >     chan_console is extended support and has some dependencies that may not always be present on all test systems.  Is there anyway this can not use that channel type?

Console/dsp is just what happens to be in the sample records I dumped during a call for purposes of self-tests. It could read TinCan/string and work fine if we wanted it to--and we do.


- Terry


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


On May 5, 2012, 12:32 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1901/
> -----------------------------------------------------------
> 
> (Updated May 5, 2012, 12:32 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Summary
> -------
> 
> This moves the CSV processing out of cdr.py into astcsv.py. CDR and CEL inherit from there. CDRTestCase is modified to allow passing expectations for CEL as well. When using CDRTestCase, one has to put in entries for *all* expected CDR or CEL entries. For CEL, that is a *lot* of entries. Instead of using CDRTestcCase to just test CDR/CELs (and only test them with Local channels), just adding a few lines of code to other existing tests might really be the best use of the CDR/CEL modules since we want to make sure CDR/CELs are correct for a wide array of situations.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/lib/python/asterisk/CDRTestCase.py 3204 
>   /asterisk/trunk/lib/python/asterisk/astcsv.py PRE-CREATION 
>   /asterisk/trunk/lib/python/asterisk/cdr.py 3204 
>   /asterisk/trunk/lib/python/asterisk/cel.py PRE-CREATION 
>   /asterisk/trunk/lib/python/asterisk/self_test/CELMaster1.csv PRE-CREATION 
>   /asterisk/trunk/lib/python/asterisk/self_test/CELMaster2.csv PRE-CREATION 
>   /asterisk/trunk/runtests.py 3204 
>   /asterisk/trunk/tests/cdr/cdr_accountcode/configs/ast1/cel.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_accountcode/configs/ast1/cel_custom.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_accountcode/run-test 3204 
> 
> Diff: https://reviewboard.asterisk.org/r/1901/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Terry
> 
>

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

More information about the asterisk-dev mailing list