[asterisk-dev] [Code Review]: regression test for r1570 (app_voicemail, bad load_config order)

[wdoekes]

> On Dec. 6, 2011, 12:15 p.m., mjordan wrote:
> >

Thanks for the review! Will post updated version asap.


> On Dec. 6, 2011, 12:15 p.m., mjordan wrote:
> > /branches/1.8/apps/app_voicemail.c, line 11823
> > <https://reviewboard.asterisk.org/r/1572/diff/1/?file=21657#file21657line11823>
> >
> >     This looks like its only called from your unit tests.  Was that intended?
> >     
> >     If so, you should probably surround it by TEST_FRAMEWORK tags (and maybe rename it to reflect that its test only)

Good point. Will do that.


> On Dec. 6, 2011, 12:15 p.m., mjordan wrote:
> > /branches/1.8/apps/app_voicemail.c, line 12978
> > <https://reviewboard.asterisk.org/r/1572/diff/1/?file=21657#file21657line12978>
> >
> >     If you don't want to make a temporary file, you can also create an ast_config object manually and add categories, variables, etc. in code.
> >     
> >     This way works fine, but just something to keep in mind.

Yeah I know. I started out doing that, but it became one giant heap of code quickly. I figured this was the easiest way to create/adjust a working config quickly.


- wdoekes


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


On Nov. 8, 2011, 3:04 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1572/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2011, 3:04 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Added a regression test for my fix in r1570. It's not real pretty, but it does the job: show that the config load order is slightly broken.
> 
> Regard it as a stub that could be expanded whenever someone wants to fiddle with the app_voicemail configuration options.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/apps/app_voicemail.c 343951 
> 
> Diff: https://reviewboard.asterisk.org/r/1572/diff
> 
> 
> Testing
> -------
> 
> The test case does this, until r1570 is committed: first run fails, second run succeeds.
> 
> *CLI> test execute category /apps/app_voicemail/ name test_voicemail_load_config
> Running all available tests matching category /apps/app_voicemail/ and name test_voicemail_load_config
> 
> START  /apps/app_voicemail/ - test_voicemail_load_config 
> [app_voicemail.c:test_voicemail_load_config:13002]: mailbox 00000002 should have locale 'de_DE.UTF-8', but has ''
> Failed to load configuration file.
> END    /apps/app_voicemail/ - test_voicemail_load_config Time: <1ms Result: FAIL
> 
> 1 Test(s) Executed  0 Passed  1 Failed
> *CLI> test execute category /apps/app_voicemail/ name test_voicemail_load_config
> Running all available tests matching category /apps/app_voicemail/ and name test_voicemail_load_config
> 
> START  /apps/app_voicemail/ - test_voicemail_load_config 
> Failed to load configuration file.
> END    /apps/app_voicemail/ - test_voicemail_load_config Time: <1ms Result: PASS
> 
> 1 Test(s) Executed  1 Passed  0 Failed*CLI> test execute category /apps/app_voicemail/ name test_voicemail_load_config
> Running all available tests matching category /apps/app_voicemail/ and name test_voicemail_load_config
> 
> START  /apps/app_voicemail/ - test_voicemail_load_config 
> [app_voicemail.c:test_voicemail_load_config:13002]: mailbox 00000002 should have locale 'de_DE.UTF-8', but has ''
> Failed to load configuration file.
> END    /apps/app_voicemail/ - test_voicemail_load_config Time: <1ms Result: FAIL
> 
> 1 Test(s) Executed  0 Passed  1 Failed
> *CLI> test execute category /apps/app_voicemail/ name test_voicemail_load_config
> Running all available tests matching category /apps/app_voicemail/ and name test_voicemail_load_config
> 
> START  /apps/app_voicemail/ - test_voicemail_load_config 
> Failed to load configuration file.
> END    /apps/app_voicemail/ - test_voicemail_load_config Time: <1ms Result: PASS
> 
> 1 Test(s) Executed  1 Passed  0 Failed
> 
> 
> When r1570 is also applied, the first run of test succeeds too, like expected.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

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