[asterisk-dev] [Code Review] 3182: testsuite: LinkedID Propagation test

[Matt Jordan]

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



/asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py
<https://reviewboard.asterisk.org/r/3182/#comment20324>

    Although I'm sure the code this was pulled from used three single quotes to denote a pydoc comment, we've since gone with three double quotes in the testsuite library modules.
    
    Yes, either are okay - but for the sake of appeasing the gods of consistency (who are wont to be filled with wrath), I'd use """ to denote pydoc comments from here on out.



/asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py
<https://reviewboard.asterisk.org/r/3182/#comment20323>

    I didn't write this :-)
    
    Or... maybe I did?
    
    If so, and multiple tests need to use this logic, there's no need to copy it. In fact, having duplicate copies of what drives the same test isn't needed. There are two ways to handle this:
    
    (1) Add the linkedid propagation verification to the existing test. This is probably preferable, if what drives the test is exactly the same as another test.
    
    (2) If there are differences between the Asterisk actions that drive the test, then refactor the common code into a shared module. You have two choices on where to put that shared module:
    (a) In lib/python/asterisk
    (b) In a Python module in a base directory. The YAML for defining pluggable modules lets you specify search paths for modules, which you can then specify to the location of the module.



/asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py
<https://reviewboard.asterisk.org/r/3182/#comment20325>

    Pedantic: pydoc comments typically don't have a space between the start of the comment and the first word of the comment.
    
    So:
    
    """Foo
    
    Instead of:
    
    """ Foo
    
    Granted, I wrote this before I had bothered to look at that... so if you remove this file, feel free to disregard these comments here. If you decide to refactor or keep this, then by all means clean up my mistakes :-)



/asterisk/trunk/tests/bridge/linkedid_propagation/configs/ast1/extensions.conf
<https://reviewboard.asterisk.org/r/3182/#comment20330>

    Formatting (probably tabs/spaces)



/asterisk/trunk/tests/bridge/linkedid_propagation/linkedid_check.py
<https://reviewboard.asterisk.org/r/3182/#comment20326>

    Pydoc: """ instead of ''' for consistency



/asterisk/trunk/tests/bridge/linkedid_propagation/linkedid_check.py
<https://reviewboard.asterisk.org/r/3182/#comment20327>

    For Pydoc comments, the summary goes on the same line as the start of the comment. So:
    
    """Foo
    
    Instead of:
    
    """
    Foo



/asterisk/trunk/tests/bridge/linkedid_propagation/test-config.yaml
<https://reviewboard.asterisk.org/r/3182/#comment20328>

    I think this should just be added as a check in the existing test that verifies the BridgeAction AMI action.



/asterisk/trunk/tests/bridge/linkedid_propagation/test-config.yaml
<https://reviewboard.asterisk.org/r/3182/#comment20329>

    I don't think we need a tag specifically for linkedid checking.
    
    Tags are only useful when you have a number of tests that all verify the same thing. Until we have a number of tests that verify linkedids, this doesn't really add any value (and even then, how often will we want to run the test suite only verifying linkedids?)


- Matt Jordan


On Feb. 4, 2014, 4:16 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3182/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2014, 4:16 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23120
>     https://issues.asterisk.org/jira/browse/ASTERISK-23120
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> This monitors bridge enter/exit and CEL events, and builds
> a picture of which channels are bridged together, and when
> their LinkedID changes, and then verifies that each change
> is correct.
> 
> Note: The BridgeEnter and CEL 'BRIDGE_ENTER' events can be
> in either order - so wait for both to arrive before making
> a judgement on the LinkedID.
> 
> This test can be added to any other bridging test to add a
> layer of LinkedId Propagation checking to it.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/bridge/tests.yaml 4671 
>   /asterisk/trunk/tests/bridge/linkedid_propagation/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/bridge/linkedid_propagation/linkedid_check.py PRE-CREATION 
>   /asterisk/trunk/tests/bridge/linkedid_propagation/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3182/diff/
> 
> 
> Testing
> -------
> 
> This test fails spuriously due to what appears to be CEL events being sent on BRIDGE_ENTER prior to the LinkedId being updated.  This bug will be fixed along with the other changes for 23120.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140206/974dc287/attachment-0001.html>