[asterisk-dev] [Code Review]: fix that ./runtests.py -t something doesn't match something_else

Matt Jordan reviewboard at asterisk.org
Mon Feb 27 07:51:14 CST 2012



> On Feb. 24, 2012, 8:37 a.m., Matt Jordan wrote:
> > Ironically, that's one of the things I *like* about that command-line switch.  Its handy to be able to run a subset of tests that fit within a particular genre.  Particularly things like run all SIP tests:
> > 
> > ./runtests.py --test=tests/channels/SIP
> > 
> > I'm fine with changing this to run just one test, but I'd prefer there to still be an option that does matching.
> 
> opticron wrote:
>     If this patch is committed, we can always use the tag system to run subsets of tests.  The downside is that not all tests are tagged properly and some non-trivial amount of time will be required to ensure that they are.
> 
> Mark Michelson wrote:
>     For Matt's case, couldn't you just let the shell expand '*' for you and do something like ./runtests.py --tests=tests/channels/SIP/* ?
> 
> wdoekes wrote:
>     Shell expansion doesn't make sense for option-arguments -- unless you emulate it using the glob module, but that can require awkward quoting. (--test=tests/* vs. -t 'tests/*').
>     
>     However, since these tests are mapped to actual files, one could argue that individual tests are the de facto arguments to runtests:
>     altering the option parser to take tests as non-option arguments instead of -t argument(s) might be a good idea.

I thought about this some more over the weekend, and I think that this patch is the right approach.  Kinsey's tags would allow us to define subsets of tests to run with a finer degree of granularity that is currently available, and the -t option does imply that it runs a single test.  If we want to modify that such that it isn't even needed, and we can run multiple tests by specifying each test as an argument to the script, that'd be fine too.

I'll think about tagging the tests with some defaults this week and put them up on a wiki page - I don't think that necessarily has to be a part of this patch.


- Matt


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


On Feb. 24, 2012, 3:02 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1771/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2012, 3:02 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> ./runtests.py -t tests/channels/SIP/sip_register
> 
> matched 
> 
> tests/channels/SIP/sip_register
> tests/channels/SIP/sip_register_domain_acl
> 
> with patch it only matches
> 
> tests/channels/SIP/sip_register
> 
> 
> (The whole -t bit could use a bit of refactoring, this is just a quick-fix.)
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/runtests.py 3064 
> 
> Diff: https://reviewboard.asterisk.org/r/1771/diff
> 
> 
> Testing
> -------
> 
> tests/channels/SIP
> 
> still matches all SIP tests.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

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


More information about the asterisk-dev mailing list