On Fri, Jun 10, 2016 at 10:02 AM, Iustin Pop <ius...@google.com> wrote:
> 2016-06-10 10:58 GMT+02:00 Viktor Bachraty <vbachr...@google.com>:
>
>>
>> On Thu, Jun 9, 2016 at 11:33 PM, Iustin Pop <ius...@k1024.org> wrote:
>>
>>> From: Iustin Pop <ius...@google.com>
>>>
>>> Commit 8b2ec2f added unittests for KVM pinning, but it introduced a
>>> non-obvious
>>> local dependency in the tests: the CPU_PINNING_OFF calls work by looking
>>> at the
>>> (current) machine's core count, and pinning to all those CPUs. In order
>>> to make
>>> this work independently from the test machine, we must also mock the
>>> result of
>>> process.cpu_count(). Do this by using a core count that is very much
>>> unlikely
>>> to ever be present in the real world.
>>>
>>> Signed-off-by: Iustin Pop <ius...@google.com>
>>> ---
>>> test/py/ganeti.hypervisor.hv_kvm_unittest.py | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py b/test/py/
>>> ganeti.hypervisor.hv_kvm_unittest.py
>>> index c7a53b5..55ffb9b 100755
>>> --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
>>> +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
>>> @@ -37,6 +37,7 @@ import socket
>>> import os
>>> import struct
>>> import re
>>> +from contextlib import nested
>>>
>>> from ganeti import serializer
>>> from ganeti import constants
>>> @@ -636,12 +637,13 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>>> cpu_mask = self.params['cpu_mask']
>>> worker_cpu_mask = self.params['worker_cpu_mask']
>>> hypervisor = hv_kvm.KVMHypervisor()
>>> - with mock.patch('psutil.Process', return_value=mock_process):
>>> + with nested(mock.patch('psutil.Process', return_value=mock_process),
>>> + mock.patch('psutil.cpu_count', return_value=1237)):
>>>
>>
>> 'nested' is deprecated in Python 2.7 but unfortunately 2.6 does not
>> support comma separated contexts as 2.7 does.
>>
>
> Yep, saw that. However, nested still works (no warnings) in 2.6 as well,
> so it should be good for now.
>
>
>> All Debian versions newer than Squeeze should have 2.7 already. I believe
>> once master goes stable, we can safely drop support for Python <2.7, what
>> do you think?
>>
>
> It is possible, but if we want to do that, I think we should do it across
> the board. E.g. on Haskell side we have a number of workaround for old
> compilers/library versions, and we could do some cleanup if we remove that,
> but we don't need to hurry.
>
> So declaring that some version (e.g. 2.19, or 2.20 - that is a round
> number) is only supported on (Jessie+, or 2015+, or something) would be a
> good move.
>
I tend see too much backwards compatibility more as a burden than a
feature, but agree that we should do declare/decide about it at release
time, not in a small patch.
LGTM.
thanks for your patches,
viktor
>
> thanks,
> iustin
>
> hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
>>> worker_cpu_mask)
>>>
>>> self.assertEqual(mock_process.set_cpu_affinity.call_count, 1)
>>> self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
>>> - mock.call(range(0,12)))
>>> + mock.call(range(0,1237)))
>>>
>>> def testCpuPinningPerVcpu(self):
>>> mock_process = mock.MagicMock()
>>> @@ -659,11 +661,12 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>>> def get_mock_process(unused_pid):
>>> return mock_process
>>>
>>> - with mock.patch('psutil.Process', get_mock_process):
>>> + with nested(mock.patch('psutil.Process', get_mock_process),
>>> + mock.patch('psutil.cpu_count', return_value=1237)):
>>> hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
>>> worker_cpu_mask)
>>> self.assertEqual(mock_process.set_cpu_affinity.call_count, 7)
>>> self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
>>> - mock.call(range(0,12)))
>>> + mock.call(range(0,1237)))
>>> self.assertEqual(mock_process.set_cpu_affinity.call_args_list[6],
>>> mock.call([15, 16, 17]))
>>>
>>> --
>>> 2.8.1
>>>
>>>
>>
>
