<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/1951/">https://reviewboard.asterisk.org/r/1951/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 31st, 2012, 9:17 a.m., <b>opticron</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1951/diff/2/?file=28327#file28327line36" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/tests/channels/SIP/sip2cause/run-test</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">36</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> super(SIP2HangupCauseTest, self).ami_connect(ami)</pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This line shouldn't be necessary since you're creating the AMI factory in run() below.</pre>
</blockquote>
<p>On May 31st, 2012, 2:23 p.m., <b>wdoekes</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't think the AMI factory in run() is relevant.
https://reviewboard.asterisk.org/r/1606/ <-- Paul says I should add it
https://reviewboard.asterisk.org/r/1606/diff/2-3/
And calling it is a no-op (until someone implements useful functionality there):
class TestCase:
...
def ami_connect(self, ami):
"""
Hook method used after create_ami_factory() successfully logs into
the Asterisk AMI.
"""
pass
Out of 177 tests, I find *at* *least* 42 that do it:
$ find . -name run-test | wc -l
177
$ find . -name run-test | xargs grep def\ ami_connect -A1 | grep -F .ami_connect | wc -l
42
I don't mind removing the no-op, but I'd like some consensus first.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I agree that there's no need for this line and it should be removed. The fact that there are 42+ tests that have the line in it is not a good reason to continue putting the line in new tests. The reason the other tests still have this line in is likely because they were not updated once the line was no longer required.</pre>
<br />
<p>- Mark</p>
<br />
<p>On June 1st, 2012, 2:02 a.m., wdoekes 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 and opticron.</div>
<div>By wdoekes.</div>
<p style="color: grey;"><i>Updated June 1, 2012, 2:02 a.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;">See ASTERISK-19914 and preceding discussion in http://lists.digium.com/pipermail/asterisk-dev/2012-May/055309.html
Apparently r351130 broke correct HANGUPCAUSE setting in certain cases. This only tests the 404 case. Feel free to expand it later if needed.
P.S. There are now 3 test cases that concern hangupcauses. That might cause confusion:
hangupcause => tests ${HASH*(HANGUPCAUSE,*)}
sip_cause => tests ${HASH*(SIP_CAUSE,*)}
sip2cause => tests _getting_ ${HANGUPCAUSE} (and implicit propagation)</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;">Before chan_sip.diff from ASTERISK-19914 on 1.8.x.
<?xml version="1.0" encoding="utf-8"?>
<testsuite errors="0" failures="1" name="AsteriskTestSuite" tests="1" time="7.43">
<testcase name="tests/channels/SIP/sip2cause" time="7.43">
<failure>
Running ['tests/channels/SIP/sip2cause/run-test'] ...
[May 29 08:31:35] ERROR[14215]: __main__:69 on_hangup: Unexpected HANGUPCAUSE 34 (not 1) for channel SIP/alice-00000000
[May 29 08:31:40] ERROR[14215]: __main__:69 on_hangup: Unexpected HANGUPCAUSE 0 (not 1) for channel Local/1111@dial-alice-1c03;1
[May 29 08:31:40] ERROR[14215]: __main__:69 on_hangup: Unexpected HANGUPCAUSE 34 (not 1) for channel Local/1111@dial-alice-1c03;2
</failure>
</testcase>
</testsuite>
After chan_sip.diff on 1.8.x (and on 1.6.2.x):
<?xml version="1.0" encoding="utf-8"?>
<testsuite errors="0" failures="0" name="AsteriskTestSuite" tests="1" time="12.54">
<testcase name="tests/channels/SIP/sip2cause" time="12.54"/>
</testsuite>
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-19914">ASTERISK-19914</a>
</div>
<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/sip2cause/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip2cause/configs/ast1/sip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip2cause/run-test <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip2cause/sipp/invite_recv_do_404.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip2cause/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/tests.yaml <span style="color: grey">(3247)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1951/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>