[Kimchi-devel] [PATCH v2] Remove Non-ASCII chars from tests when unnecessary

Christy Perez christy at linux.vnet.ibm.com
Fri Feb 27 16:38:03 UTC 2015



On 02/27/2015 10:27 AM, Aline Manera wrote:
> 
> On 16/02/2015 19:43, Christy Perez wrote:
>> A domain with non-ASCII characters will fail to start. There are
>> several tests that use such characters, and these tests fail. None
>> of the failing tests are required to test the validity of non-ascii
>> characters, so it's best to remove them and create seperate tests
>> explicitly for these chracters. This way we will not be masking actual
>> issues by ignoring an expected failure.
>>
>> The issue was first noticed with libvirt 1.1.3.
> 
> That is a Fedora issue instead of a libvirt one.
> I have libvirt 1.2.8 in my Ubuntu 14.10 system and I can not reproduce it.

Interesting. I did see mention in a RH bug that it may be a systemd
issue, not a libvirt one, so, I guess I'm not too surprised.

> 
> Also if the issue is only related to domain, why did you change the
> network, storage pool and storage volume tests?

Just in case it breaks in the same way for those things in the future. I
didn't think it made sense to add a special case for non-ascii names in
one test suite but not all.

> 
>> The character issue is discussed in this bug:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1062943
>>
>> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
>> ---
>>   tests/test_model.py               | 112
>> +++++++++++++++++++++++++++-----------
>>   tests/test_model_network.py       |   6 +-
>>   tests/test_model_storagepool.py   |  27 +++++++++
>>   tests/test_model_storagevolume.py |   1 -
>>   4 files changed, 113 insertions(+), 33 deletions(-)
>>
>> diff --git a/tests/test_model.py b/tests/test_model.py
>> index f80f1c9..ada7498 100644
>> --- a/tests/test_model.py
>> +++ b/tests/test_model.py
>> @@ -97,6 +97,44 @@ def test_vm_info(self):
>>           self.assertTrue(info['persistent'])
>>
>>       @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>> +    def test_non_ascii_vm_name(self):
>> +        inst = model.Model(objstore_loc=self.tmp_store)
>> +
>> +        with RollbackContext() as rollback:
>> +            vol_params = {'name': u'test-vol', 'capacity': 1024}
>> +            task = inst.storagevolumes_create(u'default', vol_params)
>> +            rollback.prependDefer(inst.storagevolume_delete, u'default',
>> +                                  vol_params['name'])
>> +            inst.task_wait(task['id'])
>> +            task = inst.task_lookup(task['id'])
>> +            self.assertEquals('finished', task['status'])
>> +            vol = inst.storagevolume_lookup(u'default',
>> vol_params['name'])
>> +
>> +            # Create template
>> +            params = {'name': 'test', 'disks': [{'base': vol['path'],
>> +                                                 'size': 1}],
>> +                      'cdrom': self.kimchi_iso}
>> +            inst.templates_create(params)
>> +            rollback.prependDefer(inst.template_delete, 'test')
>> +
>> +            #  Create VM with non-ascii char name
>> +            params = {'name': u'пeω-∨м', 'template': '/templates/test'}
>> +            inst.vms_create(params)
>> +            rollback.prependDefer(inst.vm_delete, u'пeω-∨м')
>> +
>> +            vms = inst.vms_get_list()
>> +            self.assertTrue(u'пeω-∨м' in vms)
>> +
>> +            # Start VM
>> +            # Note: This fails with libvirt > 1.1.3
>> +            inst.vm_start(u'пeω-∨м')
>> +            info = inst.vm_lookup(u'пeω-∨м')
>> +            self.assertEquals('running', info['state'])
>> +
>> +        vms = inst.vms_get_list()
>> +        self.assertFalse(u'пeω-∨м' in vms)
>> +
>> +    @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
>>       def test_vm_lifecycle(self):
>>           inst = model.Model(objstore_loc=self.tmp_store)
>>
>> @@ -620,6 +658,18 @@ def test_template_storage_customise(self):
>>               params = {'storagepool': '/storagepools/default'}
>>               inst.template_update('test', params)
>>
>> +    def test_non_ascii_template_create(self):
>> +        inst = model.Model('test:///default',
>> +                           objstore_loc=self.tmp_store)
>> +
>> +        with RollbackContext() as rollback:
>> +            params = {'name': u'пeω-teмplate', 'memory': 1024,
>> 'cpus': 1,
>> +                      'cdrom': self.kimchi_iso}
>> +            inst.templates_create(params)
>> +            rollback.prependDefer(inst.template_delete, u'пeω-teмplate')
>> +            info = inst.template_lookup(u'пeω-teмplate')
>> +            self.assertEquals(info['name'], u'пeω-teмplate')
>> +
>>       @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>>       def test_template_create(self):
>>           inst = model.Model('test:///default',
>> @@ -737,7 +787,7 @@ def test_template_update(self):
>>               inst.network_activate(net_name)
>>               rollback.prependDefer(inst.network_deactivate, net_name)
>>
>> -            net_name = u'kīмсhī-пet'
>> +            net_name = u'kimchi-net'
>>               net_args = {'name': net_name,
>>                           'connection': 'nat',
>>                           'subnet': '127.0.20.0/24'}
>> @@ -764,7 +814,7 @@ def test_template_update(self):
>>               self.assertEquals("default", info["networks"][0])
>>
>>               params = {'name': 'new-test', 'memory': 1024, 'cpus': 1,
>> -                      'networks': ['default', 'test-network',
>> u'kīмсhī-пet']}
>> +                      'networks': ['default', 'test-network',
>> u'kimchi-net']}
>>               inst.template_update('new-test', params)
>>               info = inst.template_lookup('new-test')
>>               for key in params.keys():
>> @@ -792,7 +842,7 @@ def test_template_update(self):
>>               self.assertEquals('some-vm', inst.vms_create(params))
>>               rollback.prependDefer(inst.vm_delete, 'some-vm')
>>
>> -            iface_args = {'type': 'network', 'network': u'kīмсhī-пet'}
>> +            iface_args = {'type': 'network', 'network': u'kimchi-net'}
>>               mac = inst.vmifaces_create('some-vm', iface_args)
>>               self.assertEquals(17, len(mac))
>>
>> @@ -865,52 +915,52 @@ def test_vm_edit(self):
>>               self.assertRaises(OperationFailed, inst.vm_update,
>>                                 'kimchi-vm1', {'name': 'kimchi-vm2'})
>>
>> -            params = {'name': u'пeω-∨м', 'cpus': 4, 'memory': 2048}
>> +            params = {'name': u'new-vm', 'cpus': 4, 'memory': 2048}
>>               inst.vm_update('kimchi-vm1', params)
>>               rollback.prependDefer(utils.rollback_wrapper,
>> inst.vm_delete,
>> -                                  u'пeω-∨м')
>> -            self.assertEquals(info['uuid'],
>> inst.vm_lookup(u'пeω-∨м')['uuid'])
>> -            info = inst.vm_lookup(u'пeω-∨м')
>> +                                  u'new-vm')
>> +            self.assertEquals(info['uuid'],
>> inst.vm_lookup(u'new-vm')['uuid'])
>> +            info = inst.vm_lookup(u'new-vm')
>>               for key in params.keys():
>>                   self.assertEquals(params[key], info[key])
>>
>>               # change only VM users - groups are not changed (default
>> is empty)
>>               users = inst.users_get_list()[:3]
>> -            inst.vm_update(u'пeω-∨м', {'users': users})
>> -            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
>> -            self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups'])
>> +            inst.vm_update(u'new-vm', {'users': users})
>> +            self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
>> +            self.assertEquals([], inst.vm_lookup(u'new-vm')['groups'])
>>
>>               # change only VM groups - users are not changed (default
>> is empty)
>>               groups = inst.groups_get_list()[:2]
>> -            inst.vm_update(u'пeω-∨м', {'groups': groups})
>> -            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
>> -            self.assertEquals(groups,
>> inst.vm_lookup(u'пeω-∨м')['groups'])
>> +            inst.vm_update(u'new-vm', {'groups': groups})
>> +            self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
>> +            self.assertEquals(groups,
>> inst.vm_lookup(u'new-vm')['groups'])
>>
>>               # change VM users and groups by adding a new element to
>> each one
>>               users.append(pwd.getpwuid(os.getuid()).pw_name)
>>               groups.append(grp.getgrgid(os.getgid()).gr_name)
>> -            inst.vm_update(u'пeω-∨м', {'users': users, 'groups':
>> groups})
>> -            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
>> -            self.assertEquals(groups,
>> inst.vm_lookup(u'пeω-∨м')['groups'])
>> +            inst.vm_update(u'new-vm', {'users': users, 'groups':
>> groups})
>> +            self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
>> +            self.assertEquals(groups,
>> inst.vm_lookup(u'new-vm')['groups'])
>>
>>               # change VM users (wrong value) and groups
>>               # when an error occurs, everything fails and nothing is
>> changed
>> -            self.assertRaises(InvalidParameter, inst.vm_update,
>> u'пeω-∨м',
>> +            self.assertRaises(InvalidParameter, inst.vm_update,
>> u'new-vm',
>>                                 {'users': ['userdoesnotexist'],
>> 'groups': []})
>> -            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
>> -            self.assertEquals(groups,
>> inst.vm_lookup(u'пeω-∨м')['groups'])
>> +            self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
>> +            self.assertEquals(groups,
>> inst.vm_lookup(u'new-vm')['groups'])
>>
>>               # change VM users and groups (wrong value)
>>               # when an error occurs, everything fails and nothing is
>> changed
>> -            self.assertRaises(InvalidParameter, inst.vm_update,
>> u'пeω-∨м',
>> +            self.assertRaises(InvalidParameter, inst.vm_update,
>> u'new-vm',
>>                                 {'users': [], 'groups':
>> ['groupdoesnotexist']})
>> -            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
>> -            self.assertEquals(groups,
>> inst.vm_lookup(u'пeω-∨м')['groups'])
>> +            self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
>> +            self.assertEquals(groups,
>> inst.vm_lookup(u'new-vm')['groups'])
>>
>>               # change VM users and groups by removing all elements
>> -            inst.vm_update(u'пeω-∨м', {'users': [], 'groups': []})
>> -            self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users'])
>> -            self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups'])
>> +            inst.vm_update(u'new-vm', {'users': [], 'groups': []})
>> +            self.assertEquals([], inst.vm_lookup(u'new-vm')['users'])
>> +            self.assertEquals([], inst.vm_lookup(u'new-vm')['groups'])
>>
>>       def test_multithreaded_connection(self):
>>           def worker():
>> @@ -1090,19 +1140,19 @@ def test_delete_running_vm(self):
>>               inst.templates_create(params)
>>               rollback.prependDefer(inst.template_delete, 'test')
>>
>> -            params = {'name': u'kīмсhī-∨м', 'template':
>> u'/templates/test'}
>> +            params = {'name': 'kimchi-vm', 'template':
>> u'/templates/test'}
>>               inst.vms_create(params)
>>               rollback.prependDefer(utils.rollback_wrapper,
>> inst.vm_delete,
>> -                                  u'kīмсhī-∨м')
>> +                                  'kimchi-vm')
>>
>> -            inst.vm_start(u'kīмсhī-∨м')
>> +            inst.vm_start('kimchi-vm')
>>               rollback.prependDefer(utils.rollback_wrapper,
>> inst.vm_poweroff,
>> -                                  u'kīмсhī-∨м')
>> +                                  'kimchi-vm')
>>
>> -            inst.vm_delete(u'kīмсhī-∨м')
>> +            inst.vm_delete('kimchi-vm')
>>
>>               vms = inst.vms_get_list()
>> -            self.assertFalse(u'kīмсhī-∨м' in vms)
>> +            self.assertFalse('kimchi-vm' in vms)
>>
>>       @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>>       def test_vm_list_sorted(self):
>> diff --git a/tests/test_model_network.py b/tests/test_model_network.py
>> index 5dbe54d..c61f37c 100644
>> --- a/tests/test_model_network.py
>> +++ b/tests/test_model_network.py
>> @@ -99,6 +99,10 @@ class NetworkTests(unittest.TestCase):
>>       def setUp(self):
>>           self.request = partial(request, host, ssl_port)
>>
>> +    def test_non_ascii_net_names(self):
>> +        net = {'name': u'kīмсhī-пet', 'connection': 'isolated'}
>> +        _do_network_test(self, model, net)
>> +
>>       def test_get_networks(self):
>>           networks = json.loads(self.request('/networks').read())
>>           self.assertIn('default', [net['name'] for net in networks])
>> @@ -127,7 +131,7 @@ def test_get_networks(self):
>>
>>       def test_network_lifecycle(self):
>>           # Verify all the supported network type
>> -        networks = [{'name': u'kīмсhī-пet', 'connection': 'isolated'},
>> +        networks = [{'name': u'kimchi-net', 'connection': 'isolated'},
>>                       {'name': u'nat-network', 'connection': 'nat'},
>>                       {'name': u'subnet-network', 'connection': 'nat',
>>                        'subnet': '127.0.100.0/24'}]
>> diff --git a/tests/test_model_storagepool.py
>> b/tests/test_model_storagepool.py
>> index eabf875..de4469d 100644
>> --- a/tests/test_model_storagepool.py
>> +++ b/tests/test_model_storagepool.py
>> @@ -60,6 +60,33 @@ class StoragepoolTests(unittest.TestCase):
>>       def setUp(self):
>>           self.request = partial(request, host, ssl_port)
>>
>> +    def test_non_ascii_storagepool_name(self):
>> +        storagepools = json.loads(self.request('/storagepools').read())
>> +        self.assertIn('default', [pool['name'] for pool in
>> storagepools])
>> +
>> +        with RollbackContext() as rollback:
>> +            name = u'kīмсhī-storagepool'
>> +            req = json.dumps({'name': name, 'type': 'dir',
>> +                              'path':
>> '/var/lib/libvirt/images/test_pool'})
>> +            resp = self.request('/storagepools', req, 'POST')
>> +            rollback.prependDefer(model.storagepool_delete, name)
>> +
>> +            self.assertEquals(201, resp.status)
>> +
>> +            # Verify pool information
>> +            resp = self.request('/storagepools/%s' %
>> name.encode("utf-8"))
>> +            p = json.loads(resp.read())
>> +            keys = [u'name', u'state', u'capacity', u'allocated',
>> +                    u'available', u'path', u'source', u'type',
>> +                    u'nr_volumes', u'autostart', u'persistent']
>> +            self.assertEquals(sorted(keys), sorted(p.keys()))
>> +            self.assertEquals(name, p['name'])
>> +            resp = self.request('/storagepools/%s/activate'
>> +                                % name.encode('utf-8'), req, 'POST')
>> +            rollback.prependDefer(model.storagepool_deactivate, name)
>> +            p = json.loads(resp.read())
>> +            self.assertEquals('active', p['state'])
>> +
>>       def test_get_storagepools(self):
>>           storagepools = json.loads(self.request('/storagepools').read())
>>           self.assertIn('default', [pool['name'] for pool in
>> storagepools])
>> diff --git a/tests/test_model_storagevolume.py
>> b/tests/test_model_storagevolume.py
>> index a3c3ce3..39531e9 100644
>> --- a/tests/test_model_storagevolume.py
>> +++ b/tests/test_model_storagevolume.py
>> @@ -1,4 +1,3 @@
>> -# -*- coding: utf-8 -*-
>>   #
>>   # Project Kimchi
>>   #
>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
> 
> 




More information about the Kimchi-devel mailing list