<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3105/">https://reviewboard.asterisk.org/r/3105/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Joshua Colp, Matt Jordan, and Mark Michelson.</div>
<div>By Jonathan Rose.</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
testsuite
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/asterisk/trunk/tests/channels/pjsip/tests.yaml <span style="color: grey">(4539)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_unhold_sans_sdp.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_media_restrict.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_restrict.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_media_restrict.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_A.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/sipp/inject.csv <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/run-test <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/pjsip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3105/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>