On Thu, Apr 23, 2015 at 4:49 PM, 'Lisa Velden' via ganeti-devel < [email protected]> wrote:
> Make sure that there is an environment variable for each parameter > with the correct value. > > Signed-off-by: Lisa Velden <[email protected]> > --- > test/py/ganeti.backend_unittest.py | 40 > +++++++++++++++++++++++++++++++++++++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ > ganeti.backend_unittest.py > index 1f05197..dcb4933 100755 > --- a/test/py/ganeti.backend_unittest.py > +++ b/test/py/ganeti.backend_unittest.py > @@ -44,9 +44,9 @@ from ganeti import hypervisor > from ganeti import netutils > from ganeti import objects > from ganeti import pathutils > +from ganeti import serializer > from ganeti import utils > > - > To be consistent with other test files, please re-insert this line. Pedantry, but unnecessary discrepancies should be avoided. > class TestX509Certificates(unittest.TestCase): > def setUp(self): > self.tmpdir = tempfile.mkdtemp() > @@ -948,5 +948,43 @@ class TestSpaceReportingConstants(unittest.TestCase): > self.assertEqual(None, backend._STORAGE_TYPE_INFO_FN[storage_type]) > > > +class TestOSEnvironment(unittest.TestCase): > + """Ensures presence of public and private parameters inside > + os environment variables""" > Please reformat this docstring according to: http://docs.ganeti.org/ganeti/master/html/dev-codestyle.html#docstrings > + > + def _create_env(self): > For helper functions, we still use capitalization: e.g. _CreateEnv > + """Creates and returns an environment""" > Same for this docstring. > + 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"}), > I would throw in another private parameter here - it could be that the conversion is mangled in some way and only processes one entry. Being paranoid while testing is useful! > + secondary_nodes=[], > + beparams={}, > + hvparams={}) > Given that you are replicating a lot of the values provided by the logic of config_mock, it would probably be better to instantiate a mock config and create an instance. This would keep this test more focused on private params. > + > + # 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) > + > + def testParamPresence(self): > + env = self._create_env() > + self.assertTrue('OSP_PUBLIC_PARAM' in env) > There is an assertIn function which will provide you with better error reporting. > + self.assertTrue('OSP_PRIVATE_PARAM' in env) > Our style doc mandates double quotes for strings. > + > + def testParamValues(self): > + env = self._create_env() > + self.assertEqual(env['OSP_PUBLIC_PARAM'], "public_info") > + self.assertEqual(env['OSP_PRIVATE_PARAM'], "private_info") > While I understand where you were going with this, I think these two tests should be combined. One will necessarily fail without the other, so they will supply the same amount of information either way. By using assertIn, the error messages emitted will be good enough for the distinction to be intuitive. > + > Two empty lines needed 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
