Change in vdsm[master]: configurator doesn't load pyc files under configurators folder
nsoffer at redhat.com
nsoffer at redhat.com
Thu Sep 10 12:04:05 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: configurator doesn't load pyc files under configurators folder
......................................................................
Patch Set 7:
(4 comments)
https://gerrit.ovirt.org/#/c/45846/7/tests/toolTests.py
File tests/toolTests.py:
Line 80:
Line 81: class PatchConfiguratorsTests(VdsmTestCase):
Line 82:
Line 83: def tempModuleFolder(self, files):
Line 84: path = tempfile.mkdtemp()
path should be an argument, so you create and delete the directory in the same place.
Creating a temporary in one place and deleting in another place is a bad pattern.
Line 85: for f in files:
Line 86: utils.touchFile('%s/%s' % (path, f))
Line 87: return path
Line 88:
Line 82:
Line 83: def tempModuleFolder(self, files):
Line 84: path = tempfile.mkdtemp()
Line 85: for f in files:
Line 86: utils.touchFile('%s/%s' % (path, f))
Use os.path.join
Line 87: return path
Line 88:
Line 89: def listModuleFiles(self, files, expected_modules):
Line 90: path = self.tempModuleFolder(files)
Line 89: def listModuleFiles(self, files, expected_modules):
Line 90: path = self.tempModuleFolder(files)
Line 91: expected = set(expected_modules)
Line 92: result = configurator._listmodules(path)
Line 93: utils.rmTree(path)
Please use testlib.temporaryDir:
with testlib.temporaryDir() as tmp:
create files
Line 94: self.assertEquals(
Line 95: result,
Line 96: expected,
Line 97: )
Line 96: expected,
Line 97: )
Line 98:
Line 99: def testOnlyPyModules(self):
Line 100: self.listModuleFiles(('a.py', 'b.py'), ('a', 'b'))
This is nice, but we better use one test with permuttions:
@permutaions([
# files, modules
(("a.py", "b.py"), ("a", "b")),
...
])
def test_list_modules(self, files, modules):
with temp dir:
create files...
list modules...
assert...
Line 101:
Line 102: def testAdditionalPyPerfixModule(self):
Line 103: self.listModuleFiles(('a.py', 'b.py', 'a.pyc', 'a.pyioas'), ('a', 'b'))
Line 104:
--
To view, visit https://gerrit.ovirt.org/45846
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia529de0069e2f4ec168a4b9df82ba62c56d66730
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list