[asterisk-dev] [Code Review]: Rework the leave_voicemail_contexts test

[Matt Jordan]

> On May 3, 2012, 10:18 a.m., Mark Michelson wrote:
> > From a pure "code does what it is supposed to" point of view, I say it's good to go. I'm not one to be able to state whether the code is necessarily "pythonic" enough, though.

I'm not sure what wouldn't be pythonic about it, at least not from my quick googling as to what "pythonic" refers to.  There aren't any "C" idioms expressed in the test, and while there are some aspects of the VoiceMailTest and the VoiceMailState objects that I would do a bit differently  now, I'm not sure the minor tweaks are justified in light of having to change 20+ tests.

Also: can't ship this until the corresponding code fix gets reviewed. :-)


- Matt


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


On May 1, 2012, 4:32 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1893/
> -----------------------------------------------------------
> 
> (Updated May 1, 2012, 4:32 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This reworks the leave_voicemail_contexts test to:
> 1) Use the TEST_FRAMEWORK and test events to control the test, as opposed to a lot of Wait() statements
> 2) Actually test valid extensions that VoiceMail has a hope and a prayer of bouncing a user out to
> 3) Test the conditions that currently fail in Asterisk, namely the d(context) option failing if the extension the user attempts to go to does not exist in the current dialplan context.
> 
> This review being checked in is contingent on review 1892 being approved.
> 
> 
> This addresses bug ASTERISK-18243.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18243
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_contexts/test-config.yaml 3212 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_contexts/configs/ast1/extensions.conf 3212 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_contexts/configs/ast1/voicemail.conf 3212 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_contexts/configs/ast2/extensions.conf 3212 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_contexts/run-test 3212 
> 
> Diff: https://reviewboard.asterisk.org/r/1893/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

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