[asterisk-dev] [Code Review] 4520: Testsuite: stasis: set a channel variable on websocket disconnect error

[Ashley Sanders]

> On March 24, 2015, 1:15 p.m., Kevin Harwell wrote:
> > ./asterisk/trunk/tests/rest_api/applications/stasis_status/test_case.py, lines 22-23
> > <https://reviewboard.asterisk.org/r/4520/diff/1/?file=72750#file72750line22>
> >
> >     A lot of the code in this object as well as others (AriClient, ObservableObject) looks similar to much of the code found in the AriTestObject or AriBaseTestObject.
> >     
> >     I'm thinking you can inherit instead from one of those two ARI test objects and simplify things greatly, overriding/adding methods where needed. You might even be able to get away with using the WebSocketEventModule and handling things like adding a channel to a bridge or disconnecting the websocket in raised callbacks. At least for a couple of these scenarios. For that though you would have to break the scenarios into separate tests.

The issue with using the AriBaseTestObject (or any of its derived types) was that there are certain events that I must intercept to start monitoring for the channel variable in unusual ways. Also, I have the need to exercise ARI after I blow away the websocket out from underneath it.

Having said that, there are many things in these classes that could be pulled up into one of the generalized classes (e.g. the observer logic, the channel variable monitor, the base test scenario class and the test case itself - assuming I change its name to something more generic.)

At this point, I think I am going to defer to Matt's comment from another issue in this review (in a nutshell, he stated that pulling everything up at this point is overkill):
--- mjordan comment begin here ---
   I don't disagree that this *could* go in that direction. But for it to be truly generic, you'd want to:
   (a) As you noted, change some of the naming and place it in lib/python/asterisk
   (b) Have it be a test object in the pluggable framework, which would have to take some careful thought about how it could be configured to look at a particular
       set of scenarios. Generally, once we have a 'generic' piece of functionality, we'd want for it to be instantiated via the TestRunner, and then have either 
       other pluggable modules attach into it, or have small Python modules provided by the test attach to it.

   I do think having the scenario factory function get put into here would be fine. 

   Overall, once you've got a reason to pull it out of run-test, I'm not against it - but I'm not sure this review is at that point.

   The other direction you could take would be fully pull it out in this review, make it generic, and put it in lib/python/asterisk. But that might be more work
   than is necessary at this point, and since we don't have a second test (yet!) that would make use of it, I'm not sure it is warranted.
--- mjordan comment end here ---


- Ashley


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


On March 27, 2015, 10:58 a.m., Ashley Sanders wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4520/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 10:58 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24802
>     https://issues.asterisk.org/jira/browse/ASTERISK-24802
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis).
> 
> This patch introduces a new channel variable: STASISSTATUS to give outside applications context when errors occur in Stasis that interrupt normal processing.
> 
> This test exercises two scenarios to elicit updates to the STASISSTATUS channel variable:
> 1) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied.
> 2) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
> 
>  ***Note*** This is a test. It is only a test. The review for the Asterisk source can be found at: https://reviewboard.asterisk.org/r/4519/
> 
> 
> Diffs
> -----
> 
>   ./asterisk/trunk/tests/rest_api/applications/tests.yaml 6547 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/test_scenario_factory.py PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/test_scenario.py PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/test_case.py PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/test-config.yaml PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/run-test PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/observable_object.py PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/monitor.py PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/configs/ast1/sip.conf PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/configs/ast1/extensions.conf PRE-CREATION 
>   ./asterisk/trunk/tests/rest_api/applications/stasisstatus/ari_client.py PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/4520/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashley Sanders
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150327/1bfdf4b6/attachment.html>