<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/3329/">https://reviewboard.asterisk.org/r/3329/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 13th, 2014, 11:13 p.m. UTC, <b>Mark Michelson</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;">One thing I notice about this test is that it is structured to call the first extension, then when that finishes, call the second, then when that finishes, call the third, etc.
Since no call depends on any of the previous call results, I think this test could be changed to originate all the calls at the same time and evaluate the UserEvents and Hangups as they arrive. This would have a few benefits:
1) The test will execute more quickly
2) You can get rid of the can_call member of your test class.
3) Your AMI hangup handler will be simplified greatly since you won't need to have a big if-else ladder to figure out what to do next. In fact, you may be able to just get rid of it altogether.
Doing it this way kind of screws up your detected member, though. As a simple workaround, in your new_exten_event_handler, you can determine whether to send the AMI hangup by checking event['AppData'] for the name of the AGI script. If event['AppData'] is 'waiting.agi' or 'executing.agi' then you issue the AMI hangup.</pre>
</blockquote>
<p>On March 14th, 2014, 12:47 a.m. UTC, <b>rmudgett</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;">Doing tests serially allows them to be debugged easier. Having all the tests run simultaneously makes it difficult to figure out which event is a result of which test. This is especially true if unexpected events happen that are a clue to what is going wrong.</pre>
</blockquote>
<p>On March 14th, 2014, 2:02 p.m. UTC, <b>Mark Michelson</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;">It's only difficult to debug if the test does a poor job of documenting what it has done and what went wrong. AMI events, especially in 12, give detailed information in them, and so you should have the necessary tools to be able to print out exactly what the failure was ("For this case, I was expecting this but I got that instead").
I agree that in the early stages of testing, running test cases individually is a good idea to be sure that each one operates as you expect, but once you've determined that your individual test cases are working, then if the test cases do not interact with each other, then parallelization is the way to go.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Having this test originate all channels simultaneously would make things a little more difficult to debug as Richard was saying, because of ASTERISK-23390. The AGI application event shows up twice currently, one time when calling the script, and another time when the script is finished running / not found / etc. This is why I have the variable "detected" in there currently, and if I ran everything at the same time, I would need two of these variables instead of one, to make sure that I only hangup these channels once. Otherwise all the "No such channel" errors pop up. I could make the change now, and have two variables, and that would potentially make it easier to make the entire change once the bug is fixed. Suggestions?</pre>
<br />
<p>- Benjamin</p>
<br />
<p>On March 14th, 2014, 2:43 p.m. UTC, Benjamin Keith Ford wrote:</p>
<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.</div>
<div>By Benjamin Keith Ford.</div>
<p style="color: grey;"><i>Updated March 14, 2014, 2:43 p.m.</i></p>
<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-19167">ASTERISK-19167</a>
</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;">Notes:
- Is a sub-task of ASTERISK-19167
- Ignore userA directory; will be removed from repository
This test runs through a few different AGI scripts to verify that AGISTATUS returns the correct values:
1. Attempts to run an AGI script that does not exist. AGISTATUS returns NOTFOUND.
2. Attempts to run an AGI script that has an invalid path. AGISTATUS returns FAILURE.
3. Attempts to run an AGI script that has a non-executable interpreter. AGISTATUS returns FAILURE.
4. Attempts to run an AGI script that is non-executable. AGISTATUS returns FAILURE.
5. Runs an AGI script that will be hung up on while waiting for a command. AGISTATUS returns HANGUP.
6. Runs an AGI script that will be hung up on while executing a command. AGISTATUS returns HANGUP.
7. Runs an AGI script that exits normally. AGISTATUS returns SUCCESS.</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/agi/exit_status/waiting.agi <span style="color: grey">(PRE-CREATION)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/test-config.yaml <span style="color: grey">(4749)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/run-test <span style="color: grey">(4749)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/executing.agi <span style="color: grey">(PRE-CREATION)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/donothing.agi <span style="color: grey">(PRE-CREATION)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/badinterpreter3.agi <span style="color: grey">(PRE-CREATION)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/badinterpreter2.agi <span style="color: grey">(PRE-CREATION)</span></li>
<li>./asterisk/trunk/tests/agi/exit_status/badinterpreter.agi <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3329/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>