Merge pull request #2801 from vkarak/feat/userlib-imports

[feat] Simplify custom user imports in tests

docs/tutorial_tips_tricks.rst

@@ -392,6 +392,61 @@ This way the test will start passing again allowing us to catch any new issues w
 Then we can run the test anytime using ``--disable-hook=fooenv_workaround`` to check if the workaround is not needed anymore.
 
 
+Import user modules from a test file
+------------------------------------
+
+When building complex test suites or test libraries it is often the case that you would like to abstract away common functionality in a different Python module and import when needed.
+Suppose the following test directory structure:
+
+.. code-block:: console
+
+   tutorials/advanced/user_imports/
+   ├── commonutil
+   │   ├── __init__.py
+   └── tests
+       ├── test.py
+       └── testutil.py
+
+The ``commonutil`` module defines a :func:`greetings` function and the ``testutil`` module defines the :func:`greetings_from_test`.
+Suppose that the tests defined in ``test.py`` would like to use both of these modules.
+Prior to ReFrame 4.2, users would have to explicitly modify the :obj:`sys.path` in their test code before importing the desired modules as follows:
+
+.. code-block:: python
+
+   import os
+   import sys
+
+   prefix = os.path.dirname(__file__)
+   sys.path += [os.path.join(prefix, '..'), prefix]
+
+   import commonutil
+   from testutil import greetings_from_test
+
+This is a lot of boilerplate code for a relatively common operation.
+ReFrame 4.2 improves substantially this by introducing the following changes:
+
+1. The directory of the test file is temporarily added to the :obj:`sys.path` when ReFrame is loading the test file, so the ``testutil`` module can be directly imported without any additional preparation.
+2. While loading the test, ReFrame changes to the test's directory.
+3. ReFrame 4.2 provides two new utility functions for importing modules or symbols from modules: the :func:`~reframe.utility.import_module` and the :func:`~reframe.utility.import_from_module`
+
+The last two modifications allow users to load a module that is not in the same directory as the test file, like the ``commonutil`` module in this example.
+Let's rewrite the previous imports in ReFrame 4.2:
+
+.. code-block::
+
+   import reframe.utility as util
+   from testutil import greetings_from_test
+
+   commonutil = util.import_module('..commonutil')
+
+
+As soon as ``commonutil`` is imported with the utility function, it can be used as if it has been imported with an ``import`` statement.
+
+.. note::
+
+   Python will complain if you try to ``import ..commonutil`` as the test file is not part of a parent package.
+
+
 .. _generate-ci-pipeline:
 
 Integrating into a CI pipeline

reframe/frontend/loader.py

@@ -20,6 +20,27 @@
 from reframe.core.logging import getlogger, time_function
 
 
+class temp_sys_path:
+    def __init__(self, path):
+        self._path = path
+        self._pos = None
+
+    def __enter__(self):
+        self._pos = len(sys.path)
+        sys.path.append(self._path)
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        sys.path.pop(self._pos)
+
+
+class no_op:
+    def __enter__(self):
+        pass
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        pass
+
+
 class RegressionCheckValidator(ast.NodeVisitor):
     def __init__(self):
         self._has_import = False
@@ -178,13 +199,17 @@ def load_from_module(self, module):
         return final_tests
 
     def load_from_file(self, filename, force=False):
+        filename = os.path.abspath(filename)
         if not self._validate_source(filename):
             return []
 
         try:
-            return self.load_from_module(
-                util.import_module_from_file(filename, force)
-            )
+            dirname = os.path.dirname(filename)
+            with osext.change_dir(dirname):
+                with temp_sys_path(dirname):
+                    return self.load_from_module(
+                        util.import_module_from_file(filename, force)
+                    )
         except Exception:
             exc_info = sys.exc_info()
             if not is_severe(*exc_info):

reframe/utility/__init__.py

