Remove recursion from import_modules_recursively

Since the function already uses os.walk that will iterate through all
subdirectories for us, there is no need to recursively call the
function.

The recursive call was not just redundant, but also had another problem,
where we were passing relative paths to subdirectories, which made the
function to attempt importing modules that are not located under the
topdir. It could crash, or, worse, import a module that is
not under the expected parent directory.

This patch also modifies an existing unit test for the function to
validate that modules from subdirectories are also imported, even
without the recursive call.

Change-Id: I8b3a844460e4987b8a8375b01353d01e57d91604
Closes-Bug: #1634735
This commit is contained in:
Martin Matyáš 2016-10-18 21:05:15 -07:00 committed by Ihar Hrachyshka
parent d2a28a8c73
commit 676257cea0
4 changed files with 10 additions and 7 deletions

View File

@ -838,8 +838,6 @@ def import_modules_recursively(topdir):
importlib.import_module(module)
modules.append(module)
for dir_ in dirs:
modules.extend(import_modules_recursively(dir_))
return modules

View File

@ -811,10 +811,15 @@ class TestExcDetails(base.BaseTestCase):
class ImportModulesRecursivelyTestCase(base.BaseTestCase):
def test_object_modules(self):
example_module = 'neutron.tests.unit.tests.example.dir.example_module'
sys.modules.pop(example_module, None)
def test_recursion(self):
expected_modules = (
'neutron.tests.unit.tests.example.dir.example_module',
'neutron.tests.unit.tests.example.dir.subdir.example_module',
)
for module in expected_modules:
sys.modules.pop(module, None)
modules = utils.import_modules_recursively(
os.path.dirname(tests.__file__))
self.assertIn(example_module, modules)
self.assertIn(example_module, sys.modules)
for module in expected_modules:
self.assertIn(module, modules)
self.assertIn(module, sys.modules)