On 06/26/2012 11:33 PM, guyanhua wrote: > > Signed-off-by: Gu Yanhua<guyanhua-f...@cn.fujitsu.com> > --- > client/tests/libvirt/tests/virsh_freecell.py | 57 > ++++++++++++++++++++++++++ > 1 files changed, 57 insertions(+), 0 deletions(-) > create mode 100644 client/tests/libvirt/tests/virsh_freecell.py > > diff --git a/client/tests/libvirt/tests/virsh_freecell.py > b/client/tests/libvirt/tests/virsh_freecell.py > new file mode 100644 > index 0000000..05b3705 > --- /dev/null > +++ b/client/tests/libvirt/tests/virsh_freecell.py > @@ -0,0 +1,57 @@ > +import re, logging > +from autotest.client.shared import utils, error > +from autotest.client.virt import libvirt_vm > +from autotest.client import * > + > +def run_virsh_freecell(test, params, env): > + """ > + Test the command virsh freecell > + > + (1) Call virsh freecell > + (2) Call virsh freecell --all > + (3) Call virsh freecell with a numeric argument > + (4) Call virsh freecell xyz > + (5) Call virsh freecell with libvirtd service stop > + """ > + > + # Prepare libvirtd service > + check_libvirtd = params.has_key("libvirtd") > + if check_libvirtd: > + libvirtd = params.get("libvirtd") > + if libvirtd == "off": > + libvirt_vm.service_libvirtd_control("stop") > + > + # Run test case > + option = params.get("virsh_freecell_options") > + cmd_result = libvirt_vm.virsh_freecell(ignore_status=True, extra=option) > + logging.info("Output:\n%s", cmd_result.stdout.strip()) > + logging.info("Status: %d", cmd_result.exit_status) > + logging.error("Error: %s", cmd_result.stderr.strip())
Here I think we can use tangchen's addition to virsh_cmd(print_info=True), posted 15 minutes before this patch (did you coordinate that?) :D > + output = cmd_result.stdout.strip() > + status = cmd_result.exit_status > + > + # Recover libvirtd service start > + if libvirtd == "off": > + libvirt_vm.service_libvirtd_control("start") Mmmm, I did not run pylint, but looking at it, I think maybe libvirtd variable is not in-scope here? It was defined w/in a conditional block above. I could be wrong though. > + > + # Check the output > + def output_check(freecell_output): > + if not re.search("kB", freecell_output): > + raise error.TestFail("virsh freecell output invalid!") > + It's a little "cleaner" if all nested function definitions appear at the beginning. Not a big deal though. > + # Check status_error > + status_error = params.get("status_error") > + if status_error == "yes": > + if status == 0: > + if libvirtd == "off": > + raise error.TestFail("Command 'virsh freecell' succeeded " > + "with libvirtd service stopped, > incorrect") > + else: > + raise error.TestFail("Command 'virsh freecell %s' succeeded" > + "(incorrect command)" % option) > + elif status_error == "no": > + output_check(output) > + if status != 0: > + raise error.TestFail("Command 'virsh freecell %s' failed " > + "(correct command)" % option) > + This checking at the end looks good, it's easy to read and understand what's happening. Good job. -- Chris Evich, RHCA, RHCE, RHCDS, RHCSS Quality Assurance Engineer e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214 _______________________________________________ Autotest mailing list Autotest@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest