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

[Daniel Henrique Barboza]

On 03/20/2015 10:27 AM, Aline Manera wrote:
>
> Does this patch replace the former [Kimchi-devel] [PATCH] 
> tests/test_osinfo.py: fixes for Power architecture ?

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

Ok! I'll create 2 imports instead of breaking the import line
>
>>   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'))
>