[asterisk-dev] [Code Review]: Initial tests for app_voicemail

[mjordan]

> On Sept. 6, 2011, 11:52 p.m., Paul Belanger wrote:
> >

This test was actually submitted 4 weeks ago, the Ship It having been discussed offline.  As it is, this test was already modified signficantly from what is shown on this review through discussions on the review https://reviewboard.asterisk.org/r/1360/.


> On Sept. 6, 2011, 11:52 p.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/run-test, line 43
> > <https://reviewboard.asterisk.org/r/1354/diff/3/?file=17907#file17907line43>
> >
> >     This will register two callback on each asterisk instance, I don't think that is required?

It is required - the hangup call path on the Asterisk instance acting as the voicemail server will never send a UserEvent.  The dialplan for the caller instance was modified to also send a UserEvent.


> On Sept. 6, 2011, 11:52 p.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast2/extensions.conf, line 9
> > <https://reviewboard.asterisk.org/r/1354/diff/3/?file=17904#file17904line9>
> >
> >     This logic needs to change and not be dependent on Wait(10).

I don't disagree, but that will require adding TestEvents to app_voicemail's leave voicemail app; this test was written quite awhile before we put those in.  I have a feeling I'll be going down that path regardless, but before I do that I'd like to know just where the timing issues are.

Regardless of Wait, the voicemail caller has to wait a significant amount of time for the file to be streamed over, received, and voicemail to get to the point where it will accept either a DTMF signal or a hangup.  10 was a number that in testing guaranteed that - the TestEvent will just be a more robust mechanism of waiting an approximately same period of time.


> On Sept. 6, 2011, 11:52 p.m., Paul Belanger wrote:
> > /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/run-test, lines 63-75
> > <https://reviewboard.asterisk.org/r/1354/diff/3/?file=17907#file17907line63>
> >
> >     I'm not sure I understand why you are calling self.stop_reactor() after we get a UserEvent?  You are processing 5 calls into asterisk, each extension with its own user event.  If you stop the reactor after 1 event, how are the other 4 getting called?

This was changed in the review for all of the additional voicemail tests.  This now looks for 5 UserEvents.


- mjordan


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


On Aug. 10, 2011, 2:09 p.m., mjordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1354/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2011, 2:09 p.m.)
> 
> 
> Review request for Asterisk Developers and Paul Belanger.
> 
> 
> Summary
> -------
> 
> Added initial test for app_voicemail.  This just tests the nominal path for dialplan function VoiceMail().
> 
> Additional tests will be added for the rest of the dialplan applications and their features.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/lib/python/asterisk/asterisk.py 1803 
>   /asterisk/trunk/lib/python/asterisk/voicemail.py PRE-CREATION 
>   /asterisk/trunk/runtests.py 1803 
>   /asterisk/trunk/tests/apps/tests.yaml 1803 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast1/manager.general.conf.inc PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast1/voicemail.conf PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast2/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast2/manager.general.conf.inc PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/configs/ast2/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/run-test PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/sounds/talking.ulaw PRE-CREATION 
>   /asterisk/trunk/tests/apps/voicemail/tests.yaml PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/1354/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> mjordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110908/476de571/attachment-0001.htm>