[asterisk-dev] [Code Review] Test Queue(F)

[Matt Jordan]

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



/asterisk/trunk/tests/queues/queue_transfer_callee/configs/ast1/modules.conf
<https://reviewboard.asterisk.org/r/1789/#comment10506>

    None of this should be necessary



/asterisk/trunk/tests/queues/queue_transfer_callee/run-test
<https://reviewboard.asterisk.org/r/1789/#comment10508>

    This should be a debug statement - not every UserEvent needs to be sent to messages.txt



/asterisk/trunk/tests/queues/queue_transfer_callee/run-test
<https://reviewboard.asterisk.org/r/1789/#comment10509>

    Instead of if event['userevent'] == 'Alpha', check to see if its not Alpha and return.  That would reduce the indentation needed here.



/asterisk/trunk/tests/queues/queue_transfer_callee/run-test
<https://reviewboard.asterisk.org/r/1789/#comment10510>

    What is expectation?
    
    In general, if you're going to print out an error message, it should give someone enough information to understand why something failed.
    
    "Event test number [3] did not match expected event test number [2]" - or something along those lines - would be much more helpful to someone debugging why a test failed.
    
    And you don't need to proceed if the test failed.  If it failed, you can stop the reactor - if your tests get out of sync, then every test after that is very likely to be off as well.



/asterisk/trunk/tests/queues/queue_transfer_callee/test-config.yaml
<https://reviewboard.asterisk.org/r/1789/#comment10505>

    witha => with a
    
    You may want to reword this, since its not clear from this what "F options" actually is.
    
    Its also not clear who the 'callee' is in this case.  You should probably elaborate on what this actually means, and how the option is triggered (for example, "when the caller hangs up, the called party is transferred to some other extension specified by the F option")


- Matt


On March 2, 2012, 3:01 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1789/
> -----------------------------------------------------------
> 
> (Updated March 2, 2012, 3:01 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson, Paul Belanger, and Matt Jordan.
> 
> 
> Summary
> -------
> 
> Adds a test for new Queue Option F.
> 
> This is basically just like the Baseline Queue Test, only there are 4 scenarios tested and they all use QueueF with different arguments.
> 
> 
> This addresses bug ASTERISK-19283.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19283
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/queues/queue_transfer_callee/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_transfer_callee/configs/ast1/modules.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_transfer_callee/configs/ast1/queues.conf PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_transfer_callee/run-test PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_transfer_callee/run-test.lua PRE-CREATION 
>   /asterisk/trunk/tests/queues/queue_transfer_callee/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/queues/tests.yaml 3072 
> 
> Diff: https://reviewboard.asterisk.org/r/1789/diff
> 
> 
> Testing
> -------
> 
> It's a test.  I ran it a number of times, checked log messages, etc.  Nothing too special.
> 
> 
> Thanks,
> 
> jrose
> 
>

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