[asterisk-dev] [Code Review] 3105: Testsuite: Test PJSIP hold and unhold for various conditions for INVITE SDPs

[Mark Michelson]

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

Ship it!


Your findings regarding the to-tag with chan_sip.c caused me to have a look at the code to see why the old tests were passing.

Dialog-matching in chan_sip is...weird. For BYE, INFO, and ACK requests, the lack of a to-tag results in an immediate rejection of the request. For other types of requests, we can match dialogs if there is no to-tag; however, we will not match an existing dialog if the incoming request contains a to-tag that does not match our local tag. This means that the chan_sip SIPp scenarios worked, even though the reinvites should have contained to-tags.

The changes you made here for the pjsip tests could be done to the chan_sip tests, but I don't think that it's a high-priority change to make.

- Mark Michelson


On Jan. 6, 2014, 9:59 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3105/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2014, 9:59 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp, Matt Jordan, and Mark Michelson.
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> Creates tests similar to the channels/SIP/sip_hold tests. Using the PJSIP channel driver, the following scenarios are tested:
> 
> 1. Put a call on hold by setting media attributes to sendonly. Unhold by setting media back to sendrecv
> 2. (new) as above, but unhold by simply not including an SDP (some devices are known to do this apparently and a patch is on reviewboard to handle that scenario in PJSIP).
> 3. Set on hold by setting the contact IP to 0.0.0.0, return to normal by providing normal SDP
> 4. Combine both the IP hold and media restriction hold methods.
> 
> There are a few noteworthy differences here to the original tests in SIP hold that might require some additional attention.
> 
> First, simply running the SIPP scenarios as is with PJSIP yielded an inability to match the new INVITES to the existing PJSIP dialogs. In addition to one invite from each scenario appearing to point to the wrong peer in the invite used to resume/unhold the call, the appropriate To tag was left out of the reinvites for both holding and unholding. I'm not sure how this worked in the first place.
> 
> Second, I noticed that verifying for hold/unhold behavior in the test script is performed by checking for Music On Hold start and stop events.  This mostly works for hold, but for unhold in particular it's unreliable since music on hold stop events will also be issued if the call simply ends. Because of that, I went ahead and added listeners for Hold and Unhold events which are required in a similar fashion to the existing MOH start and stop events.
> 
> I'm willing to apply these changes to the SIP hold test, but this does appear to be a noteworthy variance in behavior which needs to be taken note of... particularly the differences in the SIPP scenarios that needed to be made.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/channels/pjsip/tests.yaml 4539 
>   /asterisk/trunk/tests/channels/pjsip/hold/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_unhold_sans_sdp.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_media_restrict.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_restrict.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_media_restrict.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_A.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/sipp/inject.csv PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/run-test PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/pjsip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/extensions.conf PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3105/diff/
> 
> 
> Testing
> -------
> 
> Ran the tests. Checked output of user events in Asterisk. Confirmed that without the patch in 3106 that this test fails due to not receiving an Unhold event for the second scenario described above.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140107/de404963/attachment-0001.html>