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
