No subject
Fri Sep 2 03:59:05 CDT 2011
ry time. At first changing certain paths seemed to put it back on the right=
route, but as I started clearing away problems the whole thing started sta=
lling and I eventually just couldn't figure out why it was stalling, so I k=
inda assumed the reason the test was being skipped was because it was just =
not working rather than that it was bouncing. I've tested it a decent numbe=
r of times since then and it seems to be working now. I can go ahead and a=
dd a bit of a wait before sending the pound though, that's no problem.
If we want to actually make more significant changes to the test along the =
lines of how we check whether the mixmonitor has broken down once we've rea=
ched the end of the call, that's certainly doable.
...
In retrospect though, this test has been useless for determining the behavi=
or of audiohook_inherit from the get-go because it's using a blind transfer=
, so there isn't going to be a masquerade. There is never really any risk o=
f the mixmonitor falling off the call without having the audiohook inherit =
set anyway. I'll go ahead and change this to use an attended transfer. Oi.
> On Oct. 1, 2012, 3:38 p.m., Mark Michelson wrote:
> > /asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/run-test, line 3
> > <https://reviewboard.asterisk.org/r/2139/diff/1/?file=3D31588#file31588=
line3>
> >
> > If you're going to update the copyright, change it to be
> > =
> > 2010-2012
got it.
- jrose
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2139/#review7174
-----------------------------------------------------------
On Oct. 1, 2012, 1:31 p.m., jrose wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2139/
> -----------------------------------------------------------
> =
> (Updated Oct. 1, 2012, 1:31 p.m.)
> =
> =
> Review request for Asterisk Developers and Matt Jordan.
> =
> =
> Summary
> -------
> =
> This is part of an effort to get tests working that are currently being s=
kipped due to failures. I went ahead and changed this test to use the TestC=
ase class since the old way of doing things was driving me nuts with variou=
s problems.
> =
> =
> Diffs
> -----
> =
> /asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/test-config.yam=
l 3480 =
> /asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/configs/ast1/ex=
tensions.conf 3480 =
> /asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/configs/ast1/ma=
nager.conf 3480 =
> /asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/run-test 3480 =
> =
> Diff: https://reviewboard.asterisk.org/r/2139/diff
> =
> =
> Testing
> -------
> =
> To test the test I ran the test with normal conditions, with an improper =
filename specified for the mixmonitor command, and also with turning off au=
diohook_inherit. All deviations caused failure as expected.
> =
> =
> Thanks,
> =
> jrose
> =
>
--===============9151914637842142575==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
<html>
<body>
<div style=3D"font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor=3D"#f9f3c9" width=3D"100%" cellpadding=3D"8" style=3D"bor=
der: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href=3D"https://reviewboard.asterisk.org/r/2139/">https://reviewbo=
ard.asterisk.org/r/2139/</a>
</td>
</tr>
</table>
<br />
<blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; padd=
ing-left: 10px;">
<p style=3D"margin-top: 0;">On October 1st, 2012, 3:38 p.m., <b>Mark Miche=
lson</b> wrote:</p>
<blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
<pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So, a pro=
blem I have with this is that as far as I can tell, the mechanics of this t=
est are still exactly the same as they were previously. The previous test h=
ad a 60 second timeout built in that's not here, but the TestCase has i=
ts own timeout built in instead, so that's really not a change. My fear=
is that this version of the test will bounce just as the previous version =
did.
The logs from test failures are likely long gone and the 'skip' lin=
e in the test-config.yaml file gave no indication to you why the test was f=
ailing, so it's hard to guess what might have been the problem. However=
, there are a couple of things about this test that throw up red flags for =
me.
1) The blind transfer in the dialplan. This seems like something that could=
easily be "missed" by Asterisk since the # is sent immediately a=
fter answering the call. It may be that the test was failing due to the bli=
nd transfer never actually occurring. I would suggest waiting for a Bridge =
event from Asterisk and then issuing an AMI redirect at that point. That el=
iminates some of the potential randomness from the test and would simplify =
the dialplan.
2) The file size check. It seems like a very unsophisticated way to determi=
ne if the AUDIOHOOK_INHERIT worked as expected. We have tools like test_eve=
nts now that can be inserted into the code to tell for sure whether mixmoni=
tor has stopped or whether an audiohook has been moved as expected. We didn=
't have that back when this test was originally written. Something more=
explicit than a file size check would likely make this test more stable.
Finally, I disapprove of the removal of the debugging messages that previou=
sly existed in this test. Those sort of messages are very helpful when tryi=
ng to find out where or potentially why a test run has failed.</pre>
</blockquote>
</blockquote>
<pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-spac=
e: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">From my loc=
al runs of the test, it wasn't just bouncing, it was failing every time=
. At first changing certain paths seemed to put it back on the right route,=
but as I started clearing away problems the whole thing started stalling a=
nd I eventually just couldn't figure out why it was stalling, so I kind=
a assumed the reason the test was being skipped was because it was just not=
working rather than that it was bouncing. I've tested it a decent numb=
er of times since then and it seems to be working now. I can go ahead and =
add a bit of a wait before sending the pound though, that's no problem.
If we want to actually make more significant changes to the test along the =
lines of how we check whether the mixmonitor has broken down once we've=
reached the end of the call, that's certainly doable.
...
In retrospect though, this test has been useless for determining the behavi=
or of audiohook_inherit from the get-go because it's using a blind tran=
sfer, so there isn't going to be a masquerade. There is never really an=
y risk of the mixmonitor falling off the call without having the audiohook =
inherit set anyway. I'll go ahead and change this to use an attended t=
ransfer. Oi.</pre>
<br />
<blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; padd=
ing-left: 10px;">
<p style=3D"margin-top: 0;">On October 1st, 2012, 3:38 p.m., <b>Mark Miche=
lson</b> wrote:</p>
<blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
=
<table width=3D"100%" border=3D"0" bgcolor=3D"white" style=3D"border: 1px s=
olid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan=3D"4" bgcolor=3D"#F0F0F0" style=3D"border-bottom: 1px solid =
#C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href=3D"https://reviewboard.asterisk.org/r/2139/diff/1/?file=3D31588=
#file31588line3" style=3D"color: black; font-weight: bold; text-decoration:=
underline;">/asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/run-te=
st</a>
<span style=3D"font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style=3D"background-color: #e4d9cb; padding: 4px 8px; text-align: c=
enter;">
<tr>
<td colspan=3D"4"><pre style=3D"font-size: 8pt; line-height: 140%; margi=
n: 0; "></pre></td>
</tr>
</tbody>
=
=
<tbody>
<tr>
<th bgcolor=3D"#e9eaa8" style=3D"border-right: 1px solid #C0C0C0;" alig=
n=3D"right"><font size=3D"2">3</font></th>
<td bgcolor=3D"#fdfebc" width=3D"50%"><pre style=3D"font-size: 8pt; lin=
e-height: 140%; margin: 0; ">Copyright (C) 201<span class=3D"hl">0</span>, =
Digium, Inc.</pre></td>
<th bgcolor=3D"#e9eaa8" style=3D"border-left: 1px solid #C0C0C0; border=
-right: 1px solid #C0C0C0;" align=3D"right"><font size=3D"2">3</font></th>
<td bgcolor=3D"#fdfebc" width=3D"50%"><pre style=3D"font-size: 8pt; lin=
e-height: 140%; margin: 0; ">Copyright (C) 201<span class=3D"hl">2</span>, =
Digium, Inc.</pre></td>
</tr>
</tbody>
</table>
<pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If you=
9;re going to update the copyright, change it to be
2010-2012</pre>
</blockquote>
</blockquote>
<pre style=3D"margin-left: 1em; white-space: pre-wrap; white-space: -moz-pr=
e-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-=
word;"> got it.</pre>
<br />
<p>- jrose</p>
<br />
<p>On October 1st, 2012, 1:31 p.m., jrose wrote:</p>
<table bgcolor=3D"#fefadf" width=3D"100%" cellspacing=3D"0" cellpadding=3D"=
8" style=3D"background-image: url('https://reviewboard.asterisk.org/media/r=
b/images/review_request_box_top_bg.png'); background-position: left top; ba=
ckground-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and Matt Jordan.</div>
<div>By jrose.</div>
<p style=3D"color: grey;"><i>Updated Oct. 1, 2012, 1:31 p.m.</i></p>
<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Descripti=
on </h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
<tr>
<td>
<pre style=3D"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 is part of an effort to get tests working that are curr=
ently being skipped due to failures. I went ahead and changed this test to =
use the TestCase class since the old way of doing things was driving me nut=
s with various problems.</pre>
</td>
</tr>
</table>
<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing <=
/h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
<tr>
<td>
<pre style=3D"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;">To test the test I ran the test with normal conditions, with=
an improper filename specified for the mixmonitor command, and also with t=
urning off audiohook_inherit. All deviations caused failure as expected.</p=
re>
</td>
</tr>
</table>
<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b>=
</h1>
<ul style=3D"margin-left: 3em; padding-left: 0;">
<li>/asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/test-config.ya=
ml <span style=3D"color: grey">(3480)</span></li>
<li>/asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/configs/ast1/e=
xtensions.conf <span style=3D"color: grey">(3480)</span></li>
<li>/asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/configs/ast1/m=
anager.conf <span style=3D"color: grey">(3480)</span></li>
<li>/asterisk/trunk/tests/apps/mixmonitor_audiohook_inherit/run-test <span=
style=3D"color: grey">(3480)</span></li>
</ul>
<p><a href=3D"https://reviewboard.asterisk.org/r/2139/diff/" style=3D"margi=
n-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============9151914637842142575==--
More information about the asterisk-dev
mailing list