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

[Matt Jordan]

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



/asterisk/trunk/lib/python/asterisk/CDRTestCase.py
<https://reviewboard.asterisk.org/r/1901/#comment11286>

    Rename this to not be CDR specific - same with its methods.
    
    CallRecordTestCase?



/asterisk/trunk/lib/python/asterisk/CDRTestCase.py
<https://reviewboard.asterisk.org/r/1901/#comment11284>

    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.



/asterisk/trunk/lib/python/asterisk/CDRTestCase.py
<https://reviewboard.asterisk.org/r/1901/#comment11285>

    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.



/asterisk/trunk/lib/python/asterisk/astcsv.py
<https://reviewboard.asterisk.org/r/1901/#comment11288>

    I know this code is re-purposed, I'm not sure we should be trying to handle the exceptions here.  I'm all for catching it and logging it, but I think it might be better to re-throw it back to a higher layer rather then returning out of the constructor as if nothing happened.



/asterisk/trunk/lib/python/asterisk/cel.py
<https://reviewboard.asterisk.org/r/1901/#comment11289>

    I hope this isn't a CDR



/asterisk/trunk/lib/python/asterisk/cel.py
<https://reviewboard.asterisk.org/r/1901/#comment11290>

    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?



/asterisk/trunk/tests/cdr/cdr_accountcode/run-test
<https://reviewboard.asterisk.org/r/1901/#comment11291>

    I'd rename this test to reflect that it covers both CDR and CEL record matching.


- Matt


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/60e0f909/attachment-0001.htm>