On Fri, 08 Oct 2010 07:14:33 -0300 Lucas Meneghel Rodrigues <lmr@xxxxxxxxxx> wrote: > On Thu, 2010-10-07 at 16:52 -0300, Luiz Capitulino wrote: > > This series is an _initial_ work on having a full QMP test-suite in > > kvm-autotest. I think it's very near of being in a mergeable state, but I > > figured it would be a good idea to get some feedback before submitting the > > final version. > > > > You'll find all important details in the patches, being the last the most > > important one. > > > > Please, note that there are a number of TODO items to be addressed and I > > need help to solve the following issues: > > > > 1. A number of QMP commands are executed before the QMP suite is run, > > which means that some problems can be caught by non-test code (ie. > > real usage). > > We can test it in a dedicated job, that does qemu specific testing (ie, > non-vm related work, such as qmp, qemu-block, qemu-unittests and such), > or even have a fully dedicated job to qmp. Ok, maybe it will do it, but the important thing is: qmp specifc testing should run first and if it fails, I think no other test should run, as they may fail because of monitor bugs. > > Ideally, the QMP suite should be run before any command is executed, > > so that we avoid catching QMP bugs in other test suites and/or in > > kvm-autotest non-test code. > > > > 2. The qmp_capabilities command is run by the QMPMonitor class > > constructor. This makes it impossible for me to test it in the QMP > > suite. > > > > I have tried destroying and recreating the QMPMonitor object from > > my test-suite, but that didn't seem to work very well. > > We can add an additional parameter to test parameters that indicates > whether the monitors are going to be tested. Today we have this in > tests_base.cfg.sample: > > monitors = humanmonitor1 > > # Choose the main VM and monitor > main_vm = vm1 > main_monitor = humanmonitor1 > ... > # Monitor params > monitor_type = human > > We can add an additional parameter (maybe monitor_mode_[monitor_name]) > to indicate the monitor is under test: > > monitor_mode_humanmonitor1 = test > or > monitor_mode_humanmonitor1 = normal > > We are indicating whether a given monitor is under test or not with this > parameter (defaults allways to normal). If mode is 'test', we handle > monitor class initialization, and potentially, other methods > differently. What do you think? I think I now understand why we have zillions of options :-) Don't take me wrong (and this is another subject, anyway) but something I disliked in kvm-autotest is that I had to edit different files to get it running and add new tests. Not to mention that the files you change, are not the files in the repository. So you end up having to keep track of a few options in a few different files. I know you have to deal with a number of different test setups, and this is no easy job, but at the same time I feel this could be simplified. Anyway, back to QMP testing. Adding a new parameter too all tests seems a bit complex to me. What about this: we make qmp_basic.py (or any monitor specific testing) a special type of test cases and add them to 'monitor_testing' option in tests_base.cfg: monitor_testing = qmp_basic.py hmp_basic.py And then the VM.create() method (in kvm_vm.py) could test for that option and automatically run those tests even before creating the monitor classes. If a test fails, kvm-autotest aborts. I'm now wondering whether this fits in the current design. > > > I have other, more general comments, about kvm-autotest, but I'll send them > > in a different thread. > > All right Luiz, this is all excellent, thanks! > > > Thanks. > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html