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

Reply via email to