Patchwork [Autotest,2/2] virt run: add three logical case filters

login
register
mail settings
Submitter Amos Kong
Date Dec. 30, 2012, 1:02 a.m.
Message ID <1356829329-30082-2-git-send-email-akong@redhat.com>
Download mbox | patch
Permalink /patch/208689/
State New
Headers show

Comments

Amos Kong - Dec. 30, 2012, 1:02 a.m.
This patch added there options for filtering cases by logics,

For example:
./run -t qemu -c tests.cfg --oronly="WinXP Win7" --andonly="boot 64" --not="sp1"
  (following cases will be executed)

Test    1: virtio_blk.smp2.virtio_net.WinXP.64.boot
Test    2: virtio_blk.smp2.virtio_net.Win7.64.boot

Current '--tests' option will actually added OR case filter.
I would like to reserve '--tests', because it's easy to be
understood. eg:

 --tests="Win7 WinXP":
 cases of those two guests will be filtered.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 run |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
Amos Kong - Dec. 30, 2012, 1:10 a.m.
On Sun, Dec 30, 2012 at 09:02:09AM +0800, Amos Kong wrote:
> This patch added there options for filtering cases by logics,
> 
> For example:
> ./run -t qemu -c tests.cfg --oronly="WinXP Win7" --andonly="boot 64" --not="sp1"

Oh! a typo in commitlog

 ./run -t qemu -c tests.cfg --or="WinXP Win7" --and="boot 64" --not="sp1"

>   (following cases will be executed)
> 
> Test    1: virtio_blk.smp2.virtio_net.WinXP.64.boot
> Test    2: virtio_blk.smp2.virtio_net.Win7.64.boot
...

                Amos
Lucas Meneghel Rodrigues - Dec. 31, 2012, 7:55 p.m.
Hmmm, about this one, I'm worried about making things more complex...

The way I see the problem at hand, I'd say if people want to customize
things, they'd be better of creating their own, specialized config
files rather than adding command line flags to manipulate the test
sets.

So my initial stand on this particular patch is NACK, but you might
convince me otherwise :)

On Sat, Dec 29, 2012 at 11:10 PM, Amos Kong <akong@redhat.com> wrote:
> On Sun, Dec 30, 2012 at 09:02:09AM +0800, Amos Kong wrote:
>> This patch added there options for filtering cases by logics,
>>
>> For example:
>> ./run -t qemu -c tests.cfg --oronly="WinXP Win7" --andonly="boot 64" --not="sp1"
>
> Oh! a typo in commitlog
>
>  ./run -t qemu -c tests.cfg --or="WinXP Win7" --and="boot 64" --not="sp1"
>
>>   (following cases will be executed)
>>
>> Test    1: virtio_blk.smp2.virtio_net.WinXP.64.boot
>> Test    2: virtio_blk.smp2.virtio_net.Win7.64.boot
> ...
>
>                 Amos
>
Andreas Färber - Jan. 4, 2013, 1:13 p.m.
Am 31.12.2012 20:55, schrieb Lucas Meneghel Rodrigues:
> Hmmm, about this one, I'm worried about making things more complex...
> 
> The way I see the problem at hand, I'd say if people want to customize
> things, they'd be better of creating their own, specialized config
> files rather than adding command line flags to manipulate the test
> sets.
> 
> So my initial stand on this particular patch is NACK, but you might
> convince me otherwise :)

Lucas, following your nice presentation at KVM Forum I have set up
virt-test for local testing of my core CPU refactorings.
When run using
./run -t kvm --qemu-bin=/.../x86_64-softmmu/qemu-system-x86_64
it runs only 13 migration tests:

(1/13) migrate.default.tcp: PASS (41.04 s)
(2/13) migrate.default.unix: PASS (21.88 s)
(3/13) migrate.default.exec: PASS (20.84 s)
(4/13) migrate.default.fd: PASS (23.04 s)
(5/13) migrate.default.mig_cancel: PASS (18.66 s)
(6/13) migrate.with_set_speed.tcp: PASS (19.22 s)
(7/13) migrate.with_set_speed.unix: PASS (19.07 s)
(8/13) migrate.with_set_speed.exec: PASS (19.03 s)
(9/13) migrate.with_set_speed.fd: PASS (18.93 s)
(10/13) migrate.with_reboot.tcp: PASS (40.97 s)
(11/13) migrate.with_reboot.unix: PASS (42.90 s)
(12/13) migrate.with_reboot.exec: PASS (47.04 s)
(13/13) migrate.with_reboot.fd: PASS (47.95 s)

Whereas --list-tests shows 274 tests.

So while I am impartial to this specific patch, some easy way to run a
comprehensive test coverage without having to manually name each test
using --tests= would be very handy! Something like --all-tests maybe?

If there is such a thing already, it is not obvious to the novice user
and --help output may need to be extended.

