Re: [libvirt] [test-API PATCH 2/6] Added support for Gentoo

On 03/24/2012 06:42 PM, Martin Kletzander wrote: > In order to support libvirt-test-API on more distributions, this > commit adds support for Gentoo. > The file is copy-paste from dist/redhat/env_update.py just modified to > make the get_* functions work on Gentoo, some removed. Probably no one else that uses the test api uses Gentoo, so I'll do a review of this patch. > --- > dist/gentoo/env_inspect.py | 98 > ++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 98 insertions(+), 0 deletions(-) > create mode 100644 dist/gentoo/__init__.py > create mode 100644 dist/gentoo/env_inspect.py > > diff --git a/dist/gentoo/__init__.py b/dist/gentoo/__init__.py > new file mode 100644 > index 0000000..e69de29 > diff --git a/dist/gentoo/env_inspect.py b/dist/gentoo/env_inspect.py > new file mode 100644 > index 0000000..e8fccc0 > --- /dev/null > +++ b/dist/gentoo/env_inspect.py > @@ -0,0 +1,98 @@ > +#!/usr/bin/env python > +# > +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc. s/2010, //

> +# libvirt-test-API is free software: you can redistribute it and/or > modify it > +# under the terms of the GNU General Public License as published by > +# the Free Software Foundation, either version 2 of the License, or > +# (at your option) any later version. This program is distributed in > +# the hope that it will be useful, but WITHOUT ANY WARRANTY; without > +# even the implied warranties of TITLE, NON-INFRINGEMENT, > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > +# > +# The GPL text is available in the file COPYING that accompanies this > +# distribution and at<http://www.gnu.org/licenses>;. > +# > +# Filename: envinspect.py s/envinspect/env_inspect/ > +# Summary: To generate a callable class for clearing testing environment > +# Description: The module match the reference of clearing function > +# from each testcase to the corresponding testcase's > +# argument in the order of testcase running A little re-wording would help to understand what's going on here.

> + > +import os > +import portage > + > +vt_dbapi = portage.db[portage.root]['vartree'].dbapi > + > +def get_libvirt_ver(): > + pkg = vt_dbapi.match('app-emulation/libvirt') > + if pkg: > + return 0, pkg[-1] > + else: > + return 100, "No libvirt installed" > + > +def get_libvirt_pyth_ver(): > + pkgs = vt_dbapi.match('app-emulation/libvirt') > + for pkg in pkgs: > + if 'python' in vt_dbapi.aux_get(pkg, ['USE'])[0].split(): > + return 0, '%s[python]' % pkg > + > + return 100, "USE flag 'python' not enabled for libvirt" > + > +def get_qemu_kvm_ver(): > + pkg = vt_dbapi.match('qemu-kvm') or vt_dbapi.match('qemu') > + if pkg: > + return 0, pkg[-1] > + else: > + return 100, "No qemu installed" This function returns codes 0 or 100 ... > + > +def get_kernel_ver(): > + # on Gentoo, there is no need to check for kernel > + return 0, os.uname()[2] > + > + > +class EnvInspect(object): > + """to check and collect the testing enviroment infomation > + before performing testing > + """ > + > + def __init__(self, logger): > + self.logger = logger > + > + def env_checking(self): > + flag = 0 > + result = "" > + if get_libvirt_ver()[0] == 100: > + result = NOTOK > + flag = 1 > + else: > + result = OK > + self.logger.info(" %-36s%-6s" % (get_libvirt_ver()[1], > result)) > + > + if get_libvirt_pyth_ver()[0] == 100: > + result = NOTOK > + flag = 1 > + else: > + result = OK > + self.logger.info(" %-36s%-6s" % > (get_libvirt_pyth_ver()[1], result)) > + > + if get_qemu_kvm_ver()[0] == 150 and flag == 0: > + flag = 0 > + elif get_qemu_kvm_ver()[0] == 150 and flag == 1: > + flag = 1 ... so these tests here are not needed. (And are quite strange too ... but that's probably a cut&paste leftover) > + else: > + pass > + self.logger.info(" %-36s%-6s" % (get_qemu_kvm_ver()[1], OK)) And in any case, the test succeeds, so it's probably ment to be only a version check, that is not mandatory. IMO we should make this check mandatory for now, as most of the tests use the local hypervisor for testing machines and only a few tests actualy deal with remote code. > + > + if get_kernel_ver()[0] == 100: > + result = NOTOK > + flag = 1 > + else: Well, as was said in the comment, on Gentoo the kernel test always succeeds, so the return value check is not necessary and can be changed to something like: self.logger.info(" %-36s%-6s" % (get_kernel_ver()[1], OK)) and leave out the condition.

> + result = OK > + self.logger.info(" %-36s%-6s" % (get_kernel_ver()[1], > result)) > + > + return flag > + > + > +OK = "ok" > +NOTOK = "not ok" In any case, it works correctly on Gentoo and the tests are faster than querying the package states with equery. Probably worth a v2 where you change the test conditions. Peter