[asterisk-dev] [Code Review] Add tests for SIP_CAUSE and its replacement

Matt Jordan reviewboard at asterisk.org
Tue Mar 27 08:45:30 CDT 2012 

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



asterisk/trunk/lib/python/asterisk/SimpleTestCase.py
<https://reviewboard.asterisk.org/r/1832/#comment10752>

    I should have caught this when this initially went in, but an all-caps object isn't really compliant with standard python nomenclature.  Granted, we've got other modules, classes, and methods that don't really follow standard nomenclature either, but we should start trying to follow it better.



asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10760>

    Remove



asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10761>

    These should be:
    NoOp()
    Answer()



asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10762>

    Change to Answer() and Dial(...) here



asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast2/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10763>

    For readability, add a line between each of these contexts



asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast2/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10764>

    Answer() and Echo() here



asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast2/sip.conf
<https://reviewboard.asterisk.org/r/1832/#comment10765>

    I think the call completion settings here are probably held over from another test



asterisk/trunk/tests/channels/SIP/hangupcause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10754>

    Same finding here regarding all caps



asterisk/trunk/tests/channels/SIP/hangupcause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10755>

    Initialize these in the constructor (__init__).  Otherwise, they're scoped at the class level, which - while it won't cause any problems in your test - probably aren't the semantics you intended



asterisk/trunk/tests/channels/SIP/hangupcause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10756>

    You can just use the originate failure handler in the TestCase class



asterisk/trunk/tests/channels/SIP/hangupcause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10757>

    If you feel like it, you technically don't need to actually hang on to the deferred object in a local variable here.  You can, if you want, just do the following:
    
    self.ami[0].originate("local...", "dpwait", "1234", 1).addErrback(self.handleOriginateFailure)
    
    



asterisk/trunk/tests/channels/SIP/hangupcause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10758>

    You no longer need the start/stop asterisk pair



asterisk/trunk/tests/channels/SIP/hangupcause/test-config.yaml
<https://reviewboard.asterisk.org/r/1832/#comment10759>

    Since the name of these frames is very likely to change, you may want to rephrase the description.



asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10770>

    NoOp(), Answer(), Echo()



asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10771>

    Answer(), Dial() here



asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast2/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10773>

    New line between extensions here as well



asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast2/extensions.conf
<https://reviewboard.asterisk.org/r/1832/#comment10774>

    Answer(), Echo()



asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast2/sip.conf
<https://reviewboard.asterisk.org/r/1832/#comment10772>

    Call Completion settings here as well



asterisk/trunk/tests/channels/SIP/sip_cause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10766>

    Caps lock



asterisk/trunk/tests/channels/SIP/sip_cause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10776>

    Initialize these in the constructor (__init__).  Otherwise, they're scoped at the class level, which - while it won't cause any problems in your test - probably aren't the semantics you intended



asterisk/trunk/tests/channels/SIP/sip_cause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10767>

    Use the TestCase originate failure handler



asterisk/trunk/tests/channels/SIP/sip_cause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10768>

    If you want, you can also chain the .addErrback onto the originate calls here as well



asterisk/trunk/tests/channels/SIP/sip_cause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10775>

    To maintain the differentiation between the two tests, you may want to rename this to sc_callback



asterisk/trunk/tests/channels/SIP/sip_cause/run-test
<https://reviewboard.asterisk.org/r/1832/#comment10769>

    Start and stop here as well



asterisk/trunk/tests/channels/SIP/tests.yaml
<https://reviewboard.asterisk.org/r/1832/#comment10753>

    Naming convention would suggest that this should be 'hangup_cause'


- Matt


On March 26, 2012, 4:07 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1832/
> -----------------------------------------------------------
> 
> (Updated March 26, 2012, 4:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Add tests for SIP_CAUSE and its replacement (currently named HANGUPCAUSE, may be something else later).  Both tests check the non-hash HANGUPCAUSE variable to ensure that no namespace collision occurs and include tests for single channel dials, branched dials, and local channel indirected branched dials.
> 
> 
> This addresses bug SWP-4223.
>     https://issues.asterisk.org/jira/browse/SWP-4223
> 
> 
> Diffs
> -----
> 
>   asterisk/trunk/lib/python/asterisk/SimpleTestCase.py 3121 
>   asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast1/extensions.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast1/sip.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast2/extensions.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/hangupcause/configs/ast2/sip.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/hangupcause/run-test PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/hangupcause/test-config.yaml PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast1/extensions.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast1/sip.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast2/extensions.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/sip_cause/configs/ast2/sip.conf PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/sip_cause/run-test PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/sip_cause/test-config.yaml PRE-CREATION 
>   asterisk/trunk/tests/channels/SIP/tests.yaml 3121 
> 
> Diff: https://reviewboard.asterisk.org/r/1832/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> opticron
> 
>

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

More information about the asterisk-dev mailing list