[Asterisk-code-review] apps: Add a test for the BridgeAdd application (testsuite[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: apps:  Add a test for the BridgeAdd application
......................................................................


Patch Set 1: Code-Review-1

(6 comments)

https://gerrit.asterisk.org/#/c/3569/1/tests/apps/bridge/bridge_add/configs/ast1/extensions.conf
File tests/apps/bridge/bridge_add/configs/ast1/extensions.conf:

Line 3: exten = alice,1,Dial(Local/bob at default)
Should have an explicit hangup after the dial.  Otherwise you get a warning about an implicit dialplan continuation hangup because you fell off the end of dialplan.


Line 10: exten = bridgeadd,1,BridgeAdd(Local/bob at default-)
Wow.  This is ambiguous.  You currently have four channels that will match and only two of them will work because they are in a bridge.  You would appear to be lucky that it finds one of the bridged channels if this test actually passes.


https://gerrit.asterisk.org/#/c/3569/1/tests/apps/bridge/bridge_add/test-config.yaml
File tests/apps/bridge/bridge_add/test-config.yaml:

PS1, Line 4:         * We call alice
           :         * alice calls bob
           :         * bob answers and starts the AMD app
           :         * When bob enters the bridge, we call charlie
           :         * when charlie answers, bridgeadd is is called
           :         * bridgeadd calls BridgeAdd to bob's channel
           :         * charlie plays a human simulation
           :         * bob's amd detects HUMAN
           :         * charlie's playback finishes and now he starts AMD
           :         * bob's amd finishes and now he plays a human simulation
           :         * charlie's amd detects HUMAN
s/amd/AMD/g


PS1, Line 33:         ami-actions:
            :             action:
            :                 Action: 'Originate'
            :                 ActionID: '12345'
            :                 Channel: 'Local/alice at default'
            :                 Exten: 'alice'
            :                 Context: 'default'
            :                 Priority: '1'
            :                 Codecs: 'gsm,ulaw'
This is strange.  You are going to wind up with this local channel call chain:

AMD -- bob-00000002;2 -- bob-00000002;1 -- bridge2 -- alice-00000000;1 -- alice-00000000;2 -- bridge1 -- bob-00000001;1 -- bob-00000001;2 -- AMD


PS1, Line 51:             action:
            :                 Action: 'Originate'
            :                 ActionID: '12345'
            :                 Channel: 'Local/charlie at default'
            :                 Exten: 'charlie'
            :                 Context: 'default'
            :                 Priority: '1'
            :                 Codecs: 'gsm,ulaw'
            :                 Application: 'Dial'
            :                 Data: 'Local/bridgeadd at default'
Originating with both contect/exten/priority and application/data specified as the origination answer target is not good since you are specifying both methods.  It needs to be one or the other.

Ugh. It looks like the AMI Originate action is buggy when dealing with the context/exten/priority header values.  It also looks like it has always been broken.  Those values are always required and must point to a valid dialplan location.  They should be optional in case you want to use the application/data headers.


PS1, Line 83:     dependencies:
            :         - asterisk : 'app_bridgeaddchan'
app_dial is missing from the dependencies list.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d51eb1495d13ecf840a055bfbe918832381f1b
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes