[asterisk-dev] [Code Review] 3329: AGI Exit Status Test

Benjamin Keith Ford reviewboard at asterisk.org
Fri Mar 14 09:43:09 CDT 2014



> On March 13, 2014, 11:13 p.m., Mark Michelson wrote:
> > 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.
> 
> rmudgett wrote:
>     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.
> 
> Mark Michelson wrote:
>     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.

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?


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3329/#review11196
-----------------------------------------------------------


On March 14, 2014, 2:43 p.m., Benjamin Keith Ford wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3329/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 2:43 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-19167
>     https://issues.asterisk.org/jira/browse/ASTERISK-19167
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   ./asterisk/trunk/tests/agi/exit_status/waiting.agi PRE-CREATION 
>   ./asterisk/trunk/tests/agi/exit_status/test-config.yaml 4749 
>   ./asterisk/trunk/tests/agi/exit_status/run-test 4749 
>   ./asterisk/trunk/tests/agi/exit_status/executing.agi PRE-CREATION 
>   ./asterisk/trunk/tests/agi/exit_status/donothing.agi PRE-CREATION 
>   ./asterisk/trunk/tests/agi/exit_status/configs/ast1/extensions.conf PRE-CREATION 
>   ./asterisk/trunk/tests/agi/exit_status/badinterpreter3.agi PRE-CREATION 
>   ./asterisk/trunk/tests/agi/exit_status/badinterpreter2.agi PRE-CREATION 
>   ./asterisk/trunk/tests/agi/exit_status/badinterpreter.agi PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3329/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benjamin Keith Ford
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140314/31a0f258/attachment.html>


More information about the asterisk-dev mailing list