[asterisk-dev] [Code Review] Reimplement 'skip' property for testsuite

Matt Jordan reviewboard at asterisk.org
Fri Apr 13 10:37:42 CDT 2012


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



asterisk/trunk/lib/python/asterisk/TestConfig.py
<https://reviewboard.asterisk.org/r/1796/#comment10978>

    I'd still prefer this to be a functor, since that's essentially what it is.
    
    As an aside, I'm not sure we only want to be able to skip branches.  Its very conceivable that a test would be broken in a particular tag, in which case we'd want something like:
    
    min: 1.8
    skip:
      - tag: 1.8.10.1
      - tag: 10.4.1
    
    That is a separate use case then what this particular review was attempting to accomplish, but it is something we may want to add.



asterisk/trunk/lib/python/asterisk/TestConfig.py
<https://reviewboard.asterisk.org/r/1796/#comment10979>

    This logic feels like it would be better placed in the AsteriskVersion class itself.  Something like:
    
    if ast_version.is_same_branch(version):
    
    The fact that we have to check both the branch and the phone properties to determine that would be better handled by the class that maintains that logic.



asterisk/trunk/lib/python/asterisk/TestConfig.py
<https://reviewboard.asterisk.org/r/1796/#comment10977>

    This can be replaced with a list comprehension.
    
    self.can_run = len([s for s in self.skips where s.met is False] == 0)
    
    If any s.met is False, the result of the list comprehension will have the elements of s placed into it.  If any element of s exists in the list, we shouldn't run.



asterisk/trunk/lib/python/asterisk/version.py
<https://reviewboard.asterisk.org/r/1796/#comment10976>

    This could use some more unit tests.  Things the unit tests should confirm:
    
    1. Need to confirm that we can parse the 10-digiumphones branch
    2. What do we do if the phonesbranch doesn't have a parent branch?  Currently, the 10-digiumphones branch does not, as it hasn't had anything merged into it.
    3. How do we compare 1.8 to 1.8-digiumphones?  Currently, this treats 1.8-digiumphones as if it were 1.8.  That would imply that the comparison would rely on subversion revision numbers to determine if something is less than or greater than another branch.  Should the 1.8 branch ever be greater then 1.8-digiumphones?
    


- Matt


On April 12, 2012, 10:27 a.m., Paul Belanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1796/
> -----------------------------------------------------------
> 
> (Updated April 12, 2012, 10:27 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I'm looking for some initial feedback on the following patch.  Since we have the digiumphones branch now, we need to rework on the testsuite handles asterisk versions.  Before we used minversion / maxversion (which worked across branches) however this does not work well if you have a parallel branch.
> 
> So, I've removed them in favour of 'skip'.  Basically, the testsuite will run on every version of asterisk unless you have 'skip' defined, then depending on the flag (right now branch) it checks to run or skip the test.
> 
> We currently use the 'skip' flag, but only for a basic check.  At the moment to skip broken tests, this new method would give us more control on the type of tests to skip.  EG: Skip if OS is FreeBSD or skip if arch is i386.
> 
> 
> Diffs
> -----
> 
>   asterisk/trunk/lib/python/asterisk/TestConfig.py 3176 
>   asterisk/trunk/lib/python/asterisk/version.py 3176 
>   asterisk/trunk/runtests.py 3176 
>   asterisk/trunk/tests/channels/SIP/message_auth/test-config.yaml 3176 
>   asterisk/trunk/tests/channels/SIP/message_unauth/test-config.yaml 3176 
> 
> Diff: https://reviewboard.asterisk.org/r/1796/diff
> 
> 
> Testing
> -------
> 
> Local dev box.
> 
> There is also more work needed to be done in updating the test-config.yaml files, however I want to get this patch reviewed before making all those changes.
> 
> 
> Thanks,
> 
> Paul
> 
>

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


More information about the asterisk-dev mailing list