[asterisk-dev] [Code Review] Initial set of Queue tests

Tue Jun 1 13:14:27 CDT 2010

> On 2010-05-28 10:18:47, David Vossel wrote:
> > I have one comment that kind of falls over all of these tests.  What happens when Asterisk crashes or deadlocks?  Are we guaranteed that the tests will timeout at some point and return?  Since I am not very familiar with the lua framework being used, it is hard for me to know for sure what will happen.  In the python tests we have written there has always been some sort of failsafe timeout used to guarantee the test would exit if something went wrong.

It's interesting that you should bring this up. Matt Nicholson designed the test framework in such a way that we should never just block forever anywhere. Furthermore, I always invoke SIPp with a timeout of 30 seconds so that if we are waiting for a SIPp process to complete, it should at most take 30 seconds.

Now with that said, I have noticed tests hang when Asterisk crashes on me. Furthermore, Russell has reported to me that tests in the test suite will occasionally hang forever, but it's not consistent which ones do it. The test that he originally brought this up with, the RFC 2833 DTMF detection test, does not appear to have any part of it that could just hang forever though, so I'm not really sure about why such hangs will happen.

> On 2010-05-28 10:18:47, David Vossel wrote:
> > /asterisk/trunk/tests/queues/queue_baseline/test.lua, line 66
> > <https://reviewboard.asterisk.org/r/638/diff/1/?file=9937#file9937line66>
> >
> >     I noticed in your previous test that the sleep was only 1. Here it is 2.  By chance did you have it set to 1 here and find that the test did not pass consistently?  If so I don't think it would hurt to raise this in the other places as well... It would be nice if this could just be done another way rather than waiting and hoping the messages arrive in time, but I understand this limitation given the tool set.

In this case, the caller into the queue waits for 1 second, and the answerer in the queue waits for 2 seconds. I chose to sleep for 2 seconds so I could be sure that the entire call was finished by the time that I processed things any further.

> On 2010-05-28 10:18:47, David Vossel wrote:
> > /asterisk/trunk/tests/queues/queue_baseline/test.lua, lines 87-88
> > <https://reviewboard.asterisk.org/r/638/diff/1/?file=9937#file9937line87>
> >
> >     I noticed you logoff the manager here before terminating the asterisk instance.  In the other tests you did not do this.  It shouldn't matter, just thought I'd point it out since it was an inconsistency.

Yeah, consistency is good. Just for kicks, I'll go ahead and log the manager off in all these tests.

> On 2010-05-28 10:18:47, David Vossel wrote:
> > /asterisk/trunk/tests/queues/ringinuse_and_pause/test.lua, lines 129-130
> > <https://reviewboard.asterisk.org/r/638/diff/1/?file=9944#file9944line129>
> >
> >     Do you need a sleep here before pump_messages like in the other tests?

Nope. The event I care about is guaranteed to have happened by the time the originate returns.

> On 2010-05-28 10:18:47, David Vossel wrote:
> > /asterisk/trunk/tests/queues/ringinuse_and_pause/test.lua, lines 173-176
> > <https://reviewboard.asterisk.org/r/638/diff/1/?file=9944#file9944line173>
> >
> >     I don't see where Asterisk is terminated for this test.

I usually let the test framework do it for me. I can do it myself though.

> On 2010-05-28 10:18:47, David Vossel wrote:
> > /asterisk/trunk/tests/queues/wrapup_time/test.lua, line 93
> > <https://reviewboard.asterisk.org/r/638/diff/1/?file=9950#file9950line93>
> >
> >     Again, why is this 3.  If this was just experimentation on your box and 3 appeared to work best, that might be kind of aggressive for slower machines.

You can see in the originate that I have set the data for the Wait application to 3. So like the other sleep you commented on, I am waiting the same amount so the call will terminate. Really though, both the originate's wait time and this sleep could be lowered. I think I just started with 3 and since it worked, I didn't bother changing it.

> On 2010-05-28 10:18:47, David Vossel wrote:
> > /asterisk/trunk/tests/queues/wrapup_time/test.lua, lines 127-129
> > <https://reviewboard.asterisk.org/r/638/diff/1/?file=9950#file9950line127>
> >
> >     Do we need to close the asterisk instances here?

Not necessary, but I can do it anyway to make things more clear.

> On 2010-05-28 10:18:47, David Vossel wrote:
> > /asterisk/trunk/tests/queues/wrapup_time/test.lua, line 116
> > <https://reviewboard.asterisk.org/r/638/diff/1/?file=9950#file9950line116>
> >
> >     Is sleep because it is taking longer than expected for the two asterisk instances to start? You did not do this in the other tests.  If it because of this you may want to make this one longer that 1 second since on slower machines it might take quite a bit longer.  We need a better way of knowing when asterisk is fully loaded.

You finally hit something that I don't have an explanation for :). I likely did put this sleep in as a way of letting the Asterisk processes start. Since it was so long ago that I wrote this, I don't recall if I had to add it because things weren't working originally, or if I put it there to begin with and never experimented with removing it.

We discussed earlier about how waiting for the manager FullyBooted event would be a good idea for this sort of situation, though, and should actually be applied to all tests, not just this one.

- Mark

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

On 2010-04-30 16:46:45, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/638/
> -----------------------------------------------------------
> 
> (Updated 2010-04-30 16:46:45)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> A set of five queue tests.
> 
> 1. Queue Baseline: Places a call to a queue and ensures that the call goes through as expected by watching for manager events
> 2. Wrapup Time: Places calls to queue members and then ensures that the member cannot be reached during wrapuptime. Tests both regular queues and sharedlastcall instances.
> 3. Gosub/macro: Tests that macros and gosubs are executed as expected when a member answers. Tests both static configuration and overriding with parameters to the Queue application.
> 4. Ringinuse and Pause: Tests that members are not called if ringinuse=no. Ensures that autopause only takes effect if the member is attempted to be called. Ensures that paused members do not get called.
> 5. Position, Priority, and maxlen: Adds a bunch of callers to a queue and ensures that they enter at the position we expect them to.
> 
> I have ideas for about 15 more tests, but those will have to come later.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/queues/macro_gosub_test/configs/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/macro_gosub_test/configs/queues.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/macro_gosub_test/run-test PRE-CREATION 
>   /asterisk/trunk/tests/queues/macro_gosub_test/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/queues/macro_gosub_test/test.lua PRE-CREATION 
>   /asterisk/trunk/tests/queues/position_priority_maxlen/configs/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/position_priority_maxlen/configs/queues.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/position_priority_maxlen/run-test PRE-CREATION 
>   /asterisk/trunk/tests/queues/position_priority_maxlen/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/queues/position_priority_maxlen/test.lua PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_baseline/configs/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_baseline/configs/queues.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_baseline/run-test PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_baseline/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_baseline/test.lua PRE-CREATION 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/configs/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/configs/queues.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/configs/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/run-test PRE-CREATION 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/sipp/uas.xml PRE-CREATION 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/test.lua PRE-CREATION 
>   /asterisk/trunk/tests/queues/wrapup_time/configs/ast1/queues.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/wrapup_time/configs/ast2/queues.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/wrapup_time/configs/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/wrapup_time/run-test PRE-CREATION 
>   /asterisk/trunk/tests/queues/wrapup_time/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/queues/wrapup_time/test.lua PRE-CREATION 
>   /asterisk/trunk/tests/tests.yaml 275 
> 
> Diff: https://reviewboard.asterisk.org/r/638/diff
> 
> 
> Testing
> -------
> 
> Ran the tests. They pass.
> 
> 
> Thanks,
> 
> Mark
> 
>