From ee3fe1fa27109796d2783b4d6cabdf8b14b37cf8 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 29 May 2018 10:55:42 +0200 Subject: [PATCH 1/5] Use abs path if the given filename isn't a dir * Use the absolute path of the filename if the normalized path is not a directory. --- reframe/utility/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index ae0bb6152c..f955a217a0 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -43,6 +43,8 @@ def import_module_from_file(filename): filename = os.path.normpath(os.path.expandvars(filename)) if os.path.isdir(filename): filename = os.path.join(filename, '__init__.py') + else: + filename = os.path.abspath(filename) module_name = _get_module_name(filename) if os.path.isabs(filename): From f1aa8ebe7c4906033dc2db26b29d45b78de8b51d Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 29 May 2018 13:24:01 +0200 Subject: [PATCH 2/5] Add unittests for paths starting with double dot --- unittests/test_utility.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index f1a40fa634..0d8b9fc28d 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -280,6 +280,22 @@ def test_load_unknown_path(self): self.assertEqual('foo', e.name) self.assertEqual('/foo', e.path) + def test_load_dir_starts_with_ddot(self): + parent_dir = os.path.basename(os.path.abspath(os.pardir)) + filename = '../%s/reframe' % parent_dir + module = util.import_module_from_file(filename) + self.assertEqual(reframe.VERSION, module.VERSION) + self.assertEqual('reframe', module.__name__) + self.assertIs(module, sys.modules.get('reframe')) + + def test_load_file_starts_with_ddot(self): + parent_dir = os.path.basename(os.path.abspath(os.pardir)) + filename = '../%s/reframe/__init__.py' % parent_dir + module = util.import_module_from_file(filename) + self.assertEqual(reframe.VERSION, module.VERSION) + self.assertEqual('reframe', module.__name__) + self.assertIs(module, sys.modules.get('reframe')) + def test_load_twice(self): filename = os.path.abspath('reframe/__init__.py') module1 = util.import_module_from_file(filename) From bcf722efe6ca8cb071eafd76dd135b7e6a72fff8 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Fri, 1 Jun 2018 10:49:18 +0200 Subject: [PATCH 3/5] Address PR comments --- reframe/utility/__init__.py | 4 +++- unittests/test_utility.py | 24 ++++++++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index f955a217a0..0566026ec5 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -41,9 +41,11 @@ def import_module_from_file(filename): """Import module from file.""" filename = os.path.normpath(os.path.expandvars(filename)) + if os.path.isdir(filename): filename = os.path.join(filename, '__init__.py') - else: + + if filename.startswith('..'): filename = os.path.abspath(filename) module_name = _get_module_name(filename) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 0d8b9fc28d..794c0e4766 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -281,20 +281,20 @@ def test_load_unknown_path(self): self.assertEqual('/foo', e.path) def test_load_dir_starts_with_ddot(self): - parent_dir = os.path.basename(os.path.abspath(os.pardir)) - filename = '../%s/reframe' % parent_dir - module = util.import_module_from_file(filename) - self.assertEqual(reframe.VERSION, module.VERSION) - self.assertEqual('reframe', module.__name__) - self.assertIs(module, sys.modules.get('reframe')) + with os_ext.change_dir('reframe'): + filename = '..%s/reframe' % os.getcwd() + module = util.import_module_from_file(filename) + self.assertEqual(reframe.VERSION, module.VERSION) + self.assertEqual('reframe', module.__name__) + self.assertIs(module, sys.modules.get('reframe')) def test_load_file_starts_with_ddot(self): - parent_dir = os.path.basename(os.path.abspath(os.pardir)) - filename = '../%s/reframe/__init__.py' % parent_dir - module = util.import_module_from_file(filename) - self.assertEqual(reframe.VERSION, module.VERSION) - self.assertEqual('reframe', module.__name__) - self.assertIs(module, sys.modules.get('reframe')) + with os_ext.change_dir('reframe'): + filename = '..%s/reframe/__init__.py' % os.getcwd() + module = util.import_module_from_file(filename) + self.assertEqual(reframe.VERSION, module.VERSION) + self.assertEqual('reframe', module.__name__) + self.assertIs(module, sys.modules.get('reframe')) def test_load_twice(self): filename = os.path.abspath('reframe/__init__.py') From bc90a3e7a2cb7e9468dfc801970019c2d2efba56 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Fri, 1 Jun 2018 13:44:45 +0200 Subject: [PATCH 4/5] Address PR comments (version 2) --- unittests/test_utility.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 794c0e4766..471d78dbe2 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -282,7 +282,7 @@ def test_load_unknown_path(self): def test_load_dir_starts_with_ddot(self): with os_ext.change_dir('reframe'): - filename = '..%s/reframe' % os.getcwd() + filename = '../reframe' module = util.import_module_from_file(filename) self.assertEqual(reframe.VERSION, module.VERSION) self.assertEqual('reframe', module.__name__) @@ -290,7 +290,7 @@ def test_load_dir_starts_with_ddot(self): def test_load_file_starts_with_ddot(self): with os_ext.change_dir('reframe'): - filename = '..%s/reframe/__init__.py' % os.getcwd() + filename = '../reframe/__init__.py' module = util.import_module_from_file(filename) self.assertEqual(reframe.VERSION, module.VERSION) self.assertEqual('reframe', module.__name__) From 0f0baa03fede405bb118ca3e030095874a19997e Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 1 Jun 2018 14:12:52 +0200 Subject: [PATCH 5/5] Minor style improvements --- reframe/utility/__init__.py | 1 - unittests/test_utility.py | 10 ++++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 0566026ec5..d1f603ad63 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -41,7 +41,6 @@ def import_module_from_file(filename): """Import module from file.""" filename = os.path.normpath(os.path.expandvars(filename)) - if os.path.isdir(filename): filename = os.path.join(filename, '__init__.py') diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 471d78dbe2..4cb2576c57 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -280,18 +280,16 @@ def test_load_unknown_path(self): self.assertEqual('foo', e.name) self.assertEqual('/foo', e.path) - def test_load_dir_starts_with_ddot(self): + def test_load_directory_relative(self): with os_ext.change_dir('reframe'): - filename = '../reframe' - module = util.import_module_from_file(filename) + module = util.import_module_from_file('../reframe') self.assertEqual(reframe.VERSION, module.VERSION) self.assertEqual('reframe', module.__name__) self.assertIs(module, sys.modules.get('reframe')) - def test_load_file_starts_with_ddot(self): + def test_load_file_relative(self): with os_ext.change_dir('reframe'): - filename = '../reframe/__init__.py' - module = util.import_module_from_file(filename) + module = util.import_module_from_file('../reframe/__init__.py') self.assertEqual(reframe.VERSION, module.VERSION) self.assertEqual('reframe', module.__name__) self.assertIs(module, sys.modules.get('reframe'))