[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Matt Jordan (Code Review)]

Matt Jordan has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
......................................................................


Patch Set 8:

(3 comments)

https://gerrit.asterisk.org/#/c/18/8/tests/rest_api/applications/stasisstatus/ari_client.py
File tests/rest_api/applications/stasisstatus/ari_client.py:

Line 94:         return wait_for_it()
wait_for_it() does not appear to return anything. On the other hand, self.__reset() does return either a deferred, or the return value of self.__tear_down::wait_for_it().

If wait_for_it doesn't return anything, then I would just invoke it and have connect_websocket not return anything. If it should return something - such as a deferred that fires when the websocket is connected - then I'd return that deferred from wait_for_it.

In the latter case, you'd also want to document that it returns such a deferred in the documentation for connect_websocket.


Line 358:             Returns:
        :             The a twisted.defer instance.
This doesn't appear to actually be returning the twisted.defer that it is documented to return.


https://gerrit.asterisk.org/#/c/18/8/tests/rest_api/applications/stasisstatus/test-config.yaml
File tests/rest_api/applications/stasisstatus/test-config.yaml:

Line 31:         - Application
Since none of the other application ARI tests use this tag, please remove it.


-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 8
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes