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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
