[Kimchi-devel] [PATCH 1/3] tests/test_osinfo.py: fixes for Power architecture

[Aline Manera]

Does this patch replace the former [Kimchi-devel] [PATCH] 
tests/test_osinfo.py: fixes for Power architecture ?

Some comments below:

On 16/03/2015 16:29, Daniel Henrique Barboza wrote:
> - osinfo.py now has a function that returns a specific
> field from a template configuration (old or modern) which
> considers the current running arch.
>
> - a new test was added to explictly test the case where an
> unknown distro/version should return the configuration of
> an old distro/version after the lookup.
>
> Signed-off-by: Daniel Henrique Barboza <dhbarboza82 at gmail.com>
> ---
>   src/kimchi/osinfo.py |  9 +++++++++
>   tests/test_osinfo.py | 36 ++++++++++++++++++++++++++----------
>   2 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py
> index 2496e03..3c31346 100644
> --- a/src/kimchi/osinfo.py
> +++ b/src/kimchi/osinfo.py
> @@ -104,6 +104,15 @@ def _get_arch():
>               return arch
>
>
> +def get_template_default(template_type, field):
> +    host_arch = _get_arch()
> +    # Assuming 'power' = 'ppc64le' because lookup() does the same,
> +    # claiming libvirt compatibility.
> +    if host_arch in ('power', 'ppc64le'):
> +        return template_specs['power'][template_type][field]
> +    return template_specs[host_arch][template_type][field]
> +
> +
>   def lookup(distro, version):
>       """
>       Lookup all parameters needed to run a VM of a known or unknown operating
> diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py
> index d5e90b4..0185f52 100644
> --- a/tests/test_osinfo.py
> +++ b/tests/test_osinfo.py
> @@ -20,7 +20,8 @@
>   import unittest
>
>
> -from kimchi.osinfo import lookup, modern_version_bases, _get_arch
> +from kimchi.osinfo import _get_arch, get_template_default, lookup, \
> +    modern_version_bases
>

We usually do not break the import line.
Instedd of that we create 2 import lines

from kimchi.osinfo import ...
from kimchi.osinfo import ...

>   class OSInfoTests(unittest.TestCase):
> @@ -35,20 +36,35 @@ class OSInfoTests(unittest.TestCase):
>                           'centos': '5.1', 'rhel': '5.1', 'fedora': '15'}
>           for distro, version in old_versions.iteritems():
>               entry = lookup(distro, version)
> -            self.assertEquals(entry['disk_bus'], 'ide')
> -            self.assertEquals(entry['nic_model'], 'e1000')
> +            self.assertEquals(entry['disk_bus'],
> +                              get_template_default('old', 'disk_bus'))
> +            self.assertEquals(entry['nic_model'],
> +                              get_template_default('old', 'nic_model'))
>
>       def test_modern_bases(self):
>           for distro, version in modern_version_bases[_get_arch()].iteritems():
>               entry = lookup(distro, version)
> -            self.assertEquals(entry['disk_bus'], 'virtio')
> -            self.assertEquals(entry['nic_model'], 'virtio')
> +            self.assertEquals(entry['disk_bus'],
> +                              get_template_default('modern', 'disk_bus'))
> +            self.assertEquals(entry['nic_model'],
> +                              get_template_default('modern', 'nic_model'))
>
>       def test_modern_distros(self):
> -        modern_versions = {'debian': '7.0', 'ubuntu': '12.04',
> -                           'opensuse': '12.3', 'centos': '6.4', 'rhel': '6.3',
> -                           'fedora': '18', 'gentoo': '12.1'}
> +        # versions based on ppc64 modern distros
> +        modern_versions = {'ubuntu': '14.04', 'opensuse': '13.1',
> +                           'rhel': '6.5', 'fedora': '19', 'sles': '11sp3'}
>           for distro, version in modern_versions.iteritems():
>               entry = lookup(distro, version)
> -            self.assertEquals(entry['disk_bus'], 'virtio')
> -            self.assertEquals(entry['nic_model'], 'virtio')
> +            self.assertEquals(entry['disk_bus'],
> +                              get_template_default('modern', 'disk_bus'))
> +            self.assertEquals(entry['nic_model'],
> +                              get_template_default('modern', 'nic_model'))
> +
> +    def test_lookup_unknown_distro_version_returns_old_distro(self):
> +        distro = 'unknown_distro'
> +        version = 'unknown_version'
> +        entry = lookup(distro, version)
> +        self.assertEquals(entry['disk_bus'],
> +                          get_template_default('old', 'disk_bus'))
> +        self.assertEquals(entry['nic_model'],
> +                          get_template_default('old', 'nic_model'))