@@ -73,6 +73,16 @@ def _do_import_module_from_file(filename, module_name=None):
 def import_module_from_file(filename, force=False):
     '''Import module from file.
 
+    If the file location refers to a directory, the contained ``__init__.py``
+    will be loaded. If the filename resolves to a location that is within the
+    current working directory, a module name will be derived from the supplied
+    file name and Python's :func:`importlib.import_module` will be invoked to
+    actually load the module. If the file location refers to a path outside
+    the current working directory, then the module will be loaded directly
+    from the file, but it will be assigned a mangled name in
+    :obj:`sys.modules`, to avoid clashes with other modules loaded using the
+    standard import mechanism.
+
     :arg filename: The path to the filename of a Python module.
     :arg force: Force reload of module in case it is already loaded.
     :returns: The loaded Python module.
@@ -109,6 +119,54 @@ def import_module_from_file(filename, force=False):
     return importlib.import_module(module_name)
 
 
+def import_module(module_name, force=False):
+    '''Import a module.
+
+    This will not invoke directly the Python import mechanism. It will first
+    derive a path from the module name and will then call
+    :func:`import_module_from_file`.
+
+    :arg module_name: The name of the module to load.
+    :arg force: Force reload of module in case it is already loaded.
+    :returns: The loaded Python module.
+
+    .. versionadded:: 4.2
+    '''
+
+    # Calculate the number of levels that we need to go up
+    for num_dots, c in enumerate(module_name):
+        if c != '.':
+            break
+
+    if num_dots:
+        prefix = './'
+        for i in range(num_dots-1):
+            prefix += '../'
+    else:
+        prefix = ''
+
+    path = prefix + module_name.lstrip('.').replace('.', '/')
+    if os.path.isdir(path):
+        path += '/__init__.py'
+    else:
+        path += '.py'
+
+    return import_module_from_file(path, force)
+
+
+def import_from_module(module_name, symbol):
+    '''Import a symbol from module.
+
+    :arg module_name: The name of the module from which to import the symbol.
+    :arg symbol: The symbol to import.
+    :returns: The value of the requested symbol.
+
+    .. versionadded:: 4.2
+    '''
+
+    return getattr(import_module(module_name), symbol)
+
+
 def allx(iterable):
     '''Same as the built-in :py:func:`all`, except that it returns
     :class:`False` if ``iterable`` is empty.

tutorials/advanced/user_imports/commonutil/__init__.py

@@ -0,0 +1,2 @@
+def greetings(name):
+    return f'Hello, {name}'

tutorials/advanced/user_imports/tests/test.py

@@ -0,0 +1,28 @@
+import reframe as rfm
+import reframe.utility as util
+import reframe.utility.sanity as sn
+from testutil import greetings_from_test
+
+commonutil = util.import_module('..commonutil')
+
+
+@rfm.simple_test
+class MyTest(rfm.RunOnlyRegressionTest):
+    valid_systems = ['*']
+    valid_prog_environs = ['*']
+    executable = f'echo {commonutil.greetings("friends")}'
+
+    @sanity_function
+    def validate(self):
+        return sn.assert_found('Hello, friends', self.stdout)
+
+
+@rfm.simple_test
+class MyTest2(MyTest):
+    @run_before('run')
+    def set_exec(self):
+        self.executable = f'echo {greetings_from_test(self)}'
+
+    @sanity_function
+    def validate(self):
+        return sn.assert_found(f'Hello from {self.name}', self.stdout)

tutorials/advanced/user_imports/tests/testutil.py

@@ -0,0 +1,2 @@
+def greetings_from_test(test):
+    return f'Hello from {test.name}'

unittests/test_cli.py

@@ -1133,8 +1133,7 @@ def test_dynamic_tests_filtering(run_reframe, tmp_path, run_action):
     assert 'FAILED' not in stdout
 
 
-def test_testlib_inherit_fixture_in_different_files(run_reframe, monkeypatch):
-    monkeypatch.syspath_prepend('unittests/resources')
+def test_testlib_inherit_fixture_in_different_files(run_reframe):
     returncode, stdout, _ = run_reframe(
         checkpath=[
             'unittests/resources/checks_unlisted/testlib_inheritance_foo.py',

unittests/test_utility.py

@@ -535,6 +535,36 @@ def test_import_from_file_load_namespace_package():
     assert 'unittests.resources' in sys.modules
 
 
+def test_import_module(tmp_path):
+    with osext.change_dir(tmp_path):
+        os.mkdir('foo')
+        with open('foo/__init__.py', 'w') as fp:
+            fp.write('FOO = 1\n')
+
+        with open('foo/bar.py', 'w') as fp:
+            fp.write('BAR = 2\n')
+
+        with open('foobar.py', 'w') as fp:
+            fp.write('FOOBAR = 3\n')
+
+        foo = util.import_module('foo')
+        bar = util.import_module('foo.bar')
+        foobar = util.import_module('.foobar')
+        assert foo.FOO == 1
+        assert bar.BAR == 2
+        assert foobar.FOOBAR == 3
+
+        # Test relative imports
+        with osext.change_dir('foo'):
+            foobar = util.import_module('..foobar')
+            assert foobar.FOOBAR == 3
+
+        # Test import symbol
+        assert util.import_from_module('foo', 'FOO') == 1
+        assert util.import_from_module('foo.bar', 'BAR') == 2
+        assert util.import_from_module('foobar', 'FOOBAR') == 3
+
+
 def test_ppretty_simple_types():
     assert util.ppretty(1) == repr(1)
     assert util.ppretty(1.2) == repr(1.2)