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

[opticron]

> On March 20, 2012, 9:59 a.m., Russell Bryant wrote:
> > How many cases are there where we *do* want a test to run on older versions, but not newer versions?  That's the case where this change would be annoying.  If there are very very few of them, I suppose it's not a big deal.
> 
> Paul Belanger wrote:
>     At the moment, zero.  We have been making sure all tests run on trunk, regardless if a test is considered old.
> 
> opticron wrote:
>     So when we remove a deprecated feature from trunk for which there exists a test, do we remove that test?
> 
> Paul Belanger wrote:
>     No sure, we haven't come across this situation yet.  Something we have to consider, but don't think it will happen to often at the moment.
> 
> opticron wrote:
>     Given that we're going to be dropping 1.4 and 1.6.2 in the near future, only being able to skip individual branches is maintainable for the near term.  I see this more as an augment to the minversion/maxversion rather than a replacement since min/maxversion deal with open-ended ranges while this skip functionality deals with individual branches.  I don't remember if min/maxversion also handles down to the tag level, but if it does that would also be an issue.
> 
> Matt Jordan wrote:
>     I agree with Kinsey.  I don't think we should necessarily remove the minversion or maxversion checking that already exists, nor do I think we should remove the ability to simply skip a test for all branches.  I'd prefer this functionality to exist along side that.  That keeps the current capabilities of the test suite intact, but gives us the additional flexibility that is needed and that this patch does provide.
> 
> Paul Belanger wrote:
>     If we keep the minversion / maxversion flag, what is the actually version order including the digiumphones? I consider the digiumphones a parallel branch, and using something like minversion / maxversion does not work in that scenario.  So, I'm having trouble seeing how we can use ranges with different version of asterisk (1.8/10/trunk s vs 1.8-digiumphones).

I would expect 1.8-digiumphones to be treated exactly like 1.8 as far as minversion/maxversion go.  For the digiumphones branch tests, you'd then have minversion=1.8, skip 1.8, skip 10 so that the digiumphones branches of 1.8 and 10 will be run along with any future versions of asterisk that will (presumably) have this feature.  This has the potential to be confusing for who are not familiar with the digiumphones branches, but those people are unlikely to be writing tests for the digiumphones branches specifically.


- opticron


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


On March 6, 2012, 11:02 a.m., Paul Belanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1796/
> -----------------------------------------------------------
> 
> (Updated March 6, 2012, 11:02 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 3083 
>   asterisk/trunk/lib/python/asterisk/version.py 3083 
>   asterisk/trunk/runtests.py 3083 
>   asterisk/trunk/tests/channels/SIP/message_auth/test-config.yaml 3083 
> 
> 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/20120322/bce9c06e/attachment.htm>