From a3b96bc71c7c6b31f938e9cc473af82714649a17 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Thu, 5 Nov 2020 00:05:40 +0100
Subject: [PATCH 01/18] WIP: Support for TMod4 collections

---
 reframe/core/modules.py     | 132 +++++++++++++++++++++++++++++-------
 reframe/schemas/config.json |  14 +++-
 unittests/test_modules.py   |  10 +++
 3 files changed, 130 insertions(+), 26 deletions(-)

diff --git a/reframe/core/modules.py b/reframe/core/modules.py
index c8bc4bce6a..5904bac806 100644
--- a/reframe/core/modules.py
+++ b/reframe/core/modules.py
@@ -28,7 +28,7 @@ class Module:
     implementation should deal only with that.
     '''
 
-    def __init__(self, name):
+    def __init__(self, name, collection=False):
         if not isinstance(name, str):
             raise TypeError('module name not a string')
 
@@ -41,6 +41,9 @@ def __init__(self, name):
         except ValueError:
             self._name, self._version = name, None
 
+        # This module represents a "module collection" in TMod4
+        self._collection = collection
+
     @property
     def name(self):
         return self._name
@@ -49,6 +52,10 @@ def name(self):
     def version(self):
         return self._version
 
+    @property
+    def collection(self):
+        return self._collection
+
     @property
     def fullname(self):
         if self.version is not None:
@@ -66,6 +73,9 @@ def __eq__(self, other):
         if not isinstance(other, type(self)):
             return NotImplemented
 
+        if self.collection != other.collection:
+            return False
+
         if not self.version or not other.version:
             return self.name == other.name
         else:
@@ -169,14 +179,20 @@ def loaded_modules(self):
         '''
         return [str(m) for m in self._backend.loaded_modules()]
 
-    def conflicted_modules(self, name):
+    def conflicted_modules(self, name, collection=False):
         '''Return the list of the modules conflicting with module ``name``.
 
         If module ``name`` resolves to multiple real modules, then the returned
         list will be the concatenation of the conflict lists of all the real
         modules.
 
-        :rtype: List[str]
+        :arg name: The name of the module.
+        :arg collection: The module is a "module collection" (TMod4 only).
+        :returns: A list of conflicting module names.
+
+        .. versionchanged:: 3.3
+           The ``collection`` argument was added.
+
         '''
         ret = []
         for m in self.resolve_module(name):
@@ -184,18 +200,24 @@ def conflicted_modules(self, name):
 
         return ret
 
-    def _conflicted_modules(self, name):
-        return [str(m) for m in self._backend.conflicted_modules(Module(name))]
+    def _conflicted_modules(self, name, collection=False):
+        return [
+            str(m)
+            for m in self._backend.conflicted_modules(Module(name, collection))
+        ]
 
-    def load_module(self, name, force=False):
+    def load_module(self, name, force=False, collection=False):
         '''Load the module ``name``.
 
-        If ``force`` is set, forces the loading, unloading first any
-        conflicting modules currently loaded. If module ``name`` refers to
-        multiple real modules, all of the target modules will be loaded.
-
+        :arg force: If set, forces the loading, unloading first any
+            conflicting modules currently loaded. If module ``name`` refers to
+            multiple real modules, all of the target modules will be loaded.
+        :arg collection: The module is a "module collection" (TMod4 only)
         :returns: the list of unloaded modules as strings.
-        :rtype: List[str]
+
+        .. versionchanged:: 3.3
+           The ``collection`` argument was added.
+
         '''
         ret = []
         for m in self.resolve_module(name):
@@ -203,8 +225,8 @@ def load_module(self, name, force=False):
 
         return ret
 
-    def _load_module(self, name, force=False):
-        module = Module(name)
+    def _load_module(self, name, force=False, collection=False):
+        module = Module(name, collection)
         loaded_modules = self._backend.loaded_modules()
         if module in loaded_modules:
             # Do not try to load the module if it is already present
@@ -222,17 +244,23 @@ def _load_module(self, name, force=False):
         self._backend.load_module(module)
         return [str(m) for m in unload_list]
 
-    def unload_module(self, name):
+    def unload_module(self, name, collection=False):
         '''Unload module ``name``.
 
-        If module ``name`` refers to multiple real modules, all the referred to
-        modules will be unloaded in reverse order.
+        :arg name: The name of the module to unload. If module ``name`` is
+            resolved to multiple real modules, all the referred to modules
+            will be unloaded in reverse order.
+        :arg collection: The module is a "module collection" (TMod4 only)
+
+        .. versionchanged:: 3.3
+           The ``collection`` argument was added.
+
         '''
         for m in reversed(self.resolve_module(name)):
             self._unload_module(m)
 
-    def _unload_module(self, name):
-        self._backend.unload_module(Module(name))
+    def _unload_module(self, name, collection=False):
+        self._backend.unload_module(Module(name, collection))
 
     def is_module_loaded(self, name):
         '''Check if module ``name`` is loaded.
@@ -311,22 +339,38 @@ def searchpath_remove(self, *dirs):
         '''Remove ``dirs`` from the module system search path.'''
         return self._backend.searchpath_remove(*dirs)
 
-    def emit_load_commands(self, name):
+    def emit_load_commands(self, name, collection=False):
         '''Return the appropriate shell command for loading module ``name``.
 
-        :rtype: List[str]
+        :arg name: The name of the module to load.
+        :arg collection: The module is a "module collection" (TMod4 only)
+        :returns: A list of shell commands.
+
+        .. versionchanged:: 3.3
+           The ``collection`` argument was added.
+
         '''
-        return [self._backend.emit_load_instr(Module(name))
-                for name in self.resolve_module(name)]
+        ret = []
+        for name in self.resolve_module(name):
+            cmds = self._backend.emit_load_instr(Module(name, collection))
+            if cmds:
+                ret.append(cmds)
 
-    def emit_unload_commands(self, name):
+        return ret
+
+    def emit_unload_commands(self, name, collection=False):
         '''Return the appropriate shell command for unloading module
         ``name``.
 
         :rtype: List[str]
         '''
-        return [self._backend.emit_unload_instr(Module(name))
-                for name in reversed(self.resolve_module(name))]
+        ret = []
+        for name in self.resolve_module(name):
+            cmds = self._backend.emit_unload_instr(Module(name, collection))
+            if cmds:
+                ret.append(cmds)
+
+        return ret
 
     def __str__(self):
         return str(self._backend)
@@ -672,6 +716,44 @@ def _exec_module_command(self, *args, msg=None):
 
             raise EnvironError(msg)
 
+    def load_module(self, module):
+        if module.collection:
+            self._exec_module_command(
+                'restore', str(module),
+                msg="could not restore module collection '{module}'"
+            )
+            return []
+        else:
+            return super().load_module(module)
+
+    def unload_module(self, module):
+        if module.collection:
+            # Module collection are not unloaded
+            return
+
+        super().unload_module(module)
+
+    def conflicted_modules(self, module):
+        if module.collection:
+            # Conflicts have no meaning in module collection. The modules
+            # system will take care of these when restoring a module
+            # collection
+            return []
+
+        return super().conflicted_modules(module)
+
+    def emit_load_instr(self, module):
+        if module.collection:
+            return f'module restore {module}'
+
+        return super().emit_load_instr(module)
+
+    def emit_unload_instr(self, module):
+        if module.collection:
+            return ''
+
+        return super().emit_unload_instr(module)
+
 
 class LModImpl(TModImpl):
     '''Module system for Lmod (Tcl/Lua).'''
diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json
index 05ea54336e..6fea216271 100644
--- a/reframe/schemas/config.json
+++ b/reframe/schemas/config.json
@@ -27,7 +27,19 @@
         },
         "modules_list": {
             "type": "array",
-            "items": {"type": "string"}
+            "items": {
+                "anyOf": [
+                    {"type": "string"},
+                    {
+                        "type": "object",
+                        "properties": {
+                            "name": {"type": "string"},
+                            "collection": {"type": "boolean"}
+                        },
+                        "required": ["name"]
+                    }
+                ]
+            }
         },
         "loglevel": {
             "type": "string",
diff --git a/unittests/test_modules.py b/unittests/test_modules.py
index 6824b955a5..a95108427d 100644
--- a/unittests/test_modules.py
+++ b/unittests/test_modules.py
@@ -145,8 +145,11 @@ def _emit_load_commands_tmod(modules_system):
 def _emit_load_commands_tmod4(modules_system):
     emit_cmds = modules_system.emit_load_commands
     assert [emit_cmds('foo')] == ['module load foo']
+    assert [emit_cmds('foo', collection=True)] == ['module restore foo']
     assert [emit_cmds('foo/1.2')] == ['module load foo/1.2']
     assert [emit_cmds('m0')] == ['module load m1', 'module load m2']
+    assert [emit_cmds('m0', collection=True)] == ['module restore m1',
+                                                  'module restoreE m2']
 
 
 def _emit_load_commands_lmod(modules_system):
@@ -181,8 +184,10 @@ def _emit_unload_commands_tmod(modules_system):
 def _emit_unload_commands_tmod4(modules_system):
     emit_cmds = modules_system.emit_unload_commands
     assert [emit_cmds('foo')] == ['module unload foo']
+    assert [emit_cmds('foo', collection=True)] == []
     assert [emit_cmds('foo/1.2')] == ['module unload foo/1.2']
     assert [emit_cmds('m0')] == ['module unload m2', 'module unload m1']
+    assert [emit_cmds('m0', collection=True)] == []
 
 
 def _emit_unload_commands_lmod(modules_system):
@@ -203,6 +208,7 @@ def test_module_construction():
     m = modules.Module('foo/1.2')
     assert m.name == 'foo'
     assert m.version == '1.2'
+    assert m.collection == False
     with pytest.raises(ValueError):
         modules.Module('')
 
@@ -215,6 +221,9 @@ def test_module_construction():
     with pytest.raises(TypeError):
         modules.Module(23)
 
+    m = modules.Module('foo/1.2', collection=True)
+    assert m.collection == True
+
 
 def test_module_equal():
     assert modules.Module('foo') == modules.Module('foo')
@@ -227,6 +236,7 @@ def test_module_equal():
     assert modules.Module('foo/1.2') != modules.Module('foo/1.3')
     assert modules.Module('foo') != modules.Module('bar')
     assert modules.Module('foo') != modules.Module('foobar')
+    assert modules.Module('foo') != modules.Module('foo', collection=True)
 
 
 @pytest.fixture

From 8b6a4fcf0c440110398fe4a57f3b40cdda6f6019 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Thu, 5 Nov 2020 21:56:02 +0100
Subject: [PATCH 02/18] WIP: Support for module collections

---
 reframe/core/modules.py   | 92 ++++++++++++++++++++++++++++-----------
 unittests/test_modules.py | 55 +++++++++++++++++++++++
 2 files changed, 122 insertions(+), 25 deletions(-)

diff --git a/reframe/core/modules.py b/reframe/core/modules.py
index 5904bac806..384f305802 100644
--- a/reframe/core/modules.py
+++ b/reframe/core/modules.py
@@ -196,7 +196,7 @@ def conflicted_modules(self, name, collection=False):
         '''
         ret = []
         for m in self.resolve_module(name):
