[Kimchi-devel] [PATCH v2] Remove Non-ASCII chars from tests when unnecessary
Christy Perez
christy at linux.vnet.ibm.com
Wed Mar 11 18:36:55 UTC 2015
On 03/11/2015 10:57 AM, Aline Manera wrote:
>
>
> On 04/03/2015 18:08, Christy Perez wrote:
>> Ping. How do you feel about all the modifications, given my latest
>> reply? I'm willing to update the commit message since it has nothing to
>> do with libvirt. This issue is affecting my ability to use our test
>> suite reliably, so I'd like to make this go away.
>
> Sorry for the delay.
>
> My main concern regarding this patch is we will not cover all the test
> cases when testing a VM with non-ASCII characters.
> For example, from this patch, we will be able to ensure we can create
> and start a VM with non-ASCII characters. But what about cloning, adding
> multiple and different network and storage volumes?
> From the history, we already have problems on those areas: I was able to
> create a Template with non-ASCII but I couldn't update it.
I see your point, and the only way I see around this is to completely
redo the tests so that all operations are tried with ascii-based names,
and then re-run them with some non-ascii names -- but that's overkill.
>
> How is it affecting you? Do the tests stop without passing to all the
> test cases?
The whole test suite doesn't come to a halt, no. They keep going, but
with failures. For instance, test_delete_running_vm fails with the message:
======================================================================
ERROR: test_delete_running_vm (test_model.ModelTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "test_model.py", line 860, in test_delete_running_vm
inst.vm_start(u'kīмсhī-∨м')
File "/home/christy/git/kimchi-upstream/src/kimchi/model/vms.py", line
957, in start
{'name': name, 'err': e.get_error_message()})
OperationFailed: KCHVM0019E: Unable to start virtual machine
k\u012b\u043c\u0441h\u012b-\u2228\u043c. Details: error from service:
CreateMachine: Invalid machine name
----------------------------------------------------------------------
Ran 1 test in 1.361s
FAILED (errors=1)
So that function isn't tested, b/c the VM was never started. Stuff like
that (though not too much of it). And, like I said before, it's only VM
tests. I just change the rest of the tests to be consistent.
Thanks,
- Christy
>
>>
>> Thanks,
>>
>> - Christy
>>
>> On 02/27/2015 10:38 AM, Christy Perez wrote:
>>>
>>> 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
>>>>
>>> _______________________________________________
>>> Kimchi-devel mailing list
>>> Kimchi-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>
>> _______________________________________________
>> 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