Looks good to me. Can you resend entire patch #2?
On Mon, May 4, 2015 at 3:57 PM, Lisa Velden <[email protected]> wrote:
> So, the interdiff then:
>
> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/
> ganeti.backend_unittest.py
> index b1f99a0..08b0301 100755
> --- a/test/py/ganeti.backend_unittest.py
> +++ b/test/py/ganeti.backend_unittest.py
> @@ -953,7 +953,7 @@ class TestSpaceReportingConstants(unittest.TestCase):
> class TestOSEnvironment(unittest.TestCase):
> """Ensure the presence of public and private parameters.
>
> - They have to occur inside os environment variables.
> + They have to be present inside os environment variables.
>
> """
>
> @@ -965,9 +965,9 @@ class TestOSEnvironment(unittest.TestCase):
> osparams_private=serializer.PrivateDict({"private_param":
> "private_info",
>
> "another_private_param":
> - "more_privacy"}))
> + "more_privacy"}),
> + nics = [])
> inst.disks_info = ""
> - inst.nics = []
> inst.secondary_nodes = []
>
> return backend.OSEnvironment(inst, config_mock.CreateOs())
> @@ -975,13 +975,12 @@ class TestOSEnvironment(unittest.TestCase):
> def testParamPresence(self):
> env = self._CreateEnv()
> env_keys = env.keys()
> - env_values = env.values()
> - self.assertIn("OSP_PUBLIC_PARAM", env_keys)
> - self.assertIn("OSP_PRIVATE_PARAM", env_keys)
> - self.assertIn("OSP_ANOTHER_PRIVATE_PARAM", env_keys)
> - self.assertIn("public_info", env_values)
> - self.assertIn("private_info", env_values)
> - self.assertIn("more_privacy", env_values)
> + self.assertIn("OSP_PUBLIC_PARAM", env)
> + self.assertIn("OSP_PRIVATE_PARAM", env)
> + self.assertIn("OSP_ANOTHER_PRIVATE_PARAM", env)
> + self.assertEqual("public_info", env["OSP_PUBLIC_PARAM"])
> + self.assertEqual("private_info", env["OSP_PRIVATE_PARAM"])
> + self.assertEqual("more_privacy", env["OSP_ANOTHER_PRIVATE_PARAM"])
>
>
> On Mon, May 4, 2015 at 2:30 PM, Hrvoje Ribicic <[email protected]> wrote:
>
>> So first comment: usually we send out these as interdiffs to the original
>> patch, and squash them using rebase to have a cleaner commit history.
>> In the end, this should not be a separate patch.
>>
>> On Mon, May 4, 2015 at 1:07 PM, 'Lisa Velden' via ganeti-devel <
>> [email protected]> wrote:
>>
>>> Signed-off-by: Lisa Velden <[email protected]>
>>> ---
>>> test/py/ganeti.backend_unittest.py | 66
>>> ++++++++++++++++++--------------------
>>> 1 file changed, 32 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/
>>> ganeti.backend_unittest.py
>>> index dcb4933..b1f99a0 100755
>>> --- a/test/py/ganeti.backend_unittest.py
>>> +++ b/test/py/ganeti.backend_unittest.py
>>> @@ -46,6 +46,8 @@ from ganeti import objects
>>> from ganeti import pathutils
>>> from ganeti import serializer
>>> from ganeti import utils
>>> +from cmdlib.testsupport.config_mock import ConfigMock
>>> +
>>>
>>> class TestX509Certificates(unittest.TestCase):
>>> def setUp(self):
>>> @@ -949,42 +951,38 @@ class
>>> TestSpaceReportingConstants(unittest.TestCase):
>>>
>>>
>>> class TestOSEnvironment(unittest.TestCase):
>>> - """Ensures presence of public and private parameters inside
>>> - os environment variables"""
>>> -
>>> - def _create_env(self):
>>> - """Creates and returns an environment"""
>>> - inst = objects.Instance(name="test.example.com",
>>> - uuid="test-uuid",
>>> - disks=[], nics=[],
>>> - disks_info="",
>>> - disk_template=constants.DT_DISKLESS,
>>> - primary_node="node.example.com",
>>> - os="debian-image",
>>> - osparams={"public_param": "public_info"},
>>> - osparams_private=
>>> - serializer.PrivateDict({"private_param":
>>> - "private_info"}),
>>> - secondary_nodes=[],
>>> - beparams={},
>>> - hvparams={})
>>> -
>>> - # supply required os attributes in a mock object
>>> - os_mock = mock.Mock()
>>> - os_mock.api_versions = [15]
>>> - os_mock.supported_variants = [""]
>>> -
>>> - return backend.OSEnvironment(inst, os_mock)
>>> + """Ensure the presence of public and private parameters.
>>> +
>>> + They have to occur inside os environment variables.
>>>
>>
>> s/occur/be present/
>>
>>
>>> +
>>> + """
>>> +
>>> + def _CreateEnv(self):
>>> + """Create and return an environment."""
>>> + config_mock = ConfigMock()
>>> + inst = config_mock.AddNewInstance(
>>> + osparams={"public_param": "public_info"},
>>> + osparams_private=serializer.PrivateDict({"private_param":
>>> + "private_info",
>>> +
>>> "another_private_param":
>>> + "more_privacy"}))
>>> + inst.disks_info = ""
>>> + inst.nics = []
>>>
>>
>> If you do need this to be empty, you can pass it into the AddNewInstance
>> constructor.
>> It's better to have it there as it is not a property that gets filled in
>> later.
>>
>>
>>> + inst.secondary_nodes = []
>>> +
>>> + return backend.OSEnvironment(inst, config_mock.CreateOs())
>>>
>>> def testParamPresence(self):
>>> - env = self._create_env()
>>> - self.assertTrue('OSP_PUBLIC_PARAM' in env)
>>> - self.assertTrue('OSP_PRIVATE_PARAM' in env)
>>> -
>>> - def testParamValues(self):
>>> - env = self._create_env()
>>> - self.assertEqual(env['OSP_PUBLIC_PARAM'], "public_info")
>>> - self.assertEqual(env['OSP_PRIVATE_PARAM'], "private_info")
>>> + env = self._CreateEnv()
>>> + env_keys = env.keys()
>>> + env_values = env.values()
>>> + self.assertIn("OSP_PUBLIC_PARAM", env_keys)
>>> + self.assertIn("OSP_PRIVATE_PARAM", env_keys)
>>> + self.assertIn("OSP_ANOTHER_PRIVATE_PARAM", env_keys)
>>>
>>
>> Hmm, would this not work if you just passed in the dictionary instead of
>> the list of keys into assertIn?
>>
>>
>>> + self.assertIn("public_info", env_values)
>>> + self.assertIn("private_info", env_values)
>>> + self.assertIn("more_privacy", env_values)
>>>
>>
>> Here you lose the mapping between values and keys - please use
>> assertEqual with env[key] instead of assertIn here.
>>
>>
>>> +
>>>
>>> if __name__ == "__main__":
>>> testutils.GanetiTestProgram()
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>>>
>>>
>> Hrvoje Ribicic
>> Ganeti Engineering
>> Google Germany GmbH
>> Dienerstr. 12, 80331, München
>>
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
>> Geschäftsführer: Graham Law, Christine Elizabeth Flores
>> Steuernummer: 48/725/00206
>> Umsatzsteueridentifikationsnummer: DE813741370
>>
>
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