-            ret += self._conflicted_modules(m)
+            ret += self._conflicted_modules(m, collection)
 
         return ret
 
@@ -206,6 +206,15 @@ def _conflicted_modules(self, name, collection=False):
             for m in self._backend.conflicted_modules(Module(name, collection))
         ]
 
+    def execute(self, cmd, *args):
+        '''Execute an arbitrary module command.
+
+        :arg cmd: The command to execute, e.g., ``load``, ``restore`` etc.
+        :arg args: The arguments to pass to the command.
+        :returns: The command output.
+        '''
+        return self._backend.execute(cmd, *args)
+
     def load_module(self, name, force=False, collection=False):
         '''Load the module ``name``.
 
@@ -221,7 +230,7 @@ def load_module(self, name, force=False, collection=False):
         '''
         ret = []
         for m in self.resolve_module(name):
-            ret += self._load_module(m, force)
+            ret += self._load_module(m, force, collection)
 
         return ret
 
@@ -257,7 +266,7 @@ def unload_module(self, name, collection=False):
 
         '''
         for m in reversed(self.resolve_module(name)):
-            self._unload_module(m)
+            self._unload_module(m, collection)
 
     def _unload_module(self, name, collection=False):
         self._backend.unload_module(Module(name, collection))
@@ -377,7 +386,14 @@ def __str__(self):
 
 
 class ModulesSystemImpl(abc.ABC):
-    '''Abstract base class for module systems.'''
+    '''Abstract base class for module systems.
+
+    :meta private:
+    '''
+
+    @abc.abstractmethod
+    def execute(self, cmd, *args):
+        '''Execute an arbitrary command of the module system.'''
 
     @abc.abstractmethod
     def available_modules(self, substr):
@@ -527,9 +543,10 @@ def _run_module_command(self, *args, msg=None):
     def _module_command_failed(self, completed):
         return re.search(r'ERROR', completed.stderr) is not None
 
-    def _exec_module_command(self, *args, msg=None):
-        completed = self._run_module_command(*args, msg=msg)
+    def execute(self, cmd, *args, msg=None):
+        completed = self._run_module_command(cmd, *args, msg=msg)
         exec(completed.stdout)
+        return completed.stderr
 
     def available_modules(self, substr):
         completed = self._run_module_command(
@@ -556,7 +573,7 @@ def loaded_modules(self):
 
     def conflicted_modules(self, module):
         completed = self._run_module_command(
-            'show', str(module), msg="could not show module '%s'" % module)
+            'show', str(module), msg=f"could not show module '{module}'")
         return [Module(m.group(1))
                 for m in re.finditer(r'^conflict\s+(\S+)',
                                      completed.stderr, re.MULTILINE)]
@@ -565,33 +582,31 @@ def is_module_loaded(self, module):
         return module in self.loaded_modules()
 
     def load_module(self, module):
-        self._exec_module_command(
-            'load', str(module),
-            msg="could not load module '%s' correctly" % module)
+        self.execute('load', str(module),
+                     msg=f"could not load module '{module}' correctly")
 
     def unload_module(self, module):
-        self._exec_module_command(
-            'unload', str(module),
-            msg="could not unload module '%s' correctly" % module)
+        self.execute('unload', str(module),
+                     msg=f"could not unload module '{module}' correctly")
 
     def unload_all(self):
-        self._exec_module_command('purge')
+        self.execute('purge')
 
     def searchpath(self):
         path = os.getenv('MODULEPATH', '')
         return path.split(':')
 
     def searchpath_add(self, *dirs):
-        self._exec_module_command('use', *dirs)
+        self.execute('use', *dirs)
 
     def searchpath_remove(self, *dirs):
-        self._exec_module_command('unuse', *dirs)
+        self.execute('unuse', *dirs)
 
     def emit_load_instr(self, module):
-        return 'module load %s' % module
+        return f'module load {module}'
 
     def emit_unload_instr(self, module):
-        return 'module unload %s' % module
+        return f'module unload {module}'
 
 
 class TMod31Impl(TModImpl):
@@ -645,8 +660,8 @@ def __init__(self):
     def name(self):
         return 'tmod31'
 
-    def _exec_module_command(self, *args, msg=None):
-        completed = self._run_module_command(*args, msg=msg)
+    def execute(self, cmd, *args, msg=None):
+        completed = self._run_module_command(cmd, *args, msg=msg)
         exec_match = re.search(r'^exec\s\'', completed.stdout)
         if exec_match is None:
             raise ConfigError('could not use the python bindings')
@@ -660,6 +675,7 @@ def _exec_module_command(self, *args, msg=None):
                 cmd = content_file.read()
 
         exec(cmd)
+        return completed.stderr
 
 
 class TMod4Impl(TModImpl):
@@ -700,9 +716,9 @@ def __init__(self):
     def name(self):
         return 'tmod4'
 
-    def _exec_module_command(self, *args, msg=None):
-        command = ' '.join([self._command, *args])
-        completed = osext.run_command(command, check=True)
+    def execute(self, cmd, *args, msg=None):
+        command = ' '.join([self._command, cmd, *args])
+        completed = osext.run_command(command, check=False)
         namespace = {}
         exec(completed.stdout, {}, namespace)
         if not namespace['_mlstatus']:
@@ -716,6 +732,8 @@ def _exec_module_command(self, *args, msg=None):
 
             raise EnvironError(msg)
 
+        return completed.stderr
+
     def load_module(self, module):
         if module.collection:
             self._exec_module_command(
@@ -755,7 +773,7 @@ def emit_unload_instr(self, module):
         return super().emit_unload_instr(module)
 
 
-class LModImpl(TModImpl):
+class LModImpl(TMod4Impl):
     '''Module system for Lmod (Tcl/Lua).'''
 
     def __init__(self):
@@ -811,6 +829,12 @@ def available_modules(self, substr):
         return ret
 
     def conflicted_modules(self, module):
+        if module.collection:
+            # Conflicts have no meaning in module collection. The modules
+            # system will take care of these when restoring a module
+            # collection
+            return []
+
         completed = self._run_module_command(
             'show', str(module), msg="could not show module '%s'" % module)
 
@@ -830,10 +854,25 @@ def conflicted_modules(self, module):
 
         return ret
 
+    def load_module(self, module):
+        if module.collection:
+            self.execute('restore', str(module),
+                         msg="could not restore module collection '{module}'")
+            return []
+        else:
+            return super().load_module(module)
+
+    def unload_module(self, module):
+        if module.collection:
+            # Module collection are not unloaded
+            return
+
+        super().unload_module(module)
+
     def unload_all(self):
         # Currently, we don't take any provision for sticky modules in Lmod, so
         # we forcefully unload everything.
-        self._exec_module_command('--force', 'purge')
+        self.execute('--force', 'purge')
 
 
 class NoModImpl(ModulesSystemImpl):
@@ -848,6 +887,9 @@ def loaded_modules(self):
     def conflicted_modules(self, module):
         return []
 
+    def execute(self, cmd, *args):
+        return ''
+
     def load_module(self, module):
         pass
 
diff --git a/unittests/test_modules.py b/unittests/test_modules.py
index a95108427d..b70e082ef3 100644
--- a/unittests/test_modules.py
+++ b/unittests/test_modules.py
@@ -10,6 +10,7 @@
 import reframe.core.environments as env
 import reframe.core.modules as modules
 import reframe.utility as util
+import reframe.utility.osext as osext
 import unittests.fixtures as fixtures
 from reframe.core.exceptions import ConfigError, EnvironError
 from reframe.core.runtime import runtime
@@ -44,6 +45,39 @@ def test_searchpath(modules_system):
         assert fixtures.TEST_MODULES not in modules_system.searchpath
 
 
+@pytest.fixture
+def module_collection(modules_system, tmp_path):
+    import secrets
+
+    if modules_system.name not in ['tmod4', 'lmod']:
+        pytest.skip(f'test unsupported with {modules_system.name!r}')
+
+    # Generate a "random" name for the collection, because it is going to be
+    # placed under the $HOME/.module or $HOME/.lmod.d
+    coll_name = secrets.token_hex(8)
+    coll_name = 'lala'
+
+    # Create modules collections with conflicting modules
+    modules_system.load_module('testmod_base')
+    modules_system.load_module('testmod_foo')
+    modules_system.execute('save', coll_name)
+    # completed = osext.run_command(f'module save {coll_name}', shell=True)
+    # print(completed.stderr)
+    # print(completed.stdout)
+    modules_system.unload_module('testmod_foo')
+    modules_system.unload_module('testmod_base')
+
+    yield coll_name
+
+    # Remove the temporary collection
+    if modules_system.name == 'lmod':
+        prefix = os.path.join(os.environ['HOME'], '.lmod.d')
+    else:
+        prefix = os.path.join(os.environ['HOME'], '.module')
+
+    os.remove(os.path.join(prefix, coll_name))
+
+
 def test_module_load(modules_system):
     if modules_system.name == 'nomod':
         modules_system.load_module('foo')
@@ -66,6 +100,14 @@ def test_module_load(modules_system):
         assert 'TESTMOD_FOO' not in os.environ
 
 
+def test_module_load_collection(modules_system, module_collection):
+    # Load a conflicting module first
+    modules_system.load_module('testmod_bar')
+    modules_system.load_module(module_collection, collection=True)
+    assert modules_system.is_module_loaded('testmod_base')
+    assert modules_system.is_module_loaded('testmod_foo')
+
+
 def test_module_load_force(modules_system):
     if modules_system.name == 'nomod':
         modules_system.load_module('foo', force=True)
@@ -83,6 +125,16 @@ def test_module_load_force(modules_system):
         assert 'TESTMOD_BAR' in os.environ
 
 
+def test_module_load_force_collection(modules_system, module_collection):
+    # Load a conflicting module first
+    modules_system.load_module('testmod_bar')
+    unloaded = modules_system.load_module(module_collection,
+                                          force=True, collection=True)
+    assert unloaded == []
+    assert modules_system.is_module_loaded('testmod_base')
+    assert modules_system.is_module_loaded('testmod_foo')
+
+
 def test_module_unload_all(modules_system):
     if modules_system.name == 'nomod':
         modules_system.unload_all()
@@ -258,6 +310,9 @@ def loaded_modules(self):
         def conflicted_modules(self, module):
             return []
 
+        def execute(self, cmd, *args):
+            return ''
+
         def load_module(self, module):
             self.load_seq.append(module.name)
             self._loaded_modules.add(module.name)

From 4238b7eaa097291eb90fd9e4e0ad48e3158658ab Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 01:00:14 +0100
Subject: [PATCH 03/18] Better Dockerfile for Lmod

---
 Dockerfiles/Lmod.dockerfile | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Dockerfiles/Lmod.dockerfile

diff --git a/Dockerfiles/Lmod.dockerfile b/Dockerfiles/Lmod.dockerfile
new file mode 100644
index 0000000000..7b074f4e0d
--- /dev/null
+++ b/Dockerfiles/Lmod.dockerfile
@@ -0,0 +1,34 @@
+#
+# Execute this from the top-level ReFrame source directory
+#
+
+FROM ubuntu:20.04
+
+ENV TZ=Europe/Zurich
+ENV DEBIAN_FRONTEND=noninteractive
+ENV _LMOD_VER=8.4.12
+
+# Install Lmod
+WORKDIR /root
+RUN \
+  apt-get -y update && \
+  apt-get -y install python3 && \
+  apt-get -y install wget && \
+  apt-get -y install lua5.3 lua-bit32:amd64 lua-posix:amd64 lua-posix-dev liblua5.3-0:amd64 liblua5.3-dev:amd64 tcl tcl-dev tcl8.6 tcl8.6-dev:amd64 libtcl8.6:amd64 && \
+  wget -q https://github.com/TACC/Lmod/archive/${_LMOD_VER}.tar.gz -O lmod.tar.gz && \
+  tar xzf lmod.tar.gz && \
+  cd Lmod-${_LMOD_VER} && \
+  ./configure && make install && \
+  echo '. /usr/local/lmod/lmod/init/profile' >> /root/.bashrc
+
+# Erroneously required by ReFrame unit tests
+RUN apt-get -y install git
+
+# Install ReFrame from the current directory
+COPY . /root/reframe/
+RUN \
+  cd reframe \
+  ./bootstrap.sh
+
+WORKDIR /root/reframe
+CMD ["/bin/bash", "-c", "./test_reframe.py -v"]

From 36c48d219bf43201ad5bd2aff34189509ba558cb Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 13:50:42 +0100
Subject: [PATCH 04/18] Add Dockerfile for Tmod4 and align Lmod Dockerfile to
 it

---
 Dockerfiles/Lmod.dockerfile  | 23 +++++++++++++-------
 Dockerfiles/Tmod4.dockerfile | 41 ++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 8 deletions(-)
 create mode 100644 Dockerfiles/Tmod4.dockerfile

diff --git a/Dockerfiles/Lmod.dockerfile b/Dockerfiles/Lmod.dockerfile
index 7b074f4e0d..cd73b28899 100644
--- a/Dockerfiles/Lmod.dockerfile
+++ b/Dockerfiles/Lmod.dockerfile
@@ -7,22 +7,29 @@ FROM ubuntu:20.04
 ENV TZ=Europe/Zurich
 ENV DEBIAN_FRONTEND=noninteractive
 ENV _LMOD_VER=8.4.12
-
-# Install Lmod
 WORKDIR /root
+
+# ReFrame requirements
 RUN \
   apt-get -y update && \
-  apt-get -y install python3 && \
-  apt-get -y install wget && \
+  apt-get -y install gcc && \
+  apt-get -y install make && \
+  apt-get -y install git && \
+  apt-get -y install python3
+
+# Required utilities
+RUN apt-get -y install wget
+
+# Install Lmod
+RUN \
   apt-get -y install lua5.3 lua-bit32:amd64 lua-posix:amd64 lua-posix-dev liblua5.3-0:amd64 liblua5.3-dev:amd64 tcl tcl-dev tcl8.6 tcl8.6-dev:amd64 libtcl8.6:amd64 && \
   wget -q https://github.com/TACC/Lmod/archive/${_LMOD_VER}.tar.gz -O lmod.tar.gz && \
   tar xzf lmod.tar.gz && \
   cd Lmod-${_LMOD_VER} && \
-  ./configure && make install && \
-  echo '. /usr/local/lmod/lmod/init/profile' >> /root/.bashrc
+  ./configure && make install
+
 
-# Erroneously required by ReFrame unit tests
-RUN apt-get -y install git
+ENV BASH_ENV=/usr/local/lmod/lmod/init/profile
 
 # Install ReFrame from the current directory
 COPY . /root/reframe/
diff --git a/Dockerfiles/Tmod4.dockerfile b/Dockerfiles/Tmod4.dockerfile
new file mode 100644
index 0000000000..32553b5925
--- /dev/null
+++ b/Dockerfiles/Tmod4.dockerfile
@@ -0,0 +1,41 @@
+#
+# Execute this from the top-level ReFrame source directory
+#
+
+FROM ubuntu:20.04
+
+ENV TZ=Europe/Zurich
+ENV DEBIAN_FRONTEND=noninteractive
+ENV _TMOD_VER=4.6.0
+WORKDIR /root
+
+# ReFrame requirements
+RUN \
+  apt-get -y update && \
+  apt-get -y install gcc && \
+  apt-get -y install make && \
+  apt-get -y install git && \
+  apt-get -y install python3
+
+# Required utilities
+RUN apt-get -y install wget
+
+# Install Tmod4
+RUN \
+  apt-get -y install autoconf && \
+  apt-get -y install tcl-dev && \
+  wget -q https://github.com/cea-hpc/modules/archive/v${_TMOD_VER}.tar.gz -O tmod.tar.gz && \
+  tar xzf tmod.tar.gz && \
+  cd modules-${_TMOD_VER} && \
+  ./configure && make install
+
+ENV BASH_ENV=/usr/local/Modules/init/profile.sh
+
+# Install ReFrame from the current directory
+COPY . /root/reframe/
+RUN \
+  cd reframe \
+  ./bootstrap.sh
+
+WORKDIR /root/reframe
+CMD ["/bin/bash", "-c", "./test_reframe.py -v"]

From 2b8512021827fbf4a7180dbfd3105318336c6687 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 13:51:44 +0100
Subject: [PATCH 05/18] Move Dockerfiles under the ci-scripts/ dir

---
 {Dockerfiles => ci-scripts/dockerfiles}/Lmod.dockerfile  | 0
 {Dockerfiles => ci-scripts/dockerfiles}/Tmod4.dockerfile | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename {Dockerfiles => ci-scripts/dockerfiles}/Lmod.dockerfile (100%)
 rename {Dockerfiles => ci-scripts/dockerfiles}/Tmod4.dockerfile (100%)

diff --git a/Dockerfiles/Lmod.dockerfile b/ci-scripts/dockerfiles/Lmod.dockerfile
similarity index 100%
rename from Dockerfiles/Lmod.dockerfile
rename to ci-scripts/dockerfiles/Lmod.dockerfile
diff --git a/Dockerfiles/Tmod4.dockerfile b/ci-scripts/dockerfiles/Tmod4.dockerfile
similarity index 100%
rename from Dockerfiles/Tmod4.dockerfile
rename to ci-scripts/dockerfiles/Tmod4.dockerfile

From 2f12ff62d875758aada596a5161145599a158ff5 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 13:51:56 +0100
Subject: [PATCH 06/18] Fix Tmod4 implementation

---
 reframe/core/modules.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/reframe/core/modules.py b/reframe/core/modules.py
index 384f305802..c7de9a04b6 100644
--- a/reframe/core/modules.py
+++ b/reframe/core/modules.py
@@ -721,8 +721,10 @@ def execute(self, cmd, *args, msg=None):
         completed = osext.run_command(command, check=False)
         namespace = {}
         exec(completed.stdout, {}, namespace)
-        if not namespace['_mlstatus']:
-            # _mlstatus is set by the TMod4 Python bindings
+
+        # _mlstatus is set by the TMod4 only if the command was unsuccessful,
+        # but Lmod sets it always
+        if not namespace.get('_mlstatus', True):
             if msg is None:
                 msg = 'modules system command failed: '
                 if isinstance(completed.args, str):
@@ -736,10 +738,8 @@ def execute(self, cmd, *args, msg=None):
 
     def load_module(self, module):
         if module.collection:
-            self._exec_module_command(
-                'restore', str(module),
-                msg="could not restore module collection '{module}'"
-            )
+            self.execute('restore', str(module),
+                         msg="could not restore module collection '{module}'")
             return []
         else:
             return super().load_module(module)

From 655c6d51bf741b432dab20261713df1c84df973c Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 18:15:24 +0100
Subject: [PATCH 07/18] Introduce syntax for module collections in the
 configuration.

---
 reframe/core/config.py          | 26 ++++++++++++++++++++++++
 reframe/core/environments.py    | 35 ++++++++++++++++++++++++++++++++-
 reframe/core/runtime.py         |  6 +++---
 unittests/resources/settings.py |  6 ++++--
 unittests/test_config.py        |  9 ++++++---
 unittests/test_schedulers.py    | 13 ++++++------
 6 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/reframe/core/config.py b/reframe/core/config.py
index ae894abd8b..cef828b76d 100644
--- a/reframe/core/config.py
+++ b/reframe/core/config.py
@@ -5,6 +5,7 @@
 
 import copy
 import fnmatch
+import functools
 import itertools
 import json
 import jsonschema
@@ -19,6 +20,7 @@
 import reframe.utility as util
 import reframe.utility.osext as osext
 import reframe.utility.typecheck as types
+from reframe.core.environments import normalize_module_list
 from reframe.core.exceptions import ConfigError, ReframeFatalError
 from reframe.core.logging import getlogger
 from reframe.core.warnings import ReframeDeprecationWarning
@@ -39,6 +41,29 @@ def _match_option(opt, opt_map):
     raise KeyError(opt)
 
 
+def _normalize_syntax(conv):
+    '''Normalize syntax for options accepting multiple syntaxes'''
+
+    def _do_normalize(fn):
+
+        @functools.wraps(fn)
+        def _get(site_config, option, *args, **kwargs):
+            ret = fn(site_config, option, *args, **kwargs)
+            if option is None:
+                return ret
+
+            for opt_patt, norm_fn in conv.items():
+                if re.match(opt_patt, option):
+                    ret = norm_fn(ret)
+                    break
+
+            return ret
+
+        return _get
+
+    return _do_normalize
+
+
 class _SiteConfig:
     def __init__(self, site_config, filename):
         self._site_config = copy.deepcopy(site_config)
@@ -86,6 +111,7 @@ def add_sticky_option(self, option, value):
     def remove_sticky_option(self, option):
         self._sticky_options.pop(option, None)
 
+    @_normalize_syntax({'.*/modules$': normalize_module_list})
     def get(self, option, default=None):
         '''Retrieve value of option.
 
