[asterisk-dev] Change in testsuite[master]: Add SIP attended transfer for Asterisk 11.

[Ashley Sanders (Code Review)]

Ashley Sanders has posted comments on this change.

Change subject: Add SIP attended transfer for Asterisk 11.
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

I think that most of this could be collapsed into the logic for the test for 12+. The AMI/bridging logic seems to be the only difference between the two versions. I think applying the strategy pattern will solve the problem of needing two tests for two disparate AMI APIs. 

This will help future-proof your code in that if any maintenance is required in the future, it will be a lot easier to apply that logic to one place rather than to n places :)

https://gerrit.asterisk.org/#/c/20/1/tests/channels/SIP/sip_attended_transfer_11/attended_transfer.py
File tests/channels/SIP/sip_attended_transfer_11/attended_transfer.py:

After comparing this version against the version for the 12+ branch, the only significant difference is with respect to the ami bridging logic. I think you could just pull that piece out into it's own module and inject one strategy or the other based on the currently running asterisk version. 

This approach would greatly reduce the amount of code needed to test this feature.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I48c7b6a9298552aa756d0c2f26afbd6a96d553b5
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-HasComments: Yes