> 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. > > Ashley Sanders wrote: > 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 ---
That's fair enough. It looked like for the couple of test cases you had implemented you could get away with using one of the current ARI test objects as you can register to receive various events and you should also have access to the websocket in order to disconnect it when needed, but it sounds like you are still adding scenarios that might not fit as neatly. So I agree with Matt that merging that functionality (making it generic) for those changes into the current classes can be done later. - Kevin ----------------------------------------------------------- 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 > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
