LGTM, thanks
On Mon, Jun 29, 2015 at 2:08 PM, Lisa Velden <[email protected]> wrote:
> Interdiff:
>
> diff --git a/test/py/ganeti.mcpu_unittest.py b/test/py/
> ganeti.mcpu_unittest.py
> index 15a2505..54bd640 100755
> --- a/test/py/ganeti.mcpu_unittest.py
> +++ b/test/py/ganeti.mcpu_unittest.py
> @@ -180,10 +180,20 @@ class TestProcessResult(unittest.TestCase):
>
> class TestSecretParams(unittest.TestCase):
> def testSecretParamsCheckNoError(self):
> - src = opcodes.OpTestDummy()
> + op = opcodes.OpInstanceCreate(
> + instance_name="plain.example.com",
> + pnode="master.example.com",
> + disk_template=constants.DT_PLAIN,
> + mode=constants.INSTANCE_CREATE,
> + nics=[{}],
> + disks=[{
> + constants.IDISK_SIZE: 1024
> + }],
> + osparams_secret= serializer.PrivateDict({"foo":"bar",
> "foo2":"bar2"}),
> + os_type="debian-image")
>
> try:
> - mcpu._CheckSecretParameters(src)
> + mcpu._CheckSecretParameters(op)
> except errors.OpPrereqError:
> self.fail("OpPrereqError raised unexpectedly in
> _CheckSecretParameters")
>
> @@ -201,12 +211,7 @@ class TestSecretParams(unittest.TestCase):
>
> "secret_param":"<redacted>"}),
> os_type="debian-image")
>
> - try:
> - mcpu._CheckSecretParameters(op)
> - except errors.OpPrereqError:
> - pass
> - else:
> - self.fail
> + self.assertRaises(errors.OpPrereqError, mcpu._CheckSecretParameters,
> op)
>
>
> if __name__ == "__main__":
>
>
> On Mon, Jun 29, 2015 at 1:25 PM, Hrvoje Ribicic <[email protected]> wrote:
>
>> On Fri, Jun 26, 2015 at 10:38 PM, 'Lisa Velden' via ganeti-devel <
>> [email protected]> wrote:
>>
>>> Test if OpPrereqError is raised only for redacted values.
>>>
>>> Signed-off-by: Lisa Velden <[email protected]>
>>> ---
>>> test/py/ganeti.mcpu_unittest.py | 33 +++++++++++++++++++++++++++++++++
>>> 1 file changed, 33 insertions(+)
>>>
>>> diff --git a/test/py/ganeti.mcpu_unittest.py b/test/py/
>>> ganeti.mcpu_unittest.py
>>> index 18fabd8..15a2505 100755
>>> --- a/test/py/ganeti.mcpu_unittest.py
>>> +++ b/test/py/ganeti.mcpu_unittest.py
>>> @@ -35,10 +35,12 @@ import unittest
>>> import itertools
>>>
>>> from ganeti import compat
>>> +from ganeti import errors
>>> from ganeti import mcpu
>>> from ganeti import opcodes
>>> from ganeti import cmdlib
>>> from ganeti import locking
>>> +from ganeti import serializer
>>> from ganeti import constants
>>> from ganeti.constants import \
>>> LOCK_ATTEMPTS_TIMEOUT, \
>>> @@ -176,5 +178,36 @@ class TestProcessResult(unittest.TestCase):
>>> self.assertEqual(op2.debug_level, 3)
>>>
>>>
>>> +class TestSecretParams(unittest.TestCase):
>>> + def testSecretParamsCheckNoError(self):
>>> + src = opcodes.OpTestDummy()
>>>
>>
>> This test could be improved a lot if you were to additionally test with
>> an opcode with secret parameters which are not redacted.
>>
>>
>>> +
>>> + try:
>>> + mcpu._CheckSecretParameters(src)
>>> + except errors.OpPrereqError:
>>> + self.fail("OpPrereqError raised unexpectedly in
>>> _CheckSecretParameters")
>>> +
>>>
>>
>> As an error is going to be reported whenever an exception is raised and
>> the test is small enough that you can pinpoint the error easily, you do not
>> have to feel obliged to have scaffolding like this.
>> Now that it is here, it can stay in place.
>>
>>
>>
>>> + def testSecretParamsCheckWithError(self):
>>> + op = opcodes.OpInstanceCreate(
>>> + instance_name="plain.example.com",
>>> + pnode="master.example.com",
>>> + disk_template=constants.DT_PLAIN,
>>> + mode=constants.INSTANCE_CREATE,
>>> + nics=[{}],
>>> + disks=[{
>>> + constants.IDISK_SIZE: 1024
>>> + }],
>>> + osparams_secret= serializer.PrivateDict({"foo":"bar",
>>> +
>>> "secret_param":"<redacted>"}),
>>> + os_type="debian-image")
>>> +
>>> + try:
>>> + mcpu._CheckSecretParameters(op)
>>> + except errors.OpPrereqError:
>>> + pass
>>> + else:
>>> + self.fail
>>>
>>
>> Please use assertRaises here.
>>
>> You should also always ensure that your tests can fail - self.fail is a
>> function, not a function invocation, and Python will neither complain nor
>> mark the test as failed with this code.
>>
>>
>>> +
>>> +
>>> if __name__ == "__main__":
>>> testutils.GanetiTestProgram()
>>> --
>>> 2.4.3.573.g4eafbef
>>>
>>>
>> Hrvoje Ribicic
>> Ganeti Engineering
>> Google Germany GmbH
>> Dienerstr. 12, 80331, München
>>
>> Geschäftsführer: Graham Law, Christine Elizabeth Flores
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
>>
>
>
>
> --
> Lisa Velden
> Software Engineer
> [email protected]
>
> Google Germany GmbH
> Dienerstraße 12
> 80331 München
>
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
