[asterisk-dev] [Code Review] 3501: testsuite: add tests for ari userevents

[Matt Jordan]

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



/asterisk/trunk/lib/python/asterisk/ari.py
<https://reviewboard.asterisk.org/r/3501/#comment21665>

    Since the rest of this module uses full pydoc with doxygen-esque markup, follow the same nomenclature



/asterisk/trunk/lib/python/asterisk/ari.py
<https://reviewboard.asterisk.org/r/3501/#comment21664>

    Nit pick: pydoc comments are formatted with the summary on the same line as the beginning of the comment with no spaces:
    
    """Send an arbitrary request to ARI ...
    """



/asterisk/trunk/lib/python/asterisk/ari.py
<https://reviewboard.asterisk.org/r/3501/#comment21666>

    I'm not sure this method is needed.
    
    If an error response is allowed, then setting allow_errors = True on this class will allow the existing request method to simply return the response. The module doing the request can then verify that the response is what it wants.



/asterisk/trunk/lib/python/asterisk/ari.py
<https://reviewboard.asterisk.org/r/3501/#comment21667>

    I'd handle this slightly differently.
    
    Rather than use a separate method, I'd simply put a try: except: around the call to request. Have the except handle a requests.exceptions.HTTPError.
    
    If the error is the expected response code, simply fall out gracefully. If not, re-throw the error.



/asterisk/trunk/tests/rest_api/applications/userevents/channel/test-config.yaml
<https://reviewboard.asterisk.org/r/3501/#comment21668>

    That sounds ominous. Racing == bad.
    
    You may want to structure this such that we only get the user event after we've subscribed to the channel. One way of doing that would be spawn the channel into the Stasis application, subscribe to it, then release it to the dialplan that raises the UserEvent.



/asterisk/trunk/tests/rest_api/applications/userevents/channel/test-config.yaml
<https://reviewboard.asterisk.org/r/3501/#comment21670>

    12.3.0



/asterisk/trunk/tests/rest_api/applications/userevents/invalid/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/3501/#comment21673>

    This could get racey (and forces the test to run for 3 seconds, when it may be done faster)
    
    I'd instead have this get dropped into an Echo, and have the test itself hangup the channel when done.



/asterisk/trunk/tests/rest_api/applications/userevents/invalid/test-config.yaml
<https://reviewboard.asterisk.org/r/3501/#comment21669>

    Blob



/asterisk/trunk/tests/rest_api/applications/userevents/invalid/test-config.yaml
<https://reviewboard.asterisk.org/r/3501/#comment21671>

    Another way of handling expected request failures would be to expose the allow_errors attribute on the ARI object through the test object controlling it. Your test-object-config could set that, then have the event matcher do its thing.
    
    That would be potentially useful for a larger number of tests, and would keep the control of the expected response in the test object.



/asterisk/trunk/tests/rest_api/applications/userevents/invalid/test-config.yaml
<https://reviewboard.asterisk.org/r/3501/#comment21672>

    12.3.0



/asterisk/trunk/tests/rest_api/applications/userevents/multi/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/3501/#comment21675>

    Egads, 25 seconds?
    
    This should be made to be event driven.



/asterisk/trunk/tests/rest_api/applications/userevents/multi/test-config.yaml
<https://reviewboard.asterisk.org/r/3501/#comment21674>

    12.3.0


- Matt Jordan


On April 29, 2014, 2:27 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3501/
> -----------------------------------------------------------
> 
> (Updated April 29, 2014, 2:27 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22697
>     https://issues.asterisk.org/jira/browse/ASTERISK-22697
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> Tests added:
> 
> 1) userevents/channel - check that the dialplan app Userevent() generated event produces correct ARI/AMI events
> 
> 2) userevents/multi - check that various forms of the ARI userevent generate correct ARI & AMI events
> 
> 3) userevents/invalid - check that correct result code is returned for invalid input cases
> 
> Changes to ari.py made to add expected: ### return code checking for HTTP requests.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/rest_api/applications/userevents/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/userevents/multi/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/userevents/multi/configs/ast1/pjsip.conf PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/userevents/multi/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/userevents/invalid/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/userevents/invalid/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/userevents/channel/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/userevents/channel/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/applications/tests.yaml 5006 
>   /asterisk/trunk/lib/python/asterisk/ari.py 5006 
> 
> Diff: https://reviewboard.asterisk.org/r/3501/diff/
> 
> 
> Testing
> -------
> 
> Tests pass with userevent code.  Also tested with r3496 valgrind support to insure no invalid references.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140502/28f669d8/attachment-0001.html>