On 03/13/2012 04:12 AM, guyanhua wrote: > > This adds configuration for "virsh capabilities" test. > > Signed-off-by: Gu Yanhua <[email protected]> > --- > client/virt/subtests.cfg.sample | 16 ++++++++++++++++ > 1 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/client/virt/subtests.cfg.sample > b/client/virt/subtests.cfg.sample > index 5061f42..8552a58 100644 > --- a/client/virt/subtests.cfg.sample > +++ b/client/virt/subtests.cfg.sample > @@ -218,6 +218,22 @@ variants: > status_error = "yes" > libvirtd = "off" > > + - virsh_capabilities: > + type = virsh_capabilities > + vms = '' > + variants: > + - no_option: > + options = "" > + status_error = "no" > + libvirtd = "on" > + - unexpect_option: > + options = "xyz" > + status_error = "yes" > + libvirtd = "on" > + - with_libvirtd_stop: > + options = "" > + status_error = "yes" > + libvirtd = "off" > > - module_probe: > type = module_probe
This looks great! One small nit-pick of mine, and maybe it's my fault (lost mail or whatever), but I thought I commented on the config. key names, no? Assuming it was lost, my suggestion was: It's good to prefix any possibly ambiguous names, with something more obvious as to purpose/source, to ease debugging. Does that make sense? Remember if you're looking at test results, all you get is 'key = value' and not much other context. So, for example, if you see something like "options = whatever", it's hard to know what 'options' and what it's used for w/o digging through code (which could be very old or hard to access or whatever reason). Also, because (IIRC) the keys are sorted alphabetically in the output, if you put a common prefix, then relevant options all get sorted together. One suggestion would be for prefixing the keys with 'virsh_cap_' or "capabilities_" or something like that. Though since it's a simple thing, maybe this can be fixed on commit. It's just a small nit-pick of mine, otherwise this contribution looks great! Thanks! -- Chris Evich, RHCA, RHCE, RHCDS, RHCSS Quality Assurance Engineer e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214 _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