Regards,
Andreas
Lucas Meneghel Rodrigues - Jan. 4, 2013, 2:21 p.m.
On 01/04/2013 11:13 AM, Andreas Färber wrote:
> Am 31.12.2012 20:55, schrieb Lucas Meneghel Rodrigues:
>> Hmmm, about this one, I'm worried about making things more complex...
>>
>> The way I see the problem at hand, I'd say if people want to customize
>> things, they'd be better of creating their own, specialized config
>> files rather than adding command line flags to manipulate the test
>> sets.
>>
>> So my initial stand on this particular patch is NACK, but you might
>> convince me otherwise :)
>
> Lucas, following your nice presentation at KVM Forum I have set up
> virt-test for local testing of my core CPU refactorings.
> When run using
> ./run -t kvm --qemu-bin=/.../x86_64-softmmu/qemu-system-x86_64
> it runs only 13 migration tests:
>
> (1/13) migrate.default.tcp: PASS (41.04 s)
> (2/13) migrate.default.unix: PASS (21.88 s)
> (3/13) migrate.default.exec: PASS (20.84 s)
> (4/13) migrate.default.fd: PASS (23.04 s)
> (5/13) migrate.default.mig_cancel: PASS (18.66 s)
> (6/13) migrate.with_set_speed.tcp: PASS (19.22 s)
> (7/13) migrate.with_set_speed.unix: PASS (19.07 s)
> (8/13) migrate.with_set_speed.exec: PASS (19.03 s)
> (9/13) migrate.with_set_speed.fd: PASS (18.93 s)
> (10/13) migrate.with_reboot.tcp: PASS (40.97 s)
> (11/13) migrate.with_reboot.unix: PASS (42.90 s)
> (12/13) migrate.with_reboot.exec: PASS (47.04 s)
> (13/13) migrate.with_reboot.fd: PASS (47.95 s)
>
> Whereas --list-tests shows 274 tests.
>
> So while I am impartial to this specific patch, some easy way to run a
> comprehensive test coverage without having to manually name each test
> using --tests= would be very handy! Something like --all-tests maybe?

Hmm, could be. Due to the long time it'd be needed to run --all-tests, I 
thought it wouldn't be necessary.

> If there is such a thing already, it is not obvious to the novice user
> and --help output may need to be extended.

One thing that does help here is to know that you can specify only the 
first component of the test name displayed on --help, and it'll run all 
derivated tests. For example, if you want to run all virtio_console 
tests, you can just specify:

./run -t kvm --qemu-bin=/path/to/qemu --tests virtio_console

SETUP: PASS (1.60 s)
DATA DIR: /home/area/virt_test/
DEBUG LOG: 
/home/lmr/Code/virt-test.git/logs/run-2013-01-04-12.20.20/debug.log
TESTS: 118
(1/118) 
virtio_console.spread_linear.specifiable.virtserialport.with_vm.open:

I need to think of a good way of exposing this information on help output...
Gerd Hoffmann - Jan. 4, 2013, 3:06 p.m.
Hi,

> So while I am impartial to this specific patch, some easy way to run a
> comprehensive test coverage without having to manually name each test
> using --tests= would be very handy! Something like --all-tests maybe?

Hint from the dirty tricks department:
  --tests=JeOS will do (for --type=kvm).

Not sure how useful that actually is though, not all tests are in a good
and tested state.

HTH,
  Gerd
Lucas Meneghel Rodrigues - Jan. 4, 2013, 5:35 p.m.
On Fri, Jan 4, 2013 at 1:06 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> So while I am impartial to this specific patch, some easy way to run a
>> comprehensive test coverage without having to manually name each test
>> using --tests= would be very handy! Something like --all-tests maybe?
>
> Hint from the dirty tricks department:
>   --tests=JeOS will do (for --type=kvm).
>
> Not sure how useful that actually is though, not all tests are in a good
> and tested state.

Yep, there's that as well. I've fixed a lot of tests during the last
weeks, but indeed some tests might be broken.

> HTH,
>   Gerd
>
Qingtang Zhou - Jan. 5, 2013, 8:52 a.m.
* On 2012-12-31 17:55:24 -0200, Lucas Meneghel Rodrigues (lookkas@gmail.com) wrote:
> Hmmm, about this one, I'm worried about making things more complex...
> 
> The way I see the problem at hand, I'd say if people want to customize
> things, they'd be better of creating their own, specialized config
> files rather than adding command line flags to manipulate the test
> sets.
Which kind of user interface do you want to provide to us (autotest user)?
A cli? But why does it can't specify more parameters or filters?
A config file? Why don't allow us to update tests.cfg?

I'm totally confused.

And why must I download that JeOS image?
I've setup my own image, why can't I use it directly?

Well, I prefer the old autotest style more now.

