From f7c97f0ad3259432cc487c8dba1871caed0c724a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 18 Dec 2025 12:16:20 +0100 Subject: [PATCH 1/7] Fix URLError handling in tests When the exception is thrown and ignored `stdout` is unassigned so the following tests cannot be done. --- test/framework/options.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index 9ca3bc07f4..1dc3d327ce 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -1529,11 +1529,11 @@ def test_copy_ec_from_commit(self): stdout = self.mocked_main(args) except URLError as err: print("Ignoring URLError '%s' in test_copy_ec_from_commit" % err) - - pattern = "_%s/e/EasyBuild/EasyBuild-4.8.2.eb copied to " % test_commit - self.assertIn(pattern, stdout) - copied_ecs = os.listdir(test_dir) - self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) + else: + pattern = "_%s/e/EasyBuild/EasyBuild-4.8.2.eb copied to " % test_commit + self.assertIn(pattern, stdout) + copied_ecs = os.listdir(test_dir) + self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) # cleanup remove_dir(test_dir) @@ -1547,10 +1547,10 @@ def test_copy_ec_from_commit(self): stdout = self.mocked_main(args) except URLError as err: print("Ignoring URLError '%s' in test_copy_ec_from_commit" % err) - - self.assertIn(pattern, stdout) - copied_ecs = os.listdir(test_dir) - self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) + else: + self.assertIn(pattern, stdout) + copied_ecs = os.listdir(test_dir) + self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) # cleanup change_dir(cwd) @@ -1574,10 +1574,10 @@ def test_copy_ec_from_commit(self): stdout = self.mocked_main(args) except URLError as err: print("Ignoring URLError '%s' in test_copy_ec_from_commit" % err) - - copied_ecs = os.listdir(test_dir) - for ec in expected_ecs: - self.assertIn(ec, copied_ecs) + else: + copied_ecs = os.listdir(test_dir) + for ec in expected_ecs: + self.assertIn(ec, copied_ecs) def test_dry_run(self): """Test dry run (long format).""" From 146fb35e40a453d8a942631653095316cb8300a7 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 18 Dec 2025 12:20:31 +0100 Subject: [PATCH 2/7] Don't remove tmpdir in URLError path unless required C&P that code is not necessary when `tmpdir` isn't used or the catch is at the end of a test. --- test/framework/options.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index 1dc3d327ce..9727d3354c 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2063,7 +2063,6 @@ def test_github_from_pr(self): self.assertRegex(outtxt, r"Extended list of robot search paths with \['%s'\]:" % pr_tmpdir) except URLError as err: print("Ignoring URLError '%s' in test_from_pr" % err) - shutil.rmtree(tmpdir) # test with multiple prs tmpdir = tempfile.mkdtemp() @@ -2096,7 +2095,6 @@ def test_github_from_pr(self): except URLError as err: print("Ignoring URLError '%s' in test_from_pr" % err) - shutil.rmtree(tmpdir) def test_github_from_pr_token_log(self): """Check that --from-pr doesn't leak GitHub token in log.""" @@ -2180,7 +2178,6 @@ def test_github_from_pr_listed_ecs(self): except URLError as err: print("Ignoring URLError '%s' in test_from_pr" % err) - shutil.rmtree(tmpdir) def test_github_from_pr_x(self): """Test combination of --from-pr with --extended-dry-run.""" @@ -2304,7 +2301,6 @@ def test_from_commit(self): self.assertRegex(outtxt, r"Extended list of robot search paths with \['%s'\]:" % pr_tmpdir) except URLError as err: print("Ignoring URLError '%s' in test_from_commit" % err) - shutil.rmtree(tmpdir) # must be run after test for --list-easyblocks, hence the '_xxx_' # cleaning up the imported easyblocks is quite difficult... @@ -2356,7 +2352,6 @@ def test_xxx_include_easyblocks_from_commit(self): except URLError as err: print("Ignoring URLError '%s' in test_include_easyblocks_from_commit" % err) - shutil.rmtree(tmpdir) def test_no_such_software(self): """Test using no arguments.""" From 78c2bead61f1ce319aa2577910434357b990dd13 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 14 Nov 2025 11:01:01 +0100 Subject: [PATCH 3/7] Add `--debug-module-cmds` option This is disabled by default and hides the output of the module command. That is usually just "noise" hiding the relevant part of the log. With it disabled the output looks like: > == 2025-11-14 10:58:31,197 run.py:511 INFO Running shell command '/opt/lmod/lmod/libexec/lmod python load GCC/4.6.3' in /git/easybuild-framework > == 2025-11-14 10:58:31,261 run.py:625 INFO Shell command completed successfully: /opt/lmod/lmod/libexec/lmod python load GCC/4.6.3 --- easybuild/framework/easyblock.py | 2 +- easybuild/tools/config.py | 1 + easybuild/tools/modules.py | 14 +++++++------- easybuild/tools/options.py | 1 + test/framework/options.py | 17 +++++++++++++++++ 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index c6fade8006..2503bb7f04 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1919,7 +1919,7 @@ def clean_up_fake_module(self, fake_mod_data): # self.short_mod_name might not be set (e.g. during unit tests) if fake_mod_path and self.short_mod_name is not None: try: - self.modules_tool.unload([self.short_mod_name], log_changes=False) + self.modules_tool.unload([self.short_mod_name], hide_output=True) self.modules_tool.remove_module_path(os.path.join(fake_mod_path, self.mod_subdir)) remove_dir(os.path.dirname(fake_mod_path)) except OSError as err: diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index b27d238bd5..3950f96ba3 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -312,6 +312,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): 'cuda_sanity_check_strict', 'debug', 'debug_lmod', + 'debug_module_cmds', 'dump_autopep8', 'dump_env_script', 'enforce_checksums', diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 75fd040200..a76401cc59 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1130,12 +1130,12 @@ def load(self, modules, mod_paths=None, purge=False, init_env=None, allow_reload for mod in modules: self.run_module('load', mod) - def unload(self, modules, log_changes=True): + def unload(self, modules, hide_output=False): """ Unload all requested modules. """ for mod in modules: - self.run_module('unload', mod, log_changes=log_changes) + self.run_module('unload', mod, hide_output=hide_output) def purge(self): """ @@ -1259,13 +1259,13 @@ def run_module(self, *args, **kwargs): self.log.debug("Changing %s from '%s' to '%s' in environment for module command", key, old_value, new_value) - log_changes = kwargs.get('log_changes', True) + log_output = build_option('debug_module_cmds') and not kwargs.get('hide_output', False) cmd_list = self.compose_cmd_list(args) cmd = ' '.join(cmd_list) # note: module commands are always run in dry mode, and are kept hidden in trace and dry run output res = run_shell_cmd(cmd_list, env=environ, fail_on_error=False, use_bash=False, split_stderr=True, hidden=True, in_dry_run=True, output_file=False, - log_output_on_success=log_changes) + log_output_on_success=log_output) # stdout will contain python code (to change environment etc) # stderr will contain text (just like the normal module command) @@ -1312,7 +1312,7 @@ def run_module(self, *args, **kwargs): if new_ld_val != curr_ld_val: self.log.debug("Correcting paths in $%s from %s to %s" % (key, curr_ld_val, new_ld_val)) - self.set_path_env_var(key, new_ld_val) + self.set_path_env_var(key, new_ld_val, log_changes=log_output) # Process stderr result = [] @@ -1695,9 +1695,9 @@ class EnvironmentModulesTcl(EnvironmentModulesC): DEPR_VERSION = '9999.9' VERSION_REGEXP = r'^Modules\s+Release\s+Tcl\s+(?P\d\S*)\s' - def set_path_env_var(self, key, paths): + def set_path_env_var(self, key, paths, log_changes=True): """Set environment variable with given name to the given list of paths.""" - super().set_path_env_var(key, paths) + super().set_path_env_var(key, paths, log_changes) # for Tcl Environment Modules, we need to make sure the _modshare env var is kept in sync setvar('%s_modshare' % key, ':1:'.join(paths), verbose=False) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index f4856de579..edc9b74bc3 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -438,6 +438,7 @@ def override_options(self): "more compute capabilities than defined in --cuda-compute-capabilities.", None, 'store_true', False), 'debug-lmod': ("Run Lmod modules tool commands in debug module", None, 'store_true', False), + 'debug-module-cmds': ("Show additional information of module commands", None, 'store_true', False), 'default-opt-level': ("Specify default optimisation level", 'choice', 'store', DEFAULT_OPT_LEVEL, Compiler.COMPILER_OPT_OPTIONS), 'deprecated': ("Run pretending to be (future) version, to test removal of deprecated code.", diff --git a/test/framework/options.py b/test/framework/options.py index 9727d3354c..030b6609db 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -5681,6 +5681,23 @@ def test_debug_lmod(self): else: print("Skipping test_debug_lmod, requires Lmod as modules tool") + def test_debug_module_cmds(self): + """Test use of --debug-module-cmds.""" + patterns = [ + "Output of ", + r"os\.env.*EBROOTGCC.*=", + r"os\.env.*PATH.*=", + ] + for enable in (True, False): + with self.subTest(debug_module_cmds=enable): + init_config(build_options={'debug_module_cmds': enable}) + with self.log_to_testlogfile(): + self.modtool.load(['GCC/4.6.3']) + logtxt = read_file(self.logfile) + self.assertRegex(logtxt, "Running .*\n.*load GCC/4.6.3") + self.assert_multi_regex(patterns, logtxt, assert_true=enable) + self.modtool.purge() + def test_use_color(self): """Test use_color function.""" self.assertTrue(use_color('always')) From b0f4781552200f7b2c33b33768a7cd28b73c56d5 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 14 Nov 2025 11:03:09 +0100 Subject: [PATCH 4/7] Add type hint for `self.modtool` --- easybuild/tools/modules.py | 2 +- test/framework/utilities.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index a76401cc59..08f0611eda 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -2263,7 +2263,7 @@ def avail_modules_tools(): return class_dict -def modules_tool(mod_paths=None, testing=False): +def modules_tool(mod_paths=None, testing=False) -> ModulesTool: """ Return interface to modules tool (EnvironmentModules, Lmod, ...) """ diff --git a/test/framework/utilities.py b/test/framework/utilities.py index 1807085d75..1d0fe1ce38 100644 --- a/test/framework/utilities.py +++ b/test/framework/utilities.py @@ -53,7 +53,7 @@ from easybuild.tools.configobj import ConfigObj from easybuild.tools.environment import modify_env from easybuild.tools.filetools import copy_dir, mkdir, read_file, which -from easybuild.tools.modules import curr_module_paths, modules_tool, reset_module_caches +from easybuild.tools.modules import curr_module_paths, modules_tool, ModulesTool, reset_module_caches from easybuild.tools.options import CONFIG_ENV_VAR_PREFIX, EasyBuildOptions, set_tmpdir @@ -211,7 +211,7 @@ def setUp(self): self.env_path = os.environ.get('PATH') self.env_pythonpath = os.environ.get('PYTHONPATH') - self.modtool = modules_tool() + self.modtool: ModulesTool = modules_tool() self.reset_modulepath([os.path.join(testdir, 'modules')]) reset_module_caches() From 44b98005603f87e541ec800f11a4d5596b8767a6 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 14 Nov 2025 11:03:36 +0100 Subject: [PATCH 5/7] Remove unused argument from test function --- test/framework/options.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index 030b6609db..4b5b6fd34b 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -3227,7 +3227,7 @@ def test_http_header_fields_urlpat(self): mentionhdr = 'Custom HTTP header field set: %s' mentionfile = 'File included in parse_http_header_fields_urlpat: %s' - def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): + def run_and_assert(args, words_expected=None, words_unexpected=None): stdout, _stderr = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False) if words_expected is not None: self.assert_multi_regex(words_expected, stdout) @@ -3242,7 +3242,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): ]) # expect to find everything passed on cmdline expected = [mentionhdr % (testdohdr), testdoval, testdonthdr, testdontval] - run_and_assert(args, "case A", expected) + run_and_assert(args, expected) # all subsequent tests share this argument list args = common_args @@ -3258,7 +3258,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): # expect to find only the header key (not its value) and only for the appropriate url expected = [mentionhdr % testdohdr, mentionfile % testcmdfile] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case B", expected, not_expected) + run_and_assert(args, expected, not_expected) # C: recursion one: header value is another file txt = '\n'.join([ @@ -3273,7 +3273,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): expected = [mentionhdr % (testdohdr), mentionfile % (testcmdfile), mentionfile % (testincfile), mentionfile % (testexcfile)] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case C", expected, not_expected) + run_and_assert(args, expected, not_expected) # D: recursion two: header field+value is another file, write_file(testcmdfile, '\n'.join([ @@ -3287,7 +3287,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): expected = [mentionhdr % (testdohdr), mentionfile % (testcmdfile), mentionfile % (testinchdrfile), mentionfile % (testexchdrfile)] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case D", expected, not_expected) + run_and_assert(args, expected, not_expected) # E: recursion three: url pattern + header field + value in another file write_file(testcmdfile, '%s\n' % (testurlpatfile)) @@ -3300,7 +3300,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): # expect to find only the header key (but not the literal filename) and only for the appropriate url expected = [mentionhdr % (testdohdr), mentionfile % (testcmdfile), mentionfile % (testurlpatfile)] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case E", expected, not_expected) + run_and_assert(args, expected, not_expected) # cleanup downloads shutil.rmtree(tmpdir) From c66354f51041ef56c0a8ffa7460f0401c8c4e824 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 14 Nov 2025 11:04:33 +0100 Subject: [PATCH 6/7] Fix misplaced argument --- test/framework/modules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/framework/modules.py b/test/framework/modules.py index b69bb69c3e..64aa5d7489 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1070,13 +1070,13 @@ def test_modules_tool_stateless(self): modtxt = read_file(os.path.join(self.test_prefix, 'Core', 'GCC', '6.4.0-2.28')) modpath_extension = os.path.join(self.test_prefix, 'Compiler', 'GCC', '6.4.0-2.28') - modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, re.M) + modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, flags=re.M) write_file(os.path.join(self.test_prefix, 'Core', 'GCC', '6.4.0-2.28'), modtxt) modtxt = read_file(os.path.join(self.test_prefix, 'Compiler', 'GCC', '6.4.0-2.28', 'OpenMPI', '2.1.2')) modpath_extension = os.path.join(self.test_prefix, 'MPI', 'GCC', '6.4.0-2.28', 'OpenMPI', '2.1.2') mkdir(modpath_extension, parents=True) - modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, re.M) + modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, flags=re.M) write_file(os.path.join(self.test_prefix, 'Compiler', 'GCC', '6.4.0-2.28', 'OpenMPI', '2.1.2'), modtxt) # force reset of any singletons by reinitiating config From 549c4ec330e01a2292393f0b4455e9ce3669781a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 14 Nov 2025 16:01:52 +0100 Subject: [PATCH 7/7] Dump command env for module commands in debug mode --- easybuild/tools/modules.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 08f0611eda..508e7bc15b 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1259,12 +1259,13 @@ def run_module(self, *args, **kwargs): self.log.debug("Changing %s from '%s' to '%s' in environment for module command", key, old_value, new_value) - log_output = build_option('debug_module_cmds') and not kwargs.get('hide_output', False) + debug_module_cmds = build_option('debug_module_cmds') + log_output = debug_module_cmds and not kwargs.get('hide_output', False) cmd_list = self.compose_cmd_list(args) cmd = ' '.join(cmd_list) # note: module commands are always run in dry mode, and are kept hidden in trace and dry run output res = run_shell_cmd(cmd_list, env=environ, fail_on_error=False, use_bash=False, split_stderr=True, - hidden=True, in_dry_run=True, output_file=False, + hidden=True, in_dry_run=True, output_file=debug_module_cmds, log_output_on_success=log_output) # stdout will contain python code (to change environment etc)