Re: [Autotest] [AUTOTEST][PATCH 2/2] Add ability to call autotest client tests from kvm tests like a subtest.

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

Hi Jiri, after reviewing the code I have comments, similar to Cleber's:

On Fri, Apr 29, 2011 at 10:59 AM, Jiří Župka <jzupka@redhat.com> wrote:
> Example run autotest/client/netperf2 like a server.

... snip

> diff --git a/client/tests/kvm/tests/subtest.py b/client/tests/kvm/tests/subtest.py
> new file mode 100644
> index 0000000..3b546dc
> --- /dev/null
> +++ b/client/tests/kvm/tests/subtest.py
> @@ -0,0 +1,43 @@
> +import os, logging
> +from autotest_lib.client.virt import virt_utils, virt_test_utils, kvm_monitor
> +from autotest_lib.client.bin import job
> +from autotest_lib.client.bin.net import net_utils
> +
> +
> +def run_subtest(test, params, env):
> +    """
> +    Run an autotest test inside a guest and subtest on host side.
> +    This test should be substitution netperf test in kvm.
> +
> +    @param test: kvm test object.
> +    @param params: Dictionary with test parameters.
> +    @param env: Dictionary with the test environment.
> +    """
> +    vm = env.get_vm(params["main_vm"])
> +    vm.verify_alive()
> +    timeout = int(params.get("login_timeout", 360))
> +    session = vm.wait_for_login(timeout=timeout)
> +
> +    # Collect test parameters
> +    timeout = int(params.get("test_timeout", 300))
> +    control_path = os.path.join(test.bindir, "autotest_control",
> +                                params.get("test_control_file"))
> +    control_args = params.get("test_control_args")
> +    outputdir = test.outputdir
> +
> +    guest_ip = vm.get_address()
> +    host_ip = net_utils.network().get_corespond_local_ip(guest_ip)
> +    if not host_ip is None:
> +        control_args = host_ip + " " + guest_ip
> +
> +        guest = virt_utils.Thread(virt_test_utils.run_autotest,
> +                                 (vm, session, control_path, control_args,
> +                                  timeout, outputdir, params))
> +        guest.start()
> +
> +        test.runsubtest("netperf2", tag="server", server_ip=host_ip,
> +             client_ip=guest_ip, role='server')

^ This really should be made generic, since as Cleber mentioned,
calling this test run_subtest wouldn't cut for cases where we run
something other than netperf2. So things that started coming to my
mind:

* We could extend the utility function to run autotest tests on a
guest in a way that it can accept a string with the control file
contents, rather than just an existing control file. This way we'd be
more free to run arbitrary control code in guests, while of course
keeping the ability to use existing control files;
* We could actually create an Autotest() class abstraction, very much
like what we have in server control files, such as

auto_vm1 = virt_utils.Autotest(vm1) # This would install autotest in a
VM and wait for further commands

control = "job.run_test('sleeptest')"

auto_vm1.run_control(control) # This would run sleeptest and bring
back the results to the host

It's a matter to see how this is modeled for server side control
files... I believe this could be cleaner and help us a lot...

In other comments, please use the idiom:

if foo is not None:

Across all places where we compare a variable with None, because it's
easier to understand the intent right away and it's on the
CODING_STYLE document.

-- 
Lucas