<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/2029/">https://reviewboard.asterisk.org/r/2029/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">My main beef with this test is that with the changes you made in response to Paul's review, the test appears as though it is impossible to fail. You set the passed variable to True in SDPPassthrough.__init__() and then there is never a point afterwards where it can be set to False.
Even if you were to change the failure conditions back to checking whether sipp_a.passed and sipp_b.passed are true, this is not enough. The reason is that you could run, say, the H.263 test and encounter a failure, but then you could run the H.264 test and its passing would overwrite the failure of the previous test. The test would end up passing even though one of the test calls failed.
Of course, I could be wrong about this statement if the test immediately bombs out when a SIPp scenario fails.</pre>
<br />
<p>- Mark</p>
<br />
<p>On July 10th, 2012, 2:30 p.m., Joshua Colp wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Joshua Colp.</div>
<p style="color: grey;"><i>Updated July 10, 2012, 2:30 p.m.</i></p>
<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;">This test ensures that attribute information is transported through Asterisk.</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 it, checked to make sure it worked okay. Tweaked test to fail and made sure it failed as expected.</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/SIP/SDP_attribute_passthrough/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/configs/ast1/sip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/run-test <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_A_h263.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_A_h264.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_A_speex.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_B_h263.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_B_h264.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_B_speex.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/tests.yaml <span style="color: grey">(3307)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2029/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>