[libvirt] [test-API] RFC: Stabilization of libvirt-test-API

[Martin Kletzander]

On 03/29/2012 03:42 PM, Osier Yang wrote:
> On 2012年03月29日 20:14, Martin Kletzander wrote:
>> Hi everyone,
>>
>> following our minutes, I'd like to start a discussion on what should be
>> done with libvirt-test-API so we can say it's stable and usable.
>>
>> I would like to stress out that everything mentioned here is just an
>> opinion and I don't mean to talk down to someone as it may have seemed
>> earlier.
>>
>> I think we should get some ideas from everyone, mostly QE as they will
>> be (are) the ones using this the most (if I understood correctly), and
>> then I'll be happy to help getting the code to the agreed status. I was
>> thinking about this from the wrong way probably and changing the angle
>> from what I look at it (and knowing there is some deadline) makes me
>> think of few levels of changes, which when introduced, could speed up
>> the test development and code understandability.
>>
>> So here are the things I would like to do definitely (the optional
>> things follow later on):
>>   - fix hard-coded options into real options (e.g. commit 65449e)
> 
> Absolutely we should either change/destroy it or generalize it
> as global config.
> 
>>   - fix some env_* and util* code (functions duplicated with different
>> behavior)
> 
> This should be caused by mutilple persons worked on that, but lacked
> reviewing.
> 
>>   - fix or remove harmful and pointless code (at this point, when
>> creating domain on remote machine, be prepared for the test to fail with
>> any other user then root and with root, have backup of both local and
>> remote '/root/.ssh' directories as the contents will be erased!)
> 
> So this means test-API only supports qemu:///system testing now,
> needs to be improved for qemu:///session too.
> 
> Also I'd guess there are cases which only considers the QEMU/KVM
> driver testing. If so, we need to either generalize it or seperate
> it (in case of it are too specific to generalize), perhaps seperate
> directories for different drivers. But this should be the future
> plan, what we should do currently is try to generalize the tests
> as much as we could.
> 
>>   - fix method names for the {connect,domain,etc.}API (get_host_name vs.
>> lookupByUUID etc.)
> 
> Yes, we need the consistent function/variable name, also consistent
> coding (including the comments on top of scripts) style.
> 
>>
>> The optional things:
>>   - get rid of classes in lib and make just few utility functions
>> covering *only* the methods that do something else than call the same
>> method in underlying class from the libvirt module.
> 
> Agreed, it looks to me many of the lib functions just simple pass
> parameter to the underlying libvirt-python API, that's just meaningless/
> useless.
> 
>>   - get rid of the new exception (I don't see any other difference than
>> in the name, which can make a difference in "except:" clause, but it's
>> converted everywhere)
> 
> Agreed. Just like the classes method in lib, it's feet of snake, ;)
> 
> 
>>   - be able to share variables between tests (connection object and
>> anything else)
> 
> Not sure what's your exact meaning, could you explain more?
> 
>>   - introduce new config file for tests (.ini format, can be parsed by
>> ConfigParser, same as env.cfg, define variables used throughout the test
>> specifications
> 
> Do you mean destroy current config parsing codes? if so, we need
> to rewrite (or much modification) codes of generator too. Any
> critical disadvantage you see in the current parsing/generator
> codes? I'd think the pricinple of current parsing (no generator)
> is right, though it might have bugs or disadvantages, we can
> improve/extend it.
> 

To answer two of your questions at once, I'll show you an example of
what I had in mind (BTW: one of the current disadvantages is also the
fact that each indentation level _must_ be 4 spaces, otherwise you'll
get an error). Please be aware that this doesn't make almost any sense,
it just shows a couple of things I think could help everyone a lot.

file testcase.cfg:

# Local variables. You can use these variables only in this testcase
# file. This requires 1 more line of code in CofigParser.
[LocalVariables]
my_hyper = "qemu"
module = connections

[GlobalVariables]
# Could be named "Defaults" as these variables will be passed to all
# test in this testcase file...
uri = %(my_hyper)s:///system

# This section is not needed if the tests are named Test1.Connect and
# so on, but it is more readable a understandable for some readers
[Tests]
Test_1 = Connect
Test_2 = Disconnect

[Test.Connect]
Module = $(connections)s
Testcase = connect
# if not specified, this could default to Test.<name>.Params
Params = SomethingElse

[SomethingElse]
# ...unless they are overwritten with some others
uri = %(my_hyper)s:///session

[Test.Connect]
Module = $(connections)s
Testcase = disconnect
# No parameters here (none needed)

And then you will have two tests that look something like this (very
rough idea with lots of things I had in my mind, just to show how nice
it could look):

file tests/connections/connect.py:

def get_params(params):
    # clean the parameters, put defaults for undefined ones, etc.
    return params

# this means that if needed, this test will create/update these
# variables in the "shares" (will get to that in a few lines)
provides = ('connection', 'uri')

def run(logger, test_params, shares):
    # no need to test the return code, we can raise an exception that
    # will be caught outside of the test and reported through logger
    params_cleaned = get_params(test_params)

    # "shares" would be object that takes care of the values shared
    # between tests, check for dependencies, etc.
    shares.update('uri', conn)

    logger.debug('using uri: %s' % params_cleaned['uri'])

    # again, no need to check for an exception
    conn = libvirt.open(params_cleaned['uri'])

    # and for example it could return the values that should be
    # provided in "shares"
    return { 'connection' : conn }

file tests/connections/disconnect.py:

def get_params(params):
    # clean the parameters, put defaults for undefined ones, etc.
    return params

# this can be either here or evaluated when the test is trying to get
# the value from the "shares" object
requires = ('connection',)

def run(logger, test_params, shares):
    params_cleaned = get_params(test_params)
    conn = shares.get('connection')
    logger.info('disconnecting from uri: %s' % conn.getURI())
    conn.close()

And this would be the two tests with test case that tries to connect and
disconnect. All the errors are caught in the test runner, if some tests
depend on exception that is raised in underlying libvirt, they can catch
it themselves without propagating it up (the whole point of exceptions),
etc.

>>   - update the documentation
> 
> Current documentation is in publican format, honestly I'm not fan of
> it, it might be good if used for an enterprise product, but for a
> community project, I never see what uses it. So I will vote if you
> mean destroy it and write just simple text docs.
> 

Actually, I meant at least update the information. However writing a
text doc is good idea (maybe some man-page styled manual?).

>>   - use some python code style (PEP-8?), make use of True/False, None
> 
> pylint should be able to take care of it.
> 
>>   - eliminate duplicated (and x-plicated) code (append_path in all the
>> files, etc.)
> 
> Guannan already starts to do it, :-)
> 
> 
> I didn't go through the codes carefully yet, so no much thoughts
> yet, but what I could think of currently is:
> 
>    * Take use of JEOS, I'd guess for most of the testing, we won't
> do much complicate works inside the guest, so JEOS will be enough.
> 
>    * Add default configuration for generalize testing functions,
> such as for domain installation, we'd want to use it many places,
> and only need to specify specific configuration when testing the
> domain installation itself, for the tests which just wants to
> create a domain, we will just want a usable domain, I.e. lazy
> guys won't want to specify the parameters again and again.
> 
> Will comment if I have further thought when went through the
> codes.
> 
> Regards,
> Osier

Martin