From Dan Kenigsberg <[email protected]>: Dan Kenigsberg has posted comments on this change.
Change subject: tests: Avoid global state modification using forked ...................................................................... Patch Set 1: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/67338/1/tests/moduleloader_test.py File tests/moduleloader_test.py: Line 39: for f in files: Line 40: utils.touchFile(os.path.join(path, f)) Line 41: utils.touchFile(os.path.join(path, '__init__.py')) Line 42: sys.path.append(os.path.dirname(path)) Line 43: yield importlib.import_module(os.path.basename(path)) it's true that we leave garbage in sys.path. We must clean it back to its original state after yield. Line 44: Line 45: @permutations( Line 46: [(('a.py', 'b.py'), ('a', 'b')), Line 47: (('a.py', 'b.py', 'a.pyc', 'a.pyioas'), ('a', 'b')), Line 47: (('a.py', 'b.py', 'a.pyc', 'a.pyioas'), ('a', 'b')), Line 48: (('a.py', 'b.py', 'a.pyc', 'a.py'), ('a', 'b')), Line 49: (('a.py', 'b.py', 'a.pyc', '_my.py'), ('a', 'b', '_my'))] Line 50: ) Line 51: @forked I do not see how running this in another process is differnt from @broken_on_ci - both hide the problem without really understanding it. I prefer the explicit @broken_on_ci. Line 52: def test_import_modules(self, files, expected_modules): Line 53: with self._setup_test_modules(files) as module_name: Line 54: result = moduleloader.load_modules(module_name) Line 55: -- To view, visit https://gerrit.ovirt.org/67338 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2130bcf39918c3c70ed9f8eb9c436c5c8f9c6c09 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Irit Goihman <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- [email protected] To unsubscribe send an email to [email protected]