diff --git a/reframe/core/environments.py b/reframe/core/environments.py
index b0475e150d..1a54c8cfcb 100644
--- a/reframe/core/environments.py
+++ b/reframe/core/environments.py
@@ -11,6 +11,21 @@
 import reframe.utility.typecheck as typ
 
 
+def normalize_module_list(modules):
+    '''Normalize module list.
+
+    :meta private:
+    '''
+    ret = []
+    for m in modules:
+        if isinstance(m, str):
+            ret.append({'name': m, 'collection': False})
+        else:
+            ret.append(m)
+
+    return ret
+
+
 class Environment:
     '''This class abstracts away an environment to run regression tests.
 
@@ -25,7 +40,8 @@ def __init__(self, name, modules=None, variables=None):
         modules = modules or []
         variables = variables or []
         self._name = name
-        self._modules = list(modules)
+        self._modules = normalize_module_list(modules)
+        self._module_names = [m['name'] for m in self._modules]
         self._variables = collections.OrderedDict(variables)
 
     @property
@@ -42,6 +58,23 @@ def modules(self):
 
         :type: :class:`List[str]`
         '''
+        return util.SequenceView(self._module_names)
+
+    @property
+    def modules_detailed(self):
+        '''A view of the modules associated with this environment in a detailed
+        format.
+
+        Each module is represented as a dictionary with the following
+        attributes:
+
+        - ``name``: the name of the module.
+        - ``collection``: :class:`True` if the module name refers to a module
+          collection.
+
+        :type: :class:`List[Dict[str, object]]`
+        '''
+
         return util.SequenceView(self._modules)
 
     @property
