From 981da4dc8b48f6ec7febcdeabd9d30a7425aaf54 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 17 Nov 2020 17:41:51 +0100 Subject: [PATCH 1/9] Support (un)using a module path before execution * Add the corresponding documentation. * Add unittests. --- docs/manpage.rst | 50 +++++++++++++++++++++++++++++++++++++ reframe/frontend/cli.py | 35 ++++++++++++++++++++++++++ reframe/schemas/config.json | 8 ++++++ unittests/test_cli.py | 37 +++++++++++++++++++++++++++ 4 files changed, 130 insertions(+) diff --git a/docs/manpage.rst b/docs/manpage.rst index f9e9607466..9664e43ad0 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -387,6 +387,26 @@ It does so by leveraging the selected system's environment modules system. This option can also be set using the :envvar:`RFM_UNLOAD_MODULES` environment variable or the :js:attr:`unload_modules` general configuration parameter. +.. option:: --unuse-module-path=PATH + + Unuse module path ``PATH`` before acting on any tests. + This option may be specified multiple times, in which case all specified modules paths will be unused in order. + + This option can also be set using the :envvar:`RFM_UNUSE_MODULE_PATHS` environment variable or the :js:attr:`unuse_module_paths` general configuration parameter. + + .. versionadded:: 3.3 + + +.. option:: --use-module-path=PATH + + Use module path ``PATH`` before acting on any tests. + This option may be specified multiple times, in which case all specified module paths will be used in order. + + This option can also be set using the :envvar:`RFM_USE_MODULE_PATHS` environment variable or the :js:attr:`use_module_paths` general configuration parameter. + + .. versionadded:: 3.3 + + .. option:: --purge-env Unload all environment modules before acting on any tests. @@ -883,6 +903,36 @@ Here is an alphabetical list of the environment variables recognized by ReFrame: ================================== ================== +.. versionadded:: 3.3 + +.. envvar:: RFM_UNUSE_MODULE_PATHS + + A colon-separated list of module paths to be unused before acting on any tests. + + .. table:: + :align: left + + ================================== ================== + Associated command line option :option:`--unuse-module-path` + Associated configuration parameter :js:attr:`unuse_module_paths` general configuration parameter + ================================== ================== + + +.. versionadded:: 3.3 + +.. envvar:: RFM_USE_MODULE_PATHS + + A colon-separated list of module paths to be used before acting on any tests. + + .. table:: + :align: left + + ================================== ================== + Associated command line option :option:`--use-module-path` + Associated configuration parameter :js:attr:`use_module_paths` general configuration parameter + ================================== ================== + + .. envvar:: RFM_USE_LOGIN_SHELL Use a login shell for the generated job scripts. diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index c77689d544..38097c07d5 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -359,6 +359,19 @@ def main(): help='Unload module MOD before running any regression check', envvar='RFM_UNLOAD_MODULES ,', configvar='general/unload_modules' ) + env_options.add_argument( + '--unuse-module-path', action='append', metavar='PATH', + dest='unuse_module_paths', default=[], + help='Unuse module path PATH before running any regression check', + envvar='RFM_UNUSE_MODULE_PATHS :', + configvar='general/unuse_module_paths' + ) + env_options.add_argument( + '--use-module-path', action='append', metavar='PATH', + dest='use_module_paths', default=[], + help='Use module path PATH before running any regression check', + envvar='RFM_USE_MODULE_PATHS :', configvar='general/use_module_paths' + ) env_options.add_argument( '--purge-env', action='store_true', dest='purge_env', default=False, help='Unload all modules before running any regression check', @@ -733,6 +746,28 @@ def print_infoline(param, value): printer.debug(str(e)) raise + printer.debug('Unusing module paths from command line') + for d in site_config.get('general/0/unuse_module_paths'): + try: + rt.modules_system.searchpath_remove(d) + except errors.EnvironError as e: + printer.warning( + f'could not unuse module path {d["name"]!r} correctly; ' + f'skipping...' + ) + printer.debug(str(e)) + + printer.debug('Using module paths from command line') + for d in site_config.get('general/0/use_module_paths'): + try: + rt.modules_system.searchpath_add(d) + except errors.EnvironError as e: + printer.warning( + f'could not use module path {d["name"]!r} correctly; ' + f'skipping...' + ) + printer.debug(str(e)) + printer.debug('Loading user modules from command line') for m in site_config.get('general/0/user_modules'): try: diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json index 6fea216271..4840cfdc25 100644 --- a/reframe/schemas/config.json +++ b/reframe/schemas/config.json @@ -385,6 +385,14 @@ "items": {"type": "string"} }, "use_login_shell": {"type": "boolean"}, + "unuse_module_paths": { + "type": "array", + "items": {"type": "string"} + }, + "use_module_paths": { + "type": "array", + "items": {"type": "string"} + }, "user_modules": { "type": "array", "items": {"type": "string"} diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 32b54f976d..4d082d8fb7 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -591,6 +591,43 @@ def test_unload_module(run_reframe, user_exec_ctx): assert returncode == 0 +def test_unuse_module_path(run_reframe, user_exec_ctx, monkeypatch): + ms = rt.runtime().modules_system + if ms.name == 'nomod': + pytest.skip('no modules system found') + + # We need to have MODULEPATH set to unuse a module + monkeypatch.setenv('MODULEPATH', '') + + module_path = os.path.abspath('unittests/modules') + returncode, stdout, stderr = run_reframe( + config_file = fixtures.USER_CONFIG_FILE, + more_options=[f'--unuse-module-path={module_path}'], action='run' + ) + + assert stdout != '' + assert 'Traceback' not in stdout + assert 'Traceback' not in stderr + assert returncode == 0 + + +def test_use_module_path(run_reframe, user_exec_ctx): + ms = rt.runtime().modules_system + if ms.name == 'nomod': + pytest.skip('no modules system found') + + module_path = os.path.abspath('unittests/modules') + returncode, stdout, stderr = run_reframe( + config_file = fixtures.USER_CONFIG_FILE, + more_options=[f'--use-module-path={module_path}'], action='run' + ) + + assert stdout != '' + assert 'Traceback' not in stdout + assert 'Traceback' not in stderr + assert returncode == 0 + + def test_failure_stats(run_reframe): returncode, stdout, stderr = run_reframe( checkpath=['unittests/resources/checks/frontend_checks.py'], From dba32c19e7543328b0115ffbfce1e5ac1eaae565 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 17 Nov 2020 17:47:31 +0100 Subject: [PATCH 2/9] Fix pep8 issues --- unittests/test_cli.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 4d082d8fb7..8eb61b89f4 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -598,11 +598,10 @@ def test_unuse_module_path(run_reframe, user_exec_ctx, monkeypatch): # We need to have MODULEPATH set to unuse a module monkeypatch.setenv('MODULEPATH', '') - module_path = os.path.abspath('unittests/modules') returncode, stdout, stderr = run_reframe( - config_file = fixtures.USER_CONFIG_FILE, - more_options=[f'--unuse-module-path={module_path}'], action='run' + config_file=fixtures.USER_CONFIG_FILE, + more_options=[f'--unuse-module-path={module_path}'], action='run' ) assert stdout != '' @@ -618,7 +617,7 @@ def test_use_module_path(run_reframe, user_exec_ctx): module_path = os.path.abspath('unittests/modules') returncode, stdout, stderr = run_reframe( - config_file = fixtures.USER_CONFIG_FILE, + config_file=fixtures.USER_CONFIG_FILE, more_options=[f'--use-module-path={module_path}'], action='run' ) From ebd0f5ca45dbdafc0491170b06bb6b3fa2e5bda3 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 18 Nov 2020 16:01:13 +0100 Subject: [PATCH 3/9] Address PR comments --- docs/manpage.rst | 16 +++++++------ reframe/frontend/cli.py | 48 +++++++++++++------------------------ reframe/schemas/config.json | 6 +---- unittests/test_cli.py | 24 +++++++++---------- 4 files changed, 38 insertions(+), 56 deletions(-) diff --git a/docs/manpage.rst b/docs/manpage.rst index 9664e43ad0..7730e99e59 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -387,12 +387,13 @@ It does so by leveraging the selected system's environment modules system. This option can also be set using the :envvar:`RFM_UNLOAD_MODULES` environment variable or the :js:attr:`unload_modules` general configuration parameter. -.. option:: --unuse-module-path=PATH +.. option:: --module-path=PATH - Unuse module path ``PATH`` before acting on any tests. - This option may be specified multiple times, in which case all specified modules paths will be unused in order. + Use module path ``PATH`` before acting on any tests. + Paths prepended with the `-` character are going to be unused. + This option may be specified multiple times, in which case all specified module paths will be (un)used in order. - This option can also be set using the :envvar:`RFM_UNUSE_MODULE_PATHS` environment variable or the :js:attr:`unuse_module_paths` general configuration parameter. + This option can also be set using the :envvar:`RFM_MODULE_PATHS` environment variable or the :js:attr:`unuse_module_paths` general configuration parameter. .. versionadded:: 3.3 @@ -920,16 +921,17 @@ Here is an alphabetical list of the environment variables recognized by ReFrame: .. versionadded:: 3.3 -.. envvar:: RFM_USE_MODULE_PATHS +.. envvar:: RFM_MODULE_PATHS A colon-separated list of module paths to be used before acting on any tests. + A module path is going to be unused if it's prepended by the `-` character. .. table:: :align: left ================================== ================== - Associated command line option :option:`--use-module-path` - Associated configuration parameter :js:attr:`use_module_paths` general configuration parameter + Associated command line option :option:`--module-path` + Associated configuration parameter :js:attr:`module_paths` general configuration parameter ================================== ================== diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 38097c07d5..1c6071bf76 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -360,17 +360,11 @@ def main(): envvar='RFM_UNLOAD_MODULES ,', configvar='general/unload_modules' ) env_options.add_argument( - '--unuse-module-path', action='append', metavar='PATH', - dest='unuse_module_paths', default=[], - help='Unuse module path PATH before running any regression check', - envvar='RFM_UNUSE_MODULE_PATHS :', - configvar='general/unuse_module_paths' - ) - env_options.add_argument( - '--use-module-path', action='append', metavar='PATH', - dest='use_module_paths', default=[], - help='Use module path PATH before running any regression check', - envvar='RFM_USE_MODULE_PATHS :', configvar='general/use_module_paths' + '--module-path', action='append', metavar='PATH', + dest='module_paths', default=[], + help='(Un)use module path PATH before running any regression check', + envvar='RFM_MODULE_PATHS :', + configvar='general/module_paths' ) env_options.add_argument( '--purge-env', action='store_true', dest='purge_env', default=False, @@ -746,27 +740,19 @@ def print_infoline(param, value): printer.debug(str(e)) raise - printer.debug('Unusing module paths from command line') - for d in site_config.get('general/0/unuse_module_paths'): - try: - rt.modules_system.searchpath_remove(d) - except errors.EnvironError as e: - printer.warning( - f'could not unuse module path {d["name"]!r} correctly; ' - f'skipping...' - ) - printer.debug(str(e)) - - printer.debug('Using module paths from command line') - for d in site_config.get('general/0/use_module_paths'): - try: + printer.debug('(Un)using module paths from command line') + for d in site_config.get('general/0/module_paths'): + if d.startswith('-'): + try: + rt.modules_system.searchpath_remove(d[1:]) + except errors.EnvironError as e: + printer.warning( + f'could not unuse module path {d["name"]!r} correctly; ' + f'skipping...' + ) + printer.verbose(str(e)) + else: rt.modules_system.searchpath_add(d) - except errors.EnvironError as e: - printer.warning( - f'could not use module path {d["name"]!r} correctly; ' - f'skipping...' - ) - printer.debug(str(e)) printer.debug('Loading user modules from command line') for m in site_config.get('general/0/user_modules'): diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json index 4840cfdc25..4798015d08 100644 --- a/reframe/schemas/config.json +++ b/reframe/schemas/config.json @@ -385,11 +385,7 @@ "items": {"type": "string"} }, "use_login_shell": {"type": "boolean"}, - "unuse_module_paths": { - "type": "array", - "items": {"type": "string"} - }, - "use_module_paths": { + "module_paths": { "type": "array", "items": {"type": "string"} }, diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 8eb61b89f4..2d1507807b 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -596,18 +596,17 @@ def test_unuse_module_path(run_reframe, user_exec_ctx, monkeypatch): if ms.name == 'nomod': pytest.skip('no modules system found') - # We need to have MODULEPATH set to unuse a module - monkeypatch.setenv('MODULEPATH', '') - module_path = os.path.abspath('unittests/modules') + module_path = 'unittests/modules' + monkeypatch.setenv('MODULEPATH', module_path) returncode, stdout, stderr = run_reframe( - config_file=fixtures.USER_CONFIG_FILE, - more_options=[f'--unuse-module-path={module_path}'], action='run' + more_options=[f'--module-path=-{module_path}', '-m testmod_foo'], + config_file=fixtures.USER_CONFIG_FILE, action='run' ) - assert stdout != '' - assert 'Traceback' not in stdout - assert 'Traceback' not in stderr - assert returncode == 0 + # Here we test that ReFrame fails to run because 'testmod_foo' cannot + # be found and therefore the module name is included in the given error + assert 'testmod_foo' in stdout + assert returncode == 1 def test_use_module_path(run_reframe, user_exec_ctx): @@ -615,13 +614,12 @@ def test_use_module_path(run_reframe, user_exec_ctx): if ms.name == 'nomod': pytest.skip('no modules system found') - module_path = os.path.abspath('unittests/modules') + module_path = 'unittests/modules' returncode, stdout, stderr = run_reframe( - config_file=fixtures.USER_CONFIG_FILE, - more_options=[f'--use-module-path={module_path}'], action='run' + more_options=[f'--module-path={module_path}', '-m testmod_foo'], + config_file=fixtures.USER_CONFIG_FILE, action='run' ) - assert stdout != '' assert 'Traceback' not in stdout assert 'Traceback' not in stderr assert returncode == 0 From b11471cff38433a5146478faaeb6383058e4bee0 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 18 Nov 2020 16:05:45 +0100 Subject: [PATCH 4/9] Fix formatting of module unuse error --- reframe/frontend/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 1c6071bf76..33567b7dbf 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -747,7 +747,7 @@ def print_infoline(param, value): rt.modules_system.searchpath_remove(d[1:]) except errors.EnvironError as e: printer.warning( - f'could not unuse module path {d["name"]!r} correctly; ' + f'could not unuse module path {d} correctly; ' f'skipping...' ) printer.verbose(str(e)) From d67b50c4b9b8654965d61d04450feb514f085083 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 19 Nov 2020 17:00:44 +0100 Subject: [PATCH 5/9] Address PR comments (version 2) --- docs/manpage.rst | 35 ++++------------------------------- reframe/frontend/cli.py | 16 +++++++++++++--- reframe/schemas/config.json | 4 ---- unittests/test_cli.py | 8 +++++--- 4 files changed, 22 insertions(+), 41 deletions(-) diff --git a/docs/manpage.rst b/docs/manpage.rst index 7730e99e59..c5d0ee891b 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -389,21 +389,10 @@ It does so by leveraging the selected system's environment modules system. .. option:: --module-path=PATH - Use module path ``PATH`` before acting on any tests. - Paths prepended with the `-` character are going to be unused. - This option may be specified multiple times, in which case all specified module paths will be (un)used in order. - - This option can also be set using the :envvar:`RFM_MODULE_PATHS` environment variable or the :js:attr:`unuse_module_paths` general configuration parameter. - - .. versionadded:: 3.3 - - -.. option:: --use-module-path=PATH - - Use module path ``PATH`` before acting on any tests. - This option may be specified multiple times, in which case all specified module paths will be used in order. - - This option can also be set using the :envvar:`RFM_USE_MODULE_PATHS` environment variable or the :js:attr:`use_module_paths` general configuration parameter. + Use ``PATH`` to manipulate the ``MODULEPATH`` before acting on any tests. + Paths starting with the `-` character are going to be removed from the ``MODULEPATH``, while the ones starting with the `+` character are going to be added to it. + In all other cases, ``PATH`` will completely override MODULEPATH. + This option may be specified multiple times, in which case all the paths specified will be added or removed in order. .. versionadded:: 3.3 @@ -919,22 +908,6 @@ Here is an alphabetical list of the environment variables recognized by ReFrame: ================================== ================== -.. versionadded:: 3.3 - -.. envvar:: RFM_MODULE_PATHS - - A colon-separated list of module paths to be used before acting on any tests. - A module path is going to be unused if it's prepended by the `-` character. - - .. table:: - :align: left - - ================================== ================== - Associated command line option :option:`--module-path` - Associated configuration parameter :js:attr:`module_paths` general configuration parameter - ================================== ================== - - .. envvar:: RFM_USE_LOGIN_SHELL Use a login shell for the generated job scripts. diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index fe00de6710..5252e1dd81 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -363,8 +363,6 @@ def main(): '--module-path', action='append', metavar='PATH', dest='module_paths', default=[], help='(Un)use module path PATH before running any regression check', - envvar='RFM_MODULE_PATHS :', - configvar='general/module_paths' ) env_options.add_argument( '--purge-env', action='store_true', dest='purge_env', default=False, @@ -741,7 +739,7 @@ def print_infoline(param, value): raise printer.debug('(Un)using module paths from command line') - for d in site_config.get('general/0/module_paths'): + for d in options.module_paths: if d.startswith('-'): try: rt.modules_system.searchpath_remove(d[1:]) @@ -751,7 +749,19 @@ def print_infoline(param, value): f'skipping...' ) printer.verbose(str(e)) + elif d.startswith('+'): + try: + rt.modules_system.searchpath_add(d[1:]) + except errors.EnvironError as e: + printer.warning( + f'could not use module path {d} correctly; ' + f'skipping...' + ) + printer.verbose(str(e)) else: + rt.modules_system.searchpath_remove( + *rt.modules_system.searchpath + ) rt.modules_system.searchpath_add(d) printer.debug('Loading user modules from command line') diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json index 4798015d08..6fea216271 100644 --- a/reframe/schemas/config.json +++ b/reframe/schemas/config.json @@ -385,10 +385,6 @@ "items": {"type": "string"} }, "use_login_shell": {"type": "boolean"}, - "module_paths": { - "type": "array", - "items": {"type": "string"} - }, "user_modules": { "type": "array", "items": {"type": "string"} diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 2d1507807b..623c355fa6 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -600,7 +600,8 @@ def test_unuse_module_path(run_reframe, user_exec_ctx, monkeypatch): monkeypatch.setenv('MODULEPATH', module_path) returncode, stdout, stderr = run_reframe( more_options=[f'--module-path=-{module_path}', '-m testmod_foo'], - config_file=fixtures.USER_CONFIG_FILE, action='run' + config_file=fixtures.USER_CONFIG_FILE, action='run', + system=rt.runtime().system.name ) # Here we test that ReFrame fails to run because 'testmod_foo' cannot @@ -616,8 +617,9 @@ def test_use_module_path(run_reframe, user_exec_ctx): module_path = 'unittests/modules' returncode, stdout, stderr = run_reframe( - more_options=[f'--module-path={module_path}', '-m testmod_foo'], - config_file=fixtures.USER_CONFIG_FILE, action='run' + more_options=[f'--module-path=+{module_path}', '-m testmod_foo'], + config_file=fixtures.USER_CONFIG_FILE, action='run', + system=rt.runtime().system.name ) assert 'Traceback' not in stdout From 26da5664aa862993a5e307207b0a2a4b5682d885 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Mon, 23 Nov 2020 11:03:06 +0100 Subject: [PATCH 6/9] Address PR comments (version 3) --- docs/manpage.rst | 4 ++-- reframe/frontend/cli.py | 13 ++++++++----- unittests/test_cli.py | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/docs/manpage.rst b/docs/manpage.rst index c5d0ee891b..eded25811f 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -389,8 +389,8 @@ It does so by leveraging the selected system's environment modules system. .. option:: --module-path=PATH - Use ``PATH`` to manipulate the ``MODULEPATH`` before acting on any tests. - Paths starting with the `-` character are going to be removed from the ``MODULEPATH``, while the ones starting with the `+` character are going to be added to it. + Manipulate the ``MODULEPATH`` environment variable before acting on any tests. + If ``PATH`` starts with the `-` character it will be removed from the ``MODULEPATH``, whereas if it starts with the `+` character, it will be added to it. In all other cases, ``PATH`` will completely override MODULEPATH. This option may be specified multiple times, in which case all the paths specified will be added or removed in order. diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 5252e1dd81..749c6177e6 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -745,7 +745,7 @@ def print_infoline(param, value): rt.modules_system.searchpath_remove(d[1:]) except errors.EnvironError as e: printer.warning( - f'could not unuse module path {d} correctly; ' + f'could not remove module path {d} correctly; ' f'skipping...' ) printer.verbose(str(e)) @@ -754,14 +754,17 @@ def print_infoline(param, value): rt.modules_system.searchpath_add(d[1:]) except errors.EnvironError as e: printer.warning( - f'could not use module path {d} correctly; ' + f'could not add module path {d} correctly; ' f'skipping...' ) printer.verbose(str(e)) else: - rt.modules_system.searchpath_remove( - *rt.modules_system.searchpath - ) + # Here we make sure that we don't try to remove an empty path + # from the searchpath + searchpath = [p for p in rt.modules_system.searchpath if p] + if searchpath: + rt.modules_system.searchpath_remove(*searchpath) + rt.modules_system.searchpath_add(d) printer.debug('Loading user modules from command line') diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 623c355fa6..b23cd61f59 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -627,6 +627,23 @@ def test_use_module_path(run_reframe, user_exec_ctx): assert returncode == 0 +def test_overwrite_module_path(run_reframe, user_exec_ctx): + ms = rt.runtime().modules_system + if ms.name == 'nomod': + pytest.skip('no modules system found') + + module_path = 'unittests/modules' + returncode, stdout, stderr = run_reframe( + more_options=[f'--module-path={module_path}', '-m testmod_foo'], + config_file=fixtures.USER_CONFIG_FILE, action='run', + system=rt.runtime().system.name + ) + + assert 'Traceback' not in stdout + assert 'Traceback' not in stderr + assert returncode == 0 + + def test_failure_stats(run_reframe): returncode, stdout, stderr = run_reframe( checkpath=['unittests/resources/checks/frontend_checks.py'], From 321bad5da41fbc83d5d698616ed5b700bfc739fc Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Mon, 23 Nov 2020 14:55:50 +0100 Subject: [PATCH 7/9] Address PR comments (version 4) --- docs/manpage.rst | 2 +- reframe/core/modules.py | 27 ++++++++++++++++++++++----- reframe/frontend/cli.py | 1 + unittests/test_cli.py | 16 ++++++++-------- unittests/test_environments.py | 2 +- unittests/test_modules.py | 4 ++-- 6 files changed, 35 insertions(+), 17 deletions(-) diff --git a/docs/manpage.rst b/docs/manpage.rst index eded25811f..8f01c4403c 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -390,7 +390,7 @@ It does so by leveraging the selected system's environment modules system. .. option:: --module-path=PATH Manipulate the ``MODULEPATH`` environment variable before acting on any tests. - If ``PATH`` starts with the `-` character it will be removed from the ``MODULEPATH``, whereas if it starts with the `+` character, it will be added to it. + If ``PATH`` starts with the `-` character, it will be removed from the ``MODULEPATH``, whereas if it starts with the `+` character, it will be added to it. In all other cases, ``PATH`` will completely override MODULEPATH. This option may be specified multiple times, in which case all the paths specified will be added or removed in order. diff --git a/reframe/core/modules.py b/reframe/core/modules.py index bdbcfba882..147d84dfeb 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -215,6 +215,7 @@ def execute(self, cmd, *args): ''' return self._backend.execute(cmd, *args) + def load_module(self, name, force=False, collection=False): '''Load the module ``name``. @@ -391,10 +392,26 @@ class ModulesSystemImpl(abc.ABC): :meta private: ''' - @abc.abstractmethod def execute(self, cmd, *args): + '''Execute an arbitrary module command using the modules backend. + + :arg cmd: The command to execute, e.g., ``load``, ``restore`` etc. + :arg args: The arguments to pass to the command. + :returns: The command output. + ''' + try: + exec_output = self._execute(cmd, *args) + except SpawnedProcessError as e: + raise EnvironError from e + + return exec_output + + + @abc.abstractmethod + def _execute(self, cmd, *args): '''Execute an arbitrary command of the module system.''' + @abc.abstractmethod def available_modules(self, substr): '''Return a list of available modules, whose name contains ``substr``. @@ -530,7 +547,7 @@ def version(self): def modulecmd(self, *args): return ' '.join(['modulecmd', 'python', *args]) - def execute(self, cmd, *args): + 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: @@ -652,7 +669,7 @@ def name(self): def modulecmd(self, *args): return ' '.join([self._command, *args]) - def execute(self, cmd, *args): + 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: @@ -713,7 +730,7 @@ def name(self): def modulecmd(self, *args): return ' '.join(['modulecmd', 'python', *args]) - def execute(self, cmd, *args): + def _execute(self, cmd, *args): modulecmd = self.modulecmd(cmd, *args) completed = osext.run_command(modulecmd, check=False) namespace = {} @@ -874,7 +891,7 @@ def loaded_modules(self): def conflicted_modules(self, module): return [] - def execute(self, cmd, *args): + def _execute(self, cmd, *args): return '' def load_module(self, module): diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 749c6177e6..bb28d344f9 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -770,6 +770,7 @@ def print_infoline(param, value): printer.debug('Loading user modules from command line') for m in site_config.get('general/0/user_modules'): try: + import pdb; pdb.set_trace() rt.modules_system.load_module(**m, force=True) except errors.EnvironError as e: printer.warning( diff --git a/unittests/test_cli.py b/unittests/test_cli.py index b23cd61f59..5c192bf89b 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -599,15 +599,13 @@ def test_unuse_module_path(run_reframe, user_exec_ctx, monkeypatch): module_path = 'unittests/modules' monkeypatch.setenv('MODULEPATH', module_path) returncode, stdout, stderr = run_reframe( - more_options=[f'--module-path=-{module_path}', '-m testmod_foo'], + more_options=[f'--module-path=-{module_path}', '--module=testmod_foo'], config_file=fixtures.USER_CONFIG_FILE, action='run', system=rt.runtime().system.name ) - - # Here we test that ReFrame fails to run because 'testmod_foo' cannot - # be found and therefore the module name is included in the given error - assert 'testmod_foo' in stdout - assert returncode == 1 + assert "could not load module 'testmod_foo' correctly" in stdout + assert 'Traceback' not in stderr + assert returncode == 0 def test_use_module_path(run_reframe, user_exec_ctx): @@ -617,13 +615,14 @@ def test_use_module_path(run_reframe, user_exec_ctx): module_path = 'unittests/modules' returncode, stdout, stderr = run_reframe( - more_options=[f'--module-path=+{module_path}', '-m testmod_foo'], + more_options=[f'--module-path=+{module_path}', '--module=testmod_foo'], config_file=fixtures.USER_CONFIG_FILE, action='run', system=rt.runtime().system.name ) assert 'Traceback' not in stdout assert 'Traceback' not in stderr + assert "could not load module 'testmod_foo' correctly" not in stdout assert returncode == 0 @@ -634,13 +633,14 @@ def test_overwrite_module_path(run_reframe, user_exec_ctx): module_path = 'unittests/modules' returncode, stdout, stderr = run_reframe( - more_options=[f'--module-path={module_path}', '-m testmod_foo'], + more_options=[f'--module-path={module_path}', '--module=testmod_foo'], config_file=fixtures.USER_CONFIG_FILE, action='run', system=rt.runtime().system.name ) assert 'Traceback' not in stdout assert 'Traceback' not in stderr + assert "could not load module 'testmod_foo' correctly" not in stdout assert returncode == 0 diff --git a/unittests/test_environments.py b/unittests/test_environments.py index 60df3f7f6f..3e55bc9858 100644 --- a/unittests/test_environments.py +++ b/unittests/test_environments.py @@ -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(SpawnedProcessError): + with contextlib.suppress(EnvironError): rt.emit_loadenv_commands(environ) assert rt.snapshot() == snap diff --git a/unittests/test_modules.py b/unittests/test_modules.py index dc6e5c02d1..db73105aa5 100644 --- a/unittests/test_modules.py +++ b/unittests/test_modules.py @@ -80,7 +80,7 @@ def test_module_load(modules_system): modules_system.load_module('foo') modules_system.unload_module('foo') else: - with pytest.raises(SpawnedProcessError): + with pytest.raises(EnvironError): modules_system.load_module('foo') assert not modules_system.is_module_loaded('foo') @@ -307,7 +307,7 @@ def loaded_modules(self): def conflicted_modules(self, module): return [] - def execute(self, cmd, *args): + def _execute(self, cmd, *args): return '' def load_module(self, module): From ac74d238a4da422da5616c58d7151ef4d775f69b Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Mon, 23 Nov 2020 15:02:05 +0100 Subject: [PATCH 8/9] Remove stale pdb line --- reframe/frontend/cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index bb28d344f9..749c6177e6 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -770,7 +770,6 @@ def print_infoline(param, value): printer.debug('Loading user modules from command line') for m in site_config.get('general/0/user_modules'): try: - import pdb; pdb.set_trace() rt.modules_system.load_module(**m, force=True) except errors.EnvironError as e: printer.warning( From 0369e4ec1504689bfb1c25e95617fe603e7a4e6a Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 23 Nov 2020 20:07:41 +0100 Subject: [PATCH 9/9] Style fixes --- reframe/core/modules.py | 5 +---- unittests/test_environments.py | 2 +- unittests/test_modules.py | 4 +--- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 147d84dfeb..4eb238fbfa 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -215,7 +215,6 @@ def execute(self, cmd, *args): ''' return self._backend.execute(cmd, *args) - def load_module(self, name, force=False, collection=False): '''Load the module ``name``. @@ -402,16 +401,14 @@ def execute(self, cmd, *args): try: exec_output = self._execute(cmd, *args) except SpawnedProcessError as e: - raise EnvironError from e + raise EnvironError('could not execute module operation') from e return exec_output - @abc.abstractmethod def _execute(self, cmd, *args): '''Execute an arbitrary command of the module system.''' - @abc.abstractmethod def available_modules(self, substr): '''Return a list of available modules, whose name contains ``substr``. diff --git a/unittests/test_environments.py b/unittests/test_environments.py index 3e55bc9858..1e9fa86a6e 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, SpawnedProcessError) +from reframe.core.exceptions import EnvironError @pytest.fixture diff --git a/unittests/test_modules.py b/unittests/test_modules.py index db73105aa5..3d80ba4b05 100644 --- a/unittests/test_modules.py +++ b/unittests/test_modules.py @@ -12,9 +12,7 @@ import reframe.utility as util import reframe.utility.osext as osext import unittests.fixtures as fixtures -from reframe.core.exceptions import (ConfigError, - EnvironError, - SpawnedProcessError) +from reframe.core.exceptions import (ConfigError, EnvironError) from reframe.core.runtime import runtime