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

[Jiri Zupka <jzupka@redhat.com>]

----- Original Message -----
> 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:

^ Yes you are right. I wanted to show how use and configure parameter 
in control file. This shouldn't be a test this test should be only a sample 
of technology. But I made wrong implanting to tests_base.conf. I try think 
about tests_base.conf and make this implantation in better way. 

I repair subtest and send patch again.

> 
> * 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')"

                                            ^ This should be standard test in client/tests/
                                                not file from client/tests/kvm/autotest_control.

> 
> 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...

And yes I agree with this. This sounds good. 

> 
> 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.

^^ I try this.
> 
> --
> Lucas