diff --git a/reframe/core/runtime.py b/reframe/core/runtime.py
index 9e7386213b..c719f82609 100644
--- a/reframe/core/runtime.py
+++ b/reframe/core/runtime.py
@@ -212,12 +212,12 @@ def loadenv(*environs):
     env_snapshot = snapshot()
     commands = []
     for env in environs:
-        for m in env.modules:
-            conflicted = modules_system.load_module(m, force=True)
+        for m in env.modules_detailed:
+            conflicted = modules_system.load_module(**m, force=True)
             for c in conflicted:
                 commands += modules_system.emit_unload_commands(c)
 
-            commands += modules_system.emit_load_commands(m)
+            commands += modules_system.emit_load_commands(**m)
 
         for k, v in env.variables.items():
             os.environ[k] = osext.expandvars(v)
diff --git a/unittests/resources/settings.py b/unittests/resources/settings.py
index f1cd42714e..b38eafa3a2 100644
--- a/unittests/resources/settings.py
+++ b/unittests/resources/settings.py
@@ -44,7 +44,9 @@
                     'descr': 'GPU partition',
                     'scheduler': 'slurm',
                     'launcher': 'srun',
-                    'modules': ['foogpu'],
+
+                    # Use the extensive syntax here
+                    'modules': [{'name': 'foogpu', 'collection': False}],
                     'variables': [['FOO_GPU', 'yes']],
                     'resources': [
                         {
@@ -88,7 +90,7 @@
     'environments': [
         {
             'name': 'PrgEnv-gnu',
-            'modules': ['PrgEnv-gnu'],
+            'modules': [{'name': 'PrgEnv-gnu', 'collection': False}],
         },
         {
             'name': 'PrgEnv-gnu',
diff --git a/unittests/test_config.py b/unittests/test_config.py
index bee766b910..6b0fe9f223 100644
--- a/unittests/test_config.py
+++ b/unittests/test_config.py
@@ -225,7 +225,8 @@ def test_select_subconfig():
     assert site_config.get('systems/0/prefix') == '.rfm_testing'
     assert (site_config.get('systems/0/resourcesdir') ==
             '.rfm_testing/resources')
-    assert site_config.get('systems/0/modules') == ['foo/1.0']
+    assert site_config.get('systems/0/modules') == [{'name': 'foo/1.0',
+                                                     'collection': False}]
     assert site_config.get('systems/0/variables') == [['FOO_CMD', 'foobar']]
     assert site_config.get('systems/0/modules_system') == 'nomod'
     assert site_config.get('systems/0/outputdir') == ''
@@ -260,7 +261,7 @@ def test_select_subconfig():
     assert site_config.get('environments/@PrgEnv-cray/cc') == 'cc'
     assert site_config.get('environments/1/cxx') == 'CC'
     assert (site_config.get('environments/@PrgEnv-cray/modules') ==
-            ['PrgEnv-cray'])
+            [{'name': 'PrgEnv-cray', 'collection': False}])
     assert len(site_config.get('general')) == 1
     assert site_config.get('general/0/check_search_path') == ['a:b']
 
@@ -273,7 +274,9 @@ def test_select_subconfig():
     assert len(site_config.get('systems/0/partitions/0/resources')) == 2
     assert (site_config.get('systems/0/partitions/0/resources/@gpu/name') ==
             'gpu')
-    assert site_config.get('systems/0/partitions/0/modules') == ['foogpu']
+    assert site_config.get('systems/0/partitions/0/modules') == [
+        {'name': 'foogpu', 'collection': False}
+    ]
     assert (site_config.get('systems/0/partitions/0/variables') ==
             [['FOO_GPU', 'yes']])
     assert site_config.get('systems/0/partitions/0/max_jobs') == 10
diff --git a/unittests/test_schedulers.py b/unittests/test_schedulers.py
index 7d465f6482..c5d7b5742c 100644
--- a/unittests/test_schedulers.py
+++ b/unittests/test_schedulers.py
@@ -518,18 +518,17 @@ def test_cancel_with_grace(minimal_job, scheduler, local_only):
     # signal handler for SIGTERM
     time.sleep(1)
 
-    t_grace = datetime.now()
+    t_grace = time.time()
     minimal_job.cancel()
     time.sleep(0.1)
     minimal_job.wait()
-    t_grace = datetime.now() - t_grace
+    t_grace = time.time() - t_grace
 
     # Read pid of spawned sleep
     with open(minimal_job.stdout) as fp:
         sleep_pid = int(fp.read())
 
-    assert t_grace.total_seconds() >= 2
-    assert t_grace.total_seconds() < 5
+    assert t_grace >= 2 and t_grace < 5
     assert minimal_job.state == 'FAILURE'
     assert minimal_job.signal == signal.SIGKILL
 
@@ -563,17 +562,17 @@ def test_cancel_term_ignore(minimal_job, scheduler, local_only):
     # signal handler for SIGTERM
     time.sleep(1)
 
-    t_grace = datetime.now()
+    t_grace = time.time()
     minimal_job.cancel()
     time.sleep(0.1)
     minimal_job.wait()
-    t_grace = datetime.now() - t_grace
+    t_grace = time.time() - t_grace
 
     # Read pid of spawned sleep
     with open(minimal_job.stdout) as fp:
         sleep_pid = int(fp.read())
 
-    assert t_grace.total_seconds() >= 2
+    assert t_grace >= 2 and t_grace < 5
     assert minimal_job.state == 'FAILURE'
     assert minimal_job.signal == signal.SIGKILL
 

From cb53e446feceb2a79c32e8dc6d3cfa7954dcb273 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 18:41:15 +0100
Subject: [PATCH 08/18] Remove datetime calls from unit tests

---
 reframe/frontend/executors/policies.py |  2 --
 unittests/test_utility.py              | 13 ++++++-------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/reframe/frontend/executors/policies.py b/reframe/frontend/executors/policies.py
index 3ce01c5018..107c34505d 100644
--- a/reframe/frontend/executors/policies.py
+++ b/reframe/frontend/executors/policies.py
@@ -10,8 +10,6 @@
 import sys
 import time
 
-from datetime import datetime
-
 from reframe.core.exceptions import (TaskDependencyError, TaskExit)
 from reframe.core.logging import (getlogger, VERBOSE)
 from reframe.frontend.executors import (ExecutionPolicy, RegressionTask,
diff --git a/unittests/test_utility.py b/unittests/test_utility.py
index e55729f7c5..7503dc7b0d 100644
--- a/unittests/test_utility.py
+++ b/unittests/test_utility.py
@@ -8,6 +8,7 @@
 import random
 import shutil
 import sys
+import time
 
 import reframe
 import reframe.core.fields as fields
@@ -49,19 +50,17 @@ def test_command_timeout():
 
 
 def test_command_async():
-    from datetime import datetime
-
-    t_launch = datetime.now()
+    t_launch = time.time()
     t_sleep  = t_launch
     proc = osext.run_command_async('sleep 1')
-    t_launch = datetime.now() - t_launch
+    t_launch = time.time() - t_launch
 
     proc.wait()
-    t_sleep = datetime.now() - t_sleep
+    t_sleep = time.time() - t_sleep
 
     # Now check the timings
-    assert t_launch.seconds < 1
-    assert t_sleep.seconds >= 1
+    assert t_launch < 1
+    assert t_sleep >= 1
 
 
 def test_copytree(tmp_path):

From 54db76aad57c195703076dd5739b729e0b4946ee Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 18:59:05 +0100
Subject: [PATCH 09/18] Document support for modules collections

---
 docs/config_reference.rst    | 41 ++++++++++++++++++++++++++++++------
 reframe/core/environments.py |  3 +++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/docs/config_reference.rst b/docs/config_reference.rst
index ca8c667d4a..f0c6e5e053 100644
--- a/docs/config_reference.rst
+++ b/docs/config_reference.rst
@@ -122,7 +122,7 @@ System Configuration
    :required: No
    :default: ``[]``
 
-   Environment modules to be loaded always when running on this system.
+   A list of `environment module objects <#module-objects>`__ to be loaded always when running on this system.
    These modules modify the ReFrame environment.
    This is useful in cases where a particular module is needed, for example, to submit jobs on a specific system.
 
@@ -280,7 +280,7 @@ System Partition Configuration
    :required: No
    :default: ``[]``
 
-   A list of environment modules to be loaded before running a regression test on this partition.
+  A list of `environment module objects <#module-objects>`__ to be loaded before running a regression test on this partition.
 
 
 .. js:attribute:: .systems[].partitions[].variables
@@ -339,7 +339,7 @@ ReFrame can launch containerized applications, but you need to configure properl
    :required: No
    :default: ``[]``
 
-  List of environment modules to be loaded when running containerized tests using this container platform.
+   A list of `environment module objects <#module-objects>`__ to be loaded when running containerized tests using this container platform.
 
 
 .. js:attribute:: .systems[].partitions[].container_platforms[].variables
@@ -458,7 +458,7 @@ They are associated with `system partitions <#system-partition-configuration>`__
    :required: No
    :default: ``[]``
 
-   A list of environment modules to be loaded when this environment is loaded.
+   A list of `environment module objects <#module-objects>`__ to be loaded when this environment is loaded.
 
 
 .. js:attribute:: .environments[].variables
@@ -1176,7 +1176,7 @@ General Configuration
    :required: No
    :default: ``[]``
 
-   A list of environment modules to unload before executing any test.
+   A list of `environment module objects <#module-objects>`__ to unload before executing any test.
    If specified using an the environment variable, a space separated list of modules is expected.
    If specified from the command line, multiple modules can be passed by passing the command line option multiple times.
 
@@ -1196,7 +1196,7 @@ General Configuration
    :required: No
    :default: ``[]``
 
-   A list of environment modules to be loaded before executing any test.
+   A list of `environment module objects <#module-objects>`__ to be loaded before executing any test.
    If specified using an the environment variable, a space separated list of modules is expected.
    If specified from the command line, multiple modules can be passed by passing the command line option multiple times.
 
@@ -1209,3 +1209,32 @@ General Configuration
    Increase the verbosity level of the output.
    The higher the number, the more verbose the output will be.
    If specified from the command line, the command line option must be specified multiple times to increase the verbosity level more than once.
+
+
+Module Objects
+--------------
+
+.. versionadded:: 3.3
+
+
+A *module object* in ReFrame's configuration represents an environment module.
+It can either be a simple string or a JSON object with the following attributes:
+
+.. js:attribute:: .name
+
+   :required: Yes
+
+   The name of the module.
+
+
+.. js:attribute:: .collection
+
+   :required: No
+   :default: ``false``
+
+   A boolean value indicating whether this module refers to a module collection.
+   Module collections are treated differently from simple modules when loading.
+
+.. seealso::
+
+   - Module collections with `Environment Modules <https://modules.readthedocs.io/en/latest/MIGRATING.html#module-collection>`__ and `Lmod <https://lmod.readthedocs.io/en/latest/010_user.html#user-collections>`__.
diff --git a/reframe/core/environments.py b/reframe/core/environments.py
index 1a54c8cfcb..c11170a19d 100644
--- a/reframe/core/environments.py
+++ b/reframe/core/environments.py
@@ -73,6 +73,9 @@ def modules_detailed(self):
           collection.
 
         :type: :class:`List[Dict[str, object]]`
+
+        .. versionadded:: 3.3
+
         '''
 
         return util.SequenceView(self._modules)

From ad87285168d3ad75209f58a29e07b7278e70e424 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 23:40:19 +0100
Subject: [PATCH 10/18] Fix '-m' and '-u' options

---
 ci-scripts/dockerfiles/Lmod.dockerfile  |  2 +-
 ci-scripts/dockerfiles/Tmod4.dockerfile |  2 +-
 reframe/core/config.py                  |  2 +-
 reframe/frontend/cli.py                 | 12 +++++++-----
 unittests/test_cli.py                   | 13 +++++++++++++
 5 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/ci-scripts/dockerfiles/Lmod.dockerfile b/ci-scripts/dockerfiles/Lmod.dockerfile
index cd73b28899..f0f38e7bb5 100644
--- a/ci-scripts/dockerfiles/Lmod.dockerfile
+++ b/ci-scripts/dockerfiles/Lmod.dockerfile
@@ -38,4 +38,4 @@ RUN \
   ./bootstrap.sh
 
 WORKDIR /root/reframe
-CMD ["/bin/bash", "-c", "./test_reframe.py -v"]
+CMD ["/bin/bash", "-c", "./test_reframe.py --rfm-user-config=ci-scripts/configs/lmod.py -v"]
diff --git a/ci-scripts/dockerfiles/Tmod4.dockerfile b/ci-scripts/dockerfiles/Tmod4.dockerfile
index 32553b5925..6667d276ed 100644
--- a/ci-scripts/dockerfiles/Tmod4.dockerfile
+++ b/ci-scripts/dockerfiles/Tmod4.dockerfile
@@ -38,4 +38,4 @@ RUN \
   ./bootstrap.sh
 
 WORKDIR /root/reframe
-CMD ["/bin/bash", "-c", "./test_reframe.py -v"]
+CMD ["/bin/bash", "-c", "./test_reframe.py --rfm-user-config=ci-scripts/configs/tmod4.py -v"]
diff --git a/reframe/core/config.py b/reframe/core/config.py
index cef828b76d..185b7f996e 100644
--- a/reframe/core/config.py
+++ b/reframe/core/config.py
@@ -111,7 +111,7 @@ def add_sticky_option(self, option, value):
     def remove_sticky_option(self, option):
         self._sticky_options.pop(option, None)
 
-    @_normalize_syntax({'.*/modules$': normalize_module_list})
+    @_normalize_syntax({'.*/.*modules$': normalize_module_list})
     def get(self, option, default=None):
         '''Retrieve value of option.
 
diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py
index 734b57662d..5c7f2fd0f5 100644
--- a/reframe/frontend/cli.py
+++ b/reframe/frontend/cli.py
@@ -687,7 +687,7 @@ def print_infoline(param, value):
             rt.modules_system.unload_all()
         else:
             for m in site_config.get('general/0/unload_modules'):
-                rt.modules_system.unload_module(m)
+                rt.modules_system.unload_module(**m)
 
         # Load the environment for the current system
         try:
@@ -699,13 +699,15 @@ def print_infoline(param, value):
             printer.debug(str(e))
             raise
 
-        printer.debug(f'Loading user modules from command line')
+        printer.debug('Loading user modules from command line')
         for m in site_config.get('general/0/user_modules'):
             try:
-                rt.modules_system.load_module(m, force=True)
+                rt.modules_system.load_module(**m, force=True)
             except errors.EnvironError as e:
-                printer.warning("could not load module '%s' correctly: "
-                                "Skipping..." % m)
+                printer.warning(
+                    f'could not load module {m["name"]!r} correctly; '
+                    f'skipping...'
+                )
                 printer.debug(str(e))
 
         options.flex_alloc_nodes = options.flex_alloc_nodes or 'idle'
diff --git a/unittests/test_cli.py b/unittests/test_cli.py
index ddf1cb31e3..d6ead4d4a6 100644
--- a/unittests/test_cli.py
+++ b/unittests/test_cli.py
@@ -541,6 +541,19 @@ def test_verbosity_with_check(run_reframe):
     assert 0 == returncode
 
 
+def test_load_user_modules(run_reframe, user_exec_ctx):
+    with rt.module_use('unittests/modules'):
+        returncode, stdout, stderr = run_reframe(
+            more_options=['-m testmod_foo'],
+            action='list'
+        )
+
+    assert stdout != ''
+    assert 'Traceback' not in stdout
+    assert 'Traceback' not in stderr
+    assert returncode == 0
+
+
 def test_unload_module(run_reframe, user_exec_ctx):
     # This test is mostly for ensuring coverage. `run_reframe()` restores
     # the current environment, so it is not easy to verify that the modules

From eaa9713ca41f9d9b9cb820ee04b2242d568335aa Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 6 Nov 2020 23:48:51 +0100
Subject: [PATCH 11/18] Update documentation for module mappings

---
 docs/config_reference.rst | 2 +-
 docs/manpage.rst          | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/docs/config_reference.rst b/docs/config_reference.rst
index f0c6e5e053..b63b93e089 100644
--- a/docs/config_reference.rst
+++ b/docs/config_reference.rst
@@ -1237,4 +1237,4 @@ It can either be a simple string or a JSON object with the following attributes:
 
 .. seealso::
 
-   - Module collections with `Environment Modules <https://modules.readthedocs.io/en/latest/MIGRATING.html#module-collection>`__ and `Lmod <https://lmod.readthedocs.io/en/latest/010_user.html#user-collections>`__.
+   Module collections with `Environment Modules <https://modules.readthedocs.io/en/latest/MIGRATING.html#module-collection>`__ and `Lmod <https://lmod.readthedocs.io/en/latest/010_user.html#user-collections>`__.
diff --git a/docs/manpage.rst b/docs/manpage.rst
index 00be732823..f9e9607466 100644
--- a/docs/manpage.rst
+++ b/docs/manpage.rst
@@ -427,6 +427,12 @@ It does so by leveraging the selected system's environment modules system.
 
    This option can also be set using the :envvar:`RFM_MODULE_MAPPINGS` environment variable or the :js:attr:`module_mappings` general configuration parameter.
 
+   .. versionchanged:: 3.3
+      If the mapping replaces a module collection, all new names must refer to module collections, too.
+
+   .. seealso::
+      Module collections with `Environment Modules <https://modules.readthedocs.io/en/latest/MIGRATING.html#module-collection>`__ and `Lmod <https://lmod.readthedocs.io/en/latest/010_user.html#user-collections>`__.
+
 
 .. option:: --module-mappings=FILE
 

From 57a8ac137d6e3ea0c107e70fd4e698219717c212 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Sun, 8 Nov 2020 18:37:44 +0100
Subject: [PATCH 12/18] Deal with sequence arguments properly in
 `run_command()` functions

---
 reframe/utility/osext.py  | 16 +++++++++++-----
 unittests/test_utility.py | 12 ++++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/reframe/utility/osext.py b/reframe/utility/osext.py
index bdac7bdade..1d3443a673 100644
--- a/reframe/utility/osext.py
+++ b/reframe/utility/osext.py
@@ -34,7 +34,8 @@ def run_command(cmd, check=False, timeout=None, shell=False, log=True):
     reached. It essentially calls :func:`run_command_async` and waits for the
     command's completion.
 
-    :arg cmd: The command to execute as a string.
+    :arg cmd: The command to execute as a string or a sequence. See
+        :func:`run_command_async` for more details.
     :arg check: Raise an error if the command exits with a non-zero exit code.
     :arg timeout: Timeout in seconds.
     :arg shell: Spawn a new shell to execute the command.
@@ -46,6 +47,7 @@ def run_command(cmd, check=False, timeout=None, shell=False, log=True):
         is :class:`True` and the command fails.
     :raises reframe.core.exceptions.SpawnedProcessTimeout: If the command
         times out.
+
     '''
 
     try:
@@ -58,7 +60,7 @@ def run_command(cmd, check=False, timeout=None, shell=False, log=True):
                                     proc.stdout.read(),
                                     proc.stderr.read(), timeout) from None
 
-    completed = subprocess.CompletedProcess(args=shlex.split(cmd),
+    completed = subprocess.CompletedProcess(cmd,
                                             returncode=proc.returncode,
                                             stdout=proc_stdout,
                                             stderr=proc_stderr)
@@ -79,9 +81,13 @@ def run_command_async(cmd,
                       **popen_args):
     '''Run command asynchronously.
 
-    This creates a :py:class:`subprocess.Popen` with
-    ``universal_newlines=True`` and returns.
+    A wrapper to :py:class:`subprocess.Popen` with the following tweaks:
+
+    - It always passes ``universal_newlines=True`` to :py:class:`Popen`.
+    - If ``shell=False`` and ``cmd`` is a string, it will lexically split
+      ``cmd`` using ``shlex.split(cmd)``.
 
+    :arg cmd: The command to run either as a string or a sequence of arguments.
     :arg stdout: Same as the corresponding argument of :py:class:`Popen`.
         Default is :py:obj:`subprocess.PIPE`.
     :arg stderr: Same as the corresponding argument of :py:class:`Popen`.
@@ -99,7 +105,7 @@ def run_command_async(cmd,
         from reframe.core.logging import getlogger
         getlogger().debug2(f'[CMD] {cmd!r}')
 
-    if not shell:
+    if isinstance(cmd, str) and not shell:
         cmd = shlex.split(cmd)
 
     return subprocess.Popen(args=cmd,
diff --git a/unittests/test_utility.py b/unittests/test_utility.py
index 7503dc7b0d..727cc28362 100644
--- a/unittests/test_utility.py
+++ b/unittests/test_utility.py
@@ -30,12 +30,24 @@ def test_command_success():
     assert completed.stdout == 'foobar\n'
 
 
+def test_command_success_cmd_seq():
+    completed = osext.run_command(['echo', 'foobar'])
+    assert completed.returncode == 0
+    assert completed.stdout == 'foobar\n'
+
+
 def test_command_error():
     with pytest.raises(SpawnedProcessError,
                        match=r"command 'false' failed with exit code 1"):
         osext.run_command('false', check=True)
 
 
+def test_command_error_cmd_seq():
+    with pytest.raises(SpawnedProcessError,
+                       match=r"command 'false' failed with exit code 1"):
+        osext.run_command(['false'], check=True)
+
+
 def test_command_timeout():
     with pytest.raises(
         SpawnedProcessTimeout, match=r"command 'sleep 3' timed out "

From ecc9aeeded0901ce62d0d2f45300f4838a3eeded Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Sun, 8 Nov 2020 19:25:16 +0100
Subject: [PATCH 13/18] Homogenize implementation of the modules backends

---
 reframe/core/modules.py   | 95 +++++++++++++++++++++------------------
 unittests/test_modules.py |  4 ++
 2 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/reframe/core/modules.py b/reframe/core/modules.py
index c7de9a04b6..a1be8cb98c 100644
--- a/reframe/core/modules.py
+++ b/reframe/core/modules.py
@@ -441,6 +441,11 @@ def name(self):
     def version(self):
         '''Return the version of this module system.'''
 
+    @property
+    @abc.abstractmethod
+    def modulecmd(self):
+        '''The low level command to use for issuing module commads'''
+
     @abc.abstractmethod
     def unload_all(self):
         '''Unload all loaded modules.'''
@@ -506,10 +511,9 @@ def __init__(self):
                 (version, self.MIN_VERSION))
 
         self._version = version
-        self._command = 'modulecmd python'
         try:
             # Try the Python bindings now
-            completed = osext.run_command(self._command)
+            completed = osext.run_command(self.modulecmd)
         except OSError as e:
             raise ConfigError(
                 'could not get the Python bindings for TMod: ' % e) from e
@@ -524,36 +528,28 @@ def name(self):
     def version(self):
         return self._version
 
-    def _run_module_command(self, *args, msg=None):
-        command = ' '.join([self._command, *args])
-        try:
-            completed = osext.run_command(command, check=True)
-        except SpawnedProcessError as e:
-            raise EnvironError(msg) from e
+    @property
+    def modulecmd(self):
+        return 'modulecmd python'
 
-        if self._module_command_failed(completed):
-            err = SpawnedProcessError(command,
+    def execute(self, cmd, *args):
+        command = [self.modulecmd, cmd, *args]
+        completed = os_ext.run_command(command)
+        if re.search(r'ERROR', completed.stderr) is not None:
+            raise SpawnedProcessError(command,
                                       completed.stdout,
                                       completed.stderr,
                                       completed.returncode)
-            raise EnvironError(msg) from err
-
-        return completed
 
-    def _module_command_failed(self, completed):
-        return re.search(r'ERROR', completed.stderr) is not None
-
-    def execute(self, cmd, *args, msg=None):
-        completed = self._run_module_command(cmd, *args, msg=msg)
         exec(completed.stdout)
         return completed.stderr
 
     def available_modules(self, substr):
-        completed = self._run_module_command(
+        output = self.execute(
             'avail', '-t', substr, msg='could not retrieve available modules'
         )
         ret = []
-        for line in completed.stderr.split('\n'):
+        for line in output.split('\n'):
             if not line or line[-1] == ':':
                 # Ignore empty lines and path entries
                 continue
@@ -572,11 +568,12 @@ def loaded_modules(self):
             return []
 
     def conflicted_modules(self, module):
-        completed = self._run_module_command(
-            'show', str(module), msg=f"could not show module '{module}'")
+        output = self.execute(
+            'show', str(module), msg=f"could not show module '{module}'"
+        )
         return [Module(m.group(1))
                 for m in re.finditer(r'^conflict\s+(\S+)',
-                                     completed.stderr, re.MULTILINE)]
+                                     output, re.MULTILINE)]
 
     def is_module_loaded(self, module):
         return module in self.loaded_modules()
@@ -660,19 +657,26 @@ def __init__(self):
     def name(self):
         return 'tmod31'
 
+    @property
+    def modulecmd(self):
+        return self._command
+
     def execute(self, cmd, *args, msg=None):
-        completed = self._run_module_command(cmd, *args, msg=msg)
-        exec_match = re.search(r'^exec\s\'', completed.stdout)
+        command = [self.modulecmd, cmd, *args]
+        completed = os_ext.run_command(command)
+        if re.search(r'ERROR', completed.stderr) is not None:
+            raise SpawnedProcessError(command,
+                                      completed.stdout,
+                                      completed.stderr,
+                                      completed.returncode)
+
+        exec_match = re.search(r"^exec\s'(\S+)'", completed.stdout,
+                               re.MULTILINE)
         if exec_match is None:
             raise ConfigError('could not use the python bindings')
-        else:
-            cmd = completed.stdout
-            exec_match = re.search(r'^exec\s\'(\S+)\'', cmd,
-                                   re.MULTILINE)
-            if exec_match is None:
-                raise ConfigError('could not use the python bindings')
-            with open(exec_match.group(1), 'r') as content_file:
-                cmd = content_file.read()
+
+        with open(exec_match.group(1), 'r') as content_file:
+            cmd = content_file.read()
 
         exec(cmd)
         return completed.stderr
@@ -684,9 +688,8 @@ class TMod4Impl(TModImpl):
     MIN_VERSION = (4, 1)
 
     def __init__(self):
-        self._command = 'modulecmd python'
         try:
-            completed = osext.run_command(self._command + ' -V', check=True)
+            completed = osext.run_command(self.modulecmd + ' -V', check=True)
         except OSError as e:
             raise ConfigError(
                 'could not find a sane TMod4 installation') from e
@@ -716,6 +719,10 @@ def __init__(self):
     def name(self):
         return 'tmod4'
 
+    @property
+    def modulecmd(self):
+        return 'modulecmd python'
+
     def execute(self, cmd, *args, msg=None):
         command = ' '.join([self._command, cmd, *args])
         completed = osext.run_command(command, check=False)
@@ -810,15 +817,12 @@ def __init__(self):
     def name(self):
         return 'lmod'
 
-    def _module_command_failed(self, completed):
-        return completed.stdout.strip() == 'false'
-
     def available_modules(self, substr):
-        completed = self._run_module_command(
+        output = self.execute(
             '-t', 'avail', substr, msg='could not retrieve available modules'
         )
         ret = []
-        for line in completed.stderr.split('\n'):
+        for line in output.split('\n'):
             if not line or line[-1] == ':':
                 # Ignore empty lines and path entries
                 continue
@@ -835,15 +839,16 @@ def conflicted_modules(self, module):
             # collection
             return []
 
-        completed = self._run_module_command(
-            'show', str(module), msg="could not show module '%s'" % module)
+        output = self.execute(
+            'show', str(module), msg=f"could not show module '{module}'"
+        )
 
         # Lmod accepts both Lua and and Tcl syntax
         # The following test allows incorrect syntax, e.g., `conflict
         # ('package"(`, but we expect this to be caught by the Lmod framework
         # in earlier stages.
         ret = []
-        for m in re.finditer(r'conflict\s*(\S+)', completed.stderr):
+        for m in re.finditer(r'conflict\s*(\S+)', output):
             conflict_arg = m.group(1)
             if conflict_arg.startswith('('):
                 # Lua syntax
@@ -909,6 +914,10 @@ def name(self):
     def version(self):
         return '1.0'
 
+    @property
+    def modulecmd(self):
+        return ''
+
     def unload_all(self):
         pass
 
diff --git a/unittests/test_modules.py b/unittests/test_modules.py
index b70e082ef3..aeab3d57be 100644
--- a/unittests/test_modules.py
+++ b/unittests/test_modules.py
@@ -336,6 +336,10 @@ def name(self):
         def version(self):
             return '1.0'
 
+        @property
+        def modulecmd(self):
+            return ''
+
         def unload_all(self):
             self._loaded_modules.clear()
 

From f6919f43b9285f4ba0160ac9db3d93cca546942a Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Sun, 8 Nov 2020 20:30:50 +0100
Subject: [PATCH 14/18] Add Dockerfile for Tmod 3.2

---
 ci-scripts/dockerfiles/Tmod32.dockerfile | 29 ++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 ci-scripts/dockerfiles/Tmod32.dockerfile

diff --git a/ci-scripts/dockerfiles/Tmod32.dockerfile b/ci-scripts/dockerfiles/Tmod32.dockerfile
new file mode 100644
index 0000000000..b2e512eeb7
--- /dev/null
+++ b/ci-scripts/dockerfiles/Tmod32.dockerfile
@@ -0,0 +1,29 @@
+#
+# Execute this from the top-level ReFrame source directory
+#
+
+FROM centos:7
+
+WORKDIR /root
+
+# ReFrame requirements
+RUN \
+  yum -y install gcc && \
+  yum -y install make && \
+  yum -y install git && \
+  yum -y install python3
+
+# # Required utilities
+# RUN apt-get -y install wget
+
+# Install Tmod 3.2
+RUN yum -y install environment-modules
+
+# Install ReFrame from the current directory
+COPY . /root/reframe/
+RUN \
+  cd reframe \
+  ./bootstrap.sh
+
+WORKDIR /root/reframe
+CMD ["/bin/bash", "-c", "./test_reframe.py --rfm-user-config=ci-scripts/configs/tmod32.py -v"]

From 23b14392b7151dbe112f3d7ad83fbb16502f0c56 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Sun, 8 Nov 2020 20:31:31 +0100
Subject: [PATCH 15/18] Add config files for environment modules tests

---
 ci-scripts/configs/lmod.py   | 70 ++++++++++++++++++++++++++++++++++++
 ci-scripts/configs/tmod32.py | 70 ++++++++++++++++++++++++++++++++++++
 ci-scripts/configs/tmod4.py  | 70 ++++++++++++++++++++++++++++++++++++
 3 files changed, 210 insertions(+)
 create mode 100644 ci-scripts/configs/lmod.py
 create mode 100644 ci-scripts/configs/tmod32.py
 create mode 100644 ci-scripts/configs/tmod4.py

diff --git a/ci-scripts/configs/lmod.py b/ci-scripts/configs/lmod.py
new file mode 100644
index 0000000000..73460323f2
--- /dev/null
+++ b/ci-scripts/configs/lmod.py
@@ -0,0 +1,70 @@
+# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich)
+# ReFrame Project Developers. See the top-level LICENSE file for details.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+
+#
+# Generic fallback configuration
+#
+
+site_configuration = {
+    'systems': [
+        {
+            'name': 'generic',
+            'descr': 'Generic example system',
+            'hostnames': ['.*'],
+            'modules_system': 'lmod',
+            'partitions': [
+                {
+                    'name': 'default',
+                    'scheduler': 'local',
+                    'launcher': 'local',
+                    'environs': ['builtin']
+                }
+            ]
+        },
+    ],
+    'environments': [
+        {
+            'name': 'builtin',
+            'cc': 'cc',
+            'cxx': '',
+            'ftn': ''
+        },
+    ],
+    'logging': [
+        {
+            'handlers': [
+                {
+                    'type': 'stream',
+                    'name': 'stdout',
+                    'level': 'info',
+                    'format': '%(message)s'
+                },
+                {
+                    'type': 'file',
+                    'level': 'debug',
+                    'format': '[%(asctime)s] %(levelname)s: %(check_info)s: %(message)s',   # noqa: E501
+                    'append': False
+                }
+            ],
+            'handlers_perflog': [
+                {
+                    'type': 'filelog',
+                    'prefix': '%(check_system)s/%(check_partition)s',
+                    'level': 'info',
+                    'format': (
+                        '%(check_job_completion_time)s|reframe %(version)s|'
+                        '%(check_info)s|jobid=%(check_jobid)s|'
+                        '%(check_perf_var)s=%(check_perf_value)s|'
+                        'ref=%(check_perf_ref)s '
+                        '(l=%(check_perf_lower_thres)s, '
+                        'u=%(check_perf_upper_thres)s)|'
+                        '%(check_perf_unit)s'
+                    ),
+                    'append': True
+                }
+            ]
+        }
+    ],
+}
diff --git a/ci-scripts/configs/tmod32.py b/ci-scripts/configs/tmod32.py
new file mode 100644
index 0000000000..8c6724d77f
--- /dev/null
+++ b/ci-scripts/configs/tmod32.py
@@ -0,0 +1,70 @@
+# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich)
+# ReFrame Project Developers. See the top-level LICENSE file for details.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+
+#
+# Generic fallback configuration
+#
+
+site_configuration = {
+    'systems': [
+        {
+            'name': 'generic',
+            'descr': 'Generic example system',
+            'hostnames': ['.*'],
+            'modules_system': 'tmod32',
+            'partitions': [
+                {
+                    'name': 'default',
+                    'scheduler': 'local',
+                    'launcher': 'local',
+                    'environs': ['builtin']
+                }
+            ]
+        },
+    ],
+    'environments': [
+        {
+            'name': 'builtin',
+            'cc': 'cc',
+            'cxx': '',
+            'ftn': ''
+        },
+    ],
+    'logging': [
+        {
+            'handlers': [
+                {
+                    'type': 'stream',
+                    'name': 'stdout',
+                    'level': 'info',
+                    'format': '%(message)s'
+                },
+                {
+                    'type': 'file',
+                    'level': 'debug',
+                    'format': '[%(asctime)s] %(levelname)s: %(check_info)s: %(message)s',   # noqa: E501
+                    'append': False
+                }
+            ],
+            'handlers_perflog': [
+                {
+                    'type': 'filelog',
+                    'prefix': '%(check_system)s/%(check_partition)s',
+                    'level': 'info',
+                    'format': (
+                        '%(check_job_completion_time)s|reframe %(version)s|'
+                        '%(check_info)s|jobid=%(check_jobid)s|'
+                        '%(check_perf_var)s=%(check_perf_value)s|'
+                        'ref=%(check_perf_ref)s '
+                        '(l=%(check_perf_lower_thres)s, '
+                        'u=%(check_perf_upper_thres)s)|'
+                        '%(check_perf_unit)s'
+                    ),
+                    'append': True
+                }
+            ]
+        }
+    ],
+}
diff --git a/ci-scripts/configs/tmod4.py b/ci-scripts/configs/tmod4.py
new file mode 100644
index 0000000000..28ebcea245
--- /dev/null
+++ b/ci-scripts/configs/tmod4.py
@@ -0,0 +1,70 @@
+# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich)
+# ReFrame Project Developers. See the top-level LICENSE file for details.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+
+#
+# Generic fallback configuration
+#
+
+site_configuration = {
+    'systems': [
+        {
+            'name': 'generic',
+            'descr': 'Generic example system',
+            'hostnames': ['.*'],
+            'modules_system': 'tmod4',
+            'partitions': [
+                {
+                    'name': 'default',
+                    'scheduler': 'local',
+                    'launcher': 'local',
+                    'environs': ['builtin']
+                }
+            ]
+        },
+    ],
+    'environments': [
+        {
+            'name': 'builtin',
+            'cc': 'cc',
+            'cxx': '',
+            'ftn': ''
+        },
+    ],
+    'logging': [
+        {
+            'handlers': [
+                {
+                    'type': 'stream',
+                    'name': 'stdout',
+                    'level': 'info',
+                    'format': '%(message)s'
+                },
+                {
+                    'type': 'file',
+                    'level': 'debug',
+                    'format': '[%(asctime)s] %(levelname)s: %(check_info)s: %(message)s',   # noqa: E501
+                    'append': False
+                }
+            ],
+            'handlers_perflog': [
+                {
+                    'type': 'filelog',
+                    'prefix': '%(check_system)s/%(check_partition)s',
+                    'level': 'info',
+                    'format': (
+                        '%(check_job_completion_time)s|reframe %(version)s|'
+                        '%(check_info)s|jobid=%(check_jobid)s|'
+                        '%(check_perf_var)s=%(check_perf_value)s|'
+                        'ref=%(check_perf_ref)s '
+                        '(l=%(check_perf_lower_thres)s, '
+                        'u=%(check_perf_upper_thres)s)|'
+                        '%(check_perf_unit)s'
+                    ),
+                    'append': True
+                }
+            ]
+        }
+    ],
+}

From 5ad71c7eef2fa5a7c5a6ffebf1d957b17290152d Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Sun, 8 Nov 2020 23:13:05 +0100
Subject: [PATCH 16/18] Homogenize implementations of modules backends

---
 reframe/core/modules.py        | 97 ++++++++++++++--------------------
 unittests/test_environments.py |  4 +-
 unittests/test_modules.py      |  6 ++-
 3 files changed, 45 insertions(+), 62 deletions(-)

diff --git a/reframe/core/modules.py b/reframe/core/modules.py
index a1be8cb98c..bdbcfba882 100644
--- a/reframe/core/modules.py
+++ b/reframe/core/modules.py
@@ -441,9 +441,8 @@ def name(self):
     def version(self):
         '''Return the version of this module system.'''
 
-    @property
     @abc.abstractmethod
-    def modulecmd(self):
+    def modulecmd(self, *args):
         '''The low level command to use for issuing module commads'''
 
     @abc.abstractmethod
@@ -513,7 +512,7 @@ def __init__(self):
         self._version = version
         try:
             # Try the Python bindings now
-            completed = osext.run_command(self.modulecmd)
+            completed = osext.run_command(self.modulecmd())
         except OSError as e:
             raise ConfigError(
                 'could not get the Python bindings for TMod: ' % e) from e
@@ -528,15 +527,14 @@ def name(self):
     def version(self):
         return self._version
 
-    @property
-    def modulecmd(self):
-        return 'modulecmd python'
+    def modulecmd(self, *args):
+        return ' '.join(['modulecmd', 'python', *args])
 
     def execute(self, cmd, *args):
-        command = [self.modulecmd, cmd, *args]
-        completed = os_ext.run_command(command)
+        modulecmd = self.modulecmd(cmd, *args)
+        completed = osext.run_command(modulecmd)
         if re.search(r'ERROR', completed.stderr) is not None:
-            raise SpawnedProcessError(command,
+            raise SpawnedProcessError(modulecmd,
                                       completed.stdout,
                                       completed.stderr,
                                       completed.returncode)
@@ -545,9 +543,7 @@ def execute(self, cmd, *args):
         return completed.stderr
 
     def available_modules(self, substr):
-        output = self.execute(
-            'avail', '-t', substr, msg='could not retrieve available modules'
-        )
+        output = self.execute('avail', '-t', substr)
         ret = []
         for line in output.split('\n'):
             if not line or line[-1] == ':':
@@ -568,9 +564,7 @@ def loaded_modules(self):
             return []
 
     def conflicted_modules(self, module):
-        output = self.execute(
-            'show', str(module), msg=f"could not show module '{module}'"
-        )
+        output = self.execute('show', str(module))
         return [Module(m.group(1))
                 for m in re.finditer(r'^conflict\s+(\S+)',
                                      output, re.MULTILINE)]
@@ -579,12 +573,10 @@ def is_module_loaded(self, module):
         return module in self.loaded_modules()
 
     def load_module(self, module):
-        self.execute('load', str(module),
-                     msg=f"could not load module '{module}' correctly")
+        self.execute('load', str(module))
 
     def unload_module(self, module):
-        self.execute('unload', str(module),
-                     msg=f"could not unload module '{module}' correctly")
+        self.execute('unload', str(module))
 
     def unload_all(self):
         self.execute('purge')
@@ -657,15 +649,14 @@ def __init__(self):
     def name(self):
         return 'tmod31'
 
-    @property
-    def modulecmd(self):
-        return self._command
+    def modulecmd(self, *args):
+        return ' '.join([self._command, *args])
 
-    def execute(self, cmd, *args, msg=None):
-        command = [self.modulecmd, cmd, *args]
-        completed = os_ext.run_command(command)
+    def execute(self, cmd, *args):
+        modulecmd = self.modulecmd(cmd, *args)
+        completed = osext.run_command(modulecmd)
         if re.search(r'ERROR', completed.stderr) is not None:
-            raise SpawnedProcessError(command,
+            raise SpawnedProcessError(modulecmd,
                                       completed.stdout,
                                       completed.stderr,
                                       completed.returncode)
@@ -689,7 +680,7 @@ class TMod4Impl(TModImpl):
 
     def __init__(self):
         try:
-            completed = osext.run_command(self.modulecmd + ' -V', check=True)
+            completed = osext.run_command(self.modulecmd('-V'), check=True)
         except OSError as e:
             raise ConfigError(
                 'could not find a sane TMod4 installation') from e
@@ -719,34 +710,28 @@ def __init__(self):
     def name(self):
         return 'tmod4'
 
-    @property
-    def modulecmd(self):
-        return 'modulecmd python'
+    def modulecmd(self, *args):
+        return ' '.join(['modulecmd', 'python', *args])
 
-    def execute(self, cmd, *args, msg=None):
-        command = ' '.join([self._command, cmd, *args])
-        completed = osext.run_command(command, check=False)
+    def execute(self, cmd, *args):
+        modulecmd = self.modulecmd(cmd, *args)
+        completed = osext.run_command(modulecmd, check=False)
         namespace = {}
         exec(completed.stdout, {}, namespace)
 
         # _mlstatus is set by the TMod4 only if the command was unsuccessful,
         # but Lmod sets it always
         if not namespace.get('_mlstatus', True):
-            if msg is None:
-                msg = 'modules system command failed: '
-                if isinstance(completed.args, str):
-                    msg += completed.args
-                else:
-                    msg += ' '.join(completed.args)
-
-            raise EnvironError(msg)
+            raise SpawnedProcessError(modulecmd,
+                                      completed.stdout,
+                                      completed.stderr,
+                                      completed.returncode)
 
         return completed.stderr
 
     def load_module(self, module):
         if module.collection:
-            self.execute('restore', str(module),
-                         msg="could not restore module collection '{module}'")
+            self.execute('restore', str(module))
             return []
         else:
             return super().load_module(module)
@@ -785,13 +770,13 @@ class LModImpl(TMod4Impl):
 
     def __init__(self):
         # Try to figure out if we are indeed using LMOD
-        lmod_cmd = os.getenv('LMOD_CMD')
-        if lmod_cmd is None:
+        self._lmod_cmd = os.getenv('LMOD_CMD')
+        if self._lmod_cmd is None:
             raise ConfigError('could not find a sane Lmod installation: '
                               'environment variable LMOD_CMD is not defined')
 
         try:
-            completed = osext.run_command('%s --version' % lmod_cmd)
+            completed = osext.run_command(f'{self._lmod_cmd} --version')
         except OSError as e:
             raise ConfigError(
                 'could not find a sane Lmod installation: %s' % e)
@@ -802,10 +787,9 @@ def __init__(self):
             raise ConfigError('could not retrieve Lmod version')
 
         self._version = version_match.group(1)
-        self._command = '%s python ' % lmod_cmd
         try:
             # Try the Python bindings now
-            completed = osext.run_command(self._command)
+            completed = osext.run_command(self.modulecmd())
         except OSError as e:
             raise ConfigError(
                 'could not get the Python bindings for Lmod: ' % e)
@@ -817,10 +801,11 @@ def __init__(self):
     def name(self):
         return 'lmod'
 
+    def modulecmd(self, *args):
+        return ' '.join([self._lmod_cmd, 'python', *args])
+
     def available_modules(self, substr):
-        output = self.execute(
-            '-t', 'avail', substr, msg='could not retrieve available modules'
-        )
+        output = self.execute('-t', 'avail', substr)
         ret = []
         for line in output.split('\n'):
             if not line or line[-1] == ':':
@@ -839,9 +824,7 @@ def conflicted_modules(self, module):
             # collection
             return []
 
-        output = self.execute(
-            'show', str(module), msg=f"could not show module '{module}'"
-        )
+        output = self.execute('show', str(module))
 
         # Lmod accepts both Lua and and Tcl syntax
         # The following test allows incorrect syntax, e.g., `conflict
@@ -861,8 +844,7 @@ def conflicted_modules(self, module):
 
     def load_module(self, module):
         if module.collection:
-            self.execute('restore', str(module),
-                         msg="could not restore module collection '{module}'")
+            self.execute('restore', str(module))
             return []
         else:
             return super().load_module(module)
@@ -914,8 +896,7 @@ def name(self):
     def version(self):
         return '1.0'
 
-    @property
-    def modulecmd(self):
+    def modulecmd(self, *args):
         return ''
 
     def unload_all(self):
diff --git a/unittests/test_environments.py b/unittests/test_environments.py
index 1e9fa86a6e..60df3f7f6f 100644
--- a/unittests/test_environments.py
+++ b/unittests/test_environments.py
@@ -10,7 +10,7 @@
 import reframe.core.environments as env
 import reframe.core.runtime as rt
 import unittests.fixtures as fixtures
-from reframe.core.exceptions import EnvironError
+from reframe.core.exceptions import (EnvironError, SpawnedProcessError)
 
 
 @pytest.fixture
@@ -258,7 +258,7 @@ def test_emit_loadenv_failure(user_runtime):
 
     # Suppress the module load error and verify that the original environment
     # is preserved
-    with contextlib.suppress(EnvironError):
+    with contextlib.suppress(SpawnedProcessError):
         rt.emit_loadenv_commands(environ)
 
     assert rt.snapshot() == snap
diff --git a/unittests/test_modules.py b/unittests/test_modules.py
index aeab3d57be..2ce4a741f3 100644
--- a/unittests/test_modules.py
+++ b/unittests/test_modules.py
@@ -12,7 +12,9 @@
 import reframe.utility as util
 import reframe.utility.osext as osext
 import unittests.fixtures as fixtures
-from reframe.core.exceptions import ConfigError, EnvironError
+from reframe.core.exceptions import (ConfigError,
+                                     EnvironError,
+                                     SpawnedProcessError)
 from reframe.core.runtime import runtime
 
 
@@ -83,7 +85,7 @@ def test_module_load(modules_system):
         modules_system.load_module('foo')
         modules_system.unload_module('foo')
     else:
-        with pytest.raises(EnvironError):
+        with pytest.raises(SpawnedProcessError):
             modules_system.load_module('foo')
 
         assert not modules_system.is_module_loaded('foo')

From 4bacd126f5a238c271e6b00856fb8467e3c0ddc9 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Mon, 9 Nov 2020 13:52:23 +0100
Subject: [PATCH 17/18] Fix PEP8 issues

---
 unittests/test_modules.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/unittests/test_modules.py b/unittests/test_modules.py
index 2ce4a741f3..b2d5f0dd16 100644
--- a/unittests/test_modules.py
+++ b/unittests/test_modules.py
@@ -262,7 +262,7 @@ def test_module_construction():
     m = modules.Module('foo/1.2')
     assert m.name == 'foo'
     assert m.version == '1.2'
-    assert m.collection == False
+    assert m.collection is False
     with pytest.raises(ValueError):
         modules.Module('')
 
@@ -276,7 +276,7 @@ def test_module_construction():
         modules.Module(23)
 
     m = modules.Module('foo/1.2', collection=True)
-    assert m.collection == True
+    assert m.collection is True
 
 
 def test_module_equal():

From a8f57b04d0eac3b0a1aa156c79b9acf15327f063 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Mon, 9 Nov 2020 17:54:26 +0100
Subject: [PATCH 18/18] Address PR comments

---
 ci-scripts/dockerfiles/Lmod.dockerfile   |  6 ++----
 ci-scripts/dockerfiles/Tmod32.dockerfile |  4 +---
 ci-scripts/dockerfiles/Tmod4.dockerfile  |  6 ++----
 unittests/test_modules.py                | 15 +++++----------
 4 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/ci-scripts/dockerfiles/Lmod.dockerfile b/ci-scripts/dockerfiles/Lmod.dockerfile
index f0f38e7bb5..adf1a60c19 100644
--- a/ci-scripts/dockerfiles/Lmod.dockerfile
+++ b/ci-scripts/dockerfiles/Lmod.dockerfile
@@ -15,7 +15,7 @@ RUN \
   apt-get -y install gcc && \
   apt-get -y install make && \
   apt-get -y install git && \
-  apt-get -y install python3
+  apt-get -y install python3 python3-pip
 
 # Required utilities
 RUN apt-get -y install wget
@@ -33,9 +33,7 @@ ENV BASH_ENV=/usr/local/lmod/lmod/init/profile
 
 # Install ReFrame from the current directory
 COPY . /root/reframe/
-RUN \
-  cd reframe \
-  ./bootstrap.sh
+RUN cd reframe && ./bootstrap.sh
 
 WORKDIR /root/reframe
 CMD ["/bin/bash", "-c", "./test_reframe.py --rfm-user-config=ci-scripts/configs/lmod.py -v"]
diff --git a/ci-scripts/dockerfiles/Tmod32.dockerfile b/ci-scripts/dockerfiles/Tmod32.dockerfile
index b2e512eeb7..aa1d1093d3 100644
--- a/ci-scripts/dockerfiles/Tmod32.dockerfile
+++ b/ci-scripts/dockerfiles/Tmod32.dockerfile
@@ -21,9 +21,7 @@ RUN yum -y install environment-modules
 
 # Install ReFrame from the current directory
 COPY . /root/reframe/
-RUN \
-  cd reframe \
-  ./bootstrap.sh
+RUN cd reframe && ./bootstrap.sh
 
 WORKDIR /root/reframe
 CMD ["/bin/bash", "-c", "./test_reframe.py --rfm-user-config=ci-scripts/configs/tmod32.py -v"]
diff --git a/ci-scripts/dockerfiles/Tmod4.dockerfile b/ci-scripts/dockerfiles/Tmod4.dockerfile
index 6667d276ed..034be1830c 100644
--- a/ci-scripts/dockerfiles/Tmod4.dockerfile
+++ b/ci-scripts/dockerfiles/Tmod4.dockerfile
@@ -15,7 +15,7 @@ RUN \
   apt-get -y install gcc && \
   apt-get -y install make && \
   apt-get -y install git && \
-  apt-get -y install python3
+  apt-get -y install python3 python3-pip
 
 # Required utilities
 RUN apt-get -y install wget
@@ -33,9 +33,7 @@ ENV BASH_ENV=/usr/local/Modules/init/profile.sh
 
 # Install ReFrame from the current directory
 COPY . /root/reframe/
-RUN \
-  cd reframe \
-  ./bootstrap.sh
+RUN cd reframe && ./bootstrap.sh
 
 WORKDIR /root/reframe
 CMD ["/bin/bash", "-c", "./test_reframe.py --rfm-user-config=ci-scripts/configs/tmod4.py -v"]
diff --git a/unittests/test_modules.py b/unittests/test_modules.py
index b2d5f0dd16..dc6e5c02d1 100644
--- a/unittests/test_modules.py
+++ b/unittests/test_modules.py
@@ -48,24 +48,19 @@ def test_searchpath(modules_system):
 
 
 @pytest.fixture
-def module_collection(modules_system, tmp_path):
-    import secrets
-
+def module_collection(modules_system, tmp_path, monkeypatch):
     if modules_system.name not in ['tmod4', 'lmod']:
         pytest.skip(f'test unsupported with {modules_system.name!r}')
 
-    # Generate a "random" name for the collection, because it is going to be
-    # placed under the $HOME/.module or $HOME/.lmod.d
-    coll_name = secrets.token_hex(8)
-    coll_name = 'lala'
+    # Both Lmod and Tmod4 place collections under the user's HOME directory,
+    # that's why we monkeypatch it
+    monkeypatch.setenv('HOME', str(tmp_path))
+    coll_name = 'test_collection'
 
     # Create modules collections with conflicting modules
     modules_system.load_module('testmod_base')
     modules_system.load_module('testmod_foo')
     modules_system.execute('save', coll_name)
-    # completed = osext.run_command(f'module save {coll_name}', shell=True)
-    # print(completed.stderr)
-    # print(completed.stdout)
     modules_system.unload_module('testmod_foo')
     modules_system.unload_module('testmod_base')