> 
> So my initial stand on this particular patch is NACK, but you might
> convince me otherwise :)
> 
> On Sat, Dec 29, 2012 at 11:10 PM, Amos Kong <akong@redhat.com> wrote:
> > On Sun, Dec 30, 2012 at 09:02:09AM +0800, Amos Kong wrote:
> >> This patch added there options for filtering cases by logics,
> >>
> >> For example:
> >> ./run -t qemu -c tests.cfg --oronly="WinXP Win7" --andonly="boot 64" --not="sp1"
> >
> > Oh! a typo in commitlog
> >
> >  ./run -t qemu -c tests.cfg --or="WinXP Win7" --and="boot 64" --not="sp1"
> >
> >>   (following cases will be executed)
> >>
> >> Test    1: virtio_blk.smp2.virtio_net.WinXP.64.boot
> >> Test    2: virtio_blk.smp2.virtio_net.Win7.64.boot
> > ...
Lucas Meneghel Rodrigues - Jan. 5, 2013, 3:29 p.m.
On Sat, Jan 5, 2013 at 6:52 AM, Qingtang Zhou <qzhou@redhat.com> wrote:
> * On 2012-12-31 17:55:24 -0200, Lucas Meneghel Rodrigues (lookkas@gmail.com) wrote:
>> Hmmm, about this one, I'm worried about making things more complex...
>>
>> The way I see the problem at hand, I'd say if people want to customize
>> things, they'd be better of creating their own, specialized config
>> files rather than adding command line flags to manipulate the test
>> sets.
> Which kind of user interface do you want to provide to us (autotest user)?
> A cli?

"Old autotest" users know and care more about using the cartesian
config, and therefore they can make their own .cfg file. The old
tests.cfg was saved as tests-example.cfg, so one could use that and
build on top of it. If the person wants to run virt-tests using
autotest, that's fine, and we'll always support it.

> But why does it can't specify more parameters or filters?
> A config file? Why don't allow us to update tests.cfg?

See, you can always specify a config file:

./run -t qemu -c qemu/cfg/my-custom-file.cfg

The idea behind tests.cfg is that it's reserved for the test runner.
It's not a lot to ask I think, given that you can use a custom file
with -c.

> I'm totally confused.

I understand. You could be more willing to understand that we're doing
the possible to accomodate users that:

a) Don't care about 'autotest'. They just want a simple way to get to
test execution.
b) Don't care about 'cartesian files'. It's always been a point of
endless complaint, and I hope eventually we can get rid of it. While
we can't figure out how to do it, I'm trying to at least mitigate the
situation by making it *as transparent as possible* to users that fit
into a) and b).

I said I'm in principle against Amos' patch, but if you guys can
enlighten me as of why it's beneficial, I can change my mind.

> And why must I download that JeOS image?

You don't. There's always the option of doing things the 'old' way,
running the tests under autotest, etc. If you want a tutorial on how
to do that, I could write one.

> I've setup my own image, why can't I use it directly?

I'm working on a way of doing it from the test runner.

> Well, I prefer the old autotest style more now.

I won't say I liked the tone of your message, but let me try to be
objective and constructive:

 1) It's not our intention to piss off the people who have been using
and developing our tests over the last 4 years.
 2) It helps a lot opening issues and stating clearly what are the
problems you are seeing with the recent changes, remarks, and how you
see it could be solved. I think we can work this out together.

Thanks,

Lucas

Patch

diff --git a/run b/run
index aac332a..da02ced 100755
--- a/run
+++ b/run
@@ -158,6 +158,15 @@  class VirtTestRunParser(optparse.OptionParser):
         general.add_option("--tests", action="store", dest="tests",
                            help=('List of space separated tests to be executed'
                                 ' - example: --tests "boot reboot shutdown"'))
+        general.add_option("--and", action="store", dest="andstr",
+                           help=('Add AND case filter'
+                                ' - example: --and "RHEL boot"'))
+        general.add_option("--or", action="store", dest="orstr",
+                           help=('Add OR case filter'
+                                ' - example: --tests "boot shutdown"'))
+        general.add_option("--not", action="store", dest="nostr",
+                           help=('Add NOT case filter'
+                                 ' - example: --not "netperf_win"'))
         general.add_option("--list-tests", action="store_true", dest="list",
                            help="List tests available")
         general.add_option("--data-dir", action="store", dest="datadir",
@@ -310,6 +319,18 @@  class VirtTestApp(object):
             tests = self.options.tests.split(" ")
             self.cartesian_parser.parse_string("only %s" % ", ".join(tests))
 
+        if self.options.type and self.options.andstr:
+            for i in self.options.andstr.split(" "):
+                self.cartesian_parser.parse_string("only %s" % i)
+
+        if self.options.type and self.options.orstr:
+            tests = self.options.orstr.split(" ")
+            self.cartesian_parser.parse_string("only %s" % ", ".join(tests))
+
+        if self.options.type and self.options.nostr:
+            for i in self.options.nostr.split(" "):
+                self.cartesian_parser.parse_string("no %s" % i)
+
         elif (self.options.type and not self.options.tests
               and not self.options.config):
             if self.options.type == 'qemu':