From 2c457e1de3b7f0c4484978162c3b650aa46c098f Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 11 Sep 2024 16:58:55 +0200 Subject: [PATCH 1/5] Remove superflous parameters from EB invocations in tests ``` '--sourcepath=%s' % self.test_sourcepath, '--buildpath=%s' % self.test_buildpath, '--installpath=%s' % self.test_installpath, ``` is not required because the test setup sets those via env variables. --- test/framework/options.py | 42 ------------------------------------- test/framework/toy_build.py | 35 ------------------------------- 2 files changed, 77 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index f10d01cf8d..95ce5aa692 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -301,9 +301,6 @@ def test_skip(self): # check log message with --skip for existing module args = [ toy_ec, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--force', '--debug', ] @@ -326,9 +323,6 @@ def test_skip(self): # check log message with --skip for non-existing module args = [ toy_ec, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--try-software-version=1.2.3.4.5.6.7.8.9', '--try-amend=sources=toy-0.0.tar.gz,toy-0.0.tar.gz', # hackish, but fine '--force', @@ -1702,9 +1696,6 @@ def test_try_robot_force(self): args = [ eb1, eb2, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--force', '--robot=%s' % test_ecs, @@ -2364,9 +2355,6 @@ def test_header_footer(self): # check log message with --skip for existing module args = [ eb_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--force', '--modules-header=%s' % modules_header, @@ -2405,9 +2393,6 @@ def test_recursive_module_unload(self): for lastarg in lastargs: args = [ eb_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--force', lastarg, @@ -2439,9 +2424,6 @@ def test_tmpdir(self): # check log message with --skip for existing module args = [ eb_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--tmpdir=%s' % tmpdir, ] @@ -2673,9 +2655,6 @@ def test_try(self): args = [ tweaked_toy_ec, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--dry-run', '--robot=%s' % ecs_path, ] @@ -2753,9 +2732,6 @@ def test_try_with_copy(self): args = [ tweaked_toy_ec, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--dry-run', '--robot=%s' % ecs_path, '--copy-ec', @@ -2798,9 +2774,6 @@ def test_software_version_ordering(self): args = [ '--software=GCC,4.10.1', - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--dry-run', '--robot=%s:%s' % (ecs_path, self.test_prefix), ] @@ -2821,8 +2794,6 @@ def test_recursive_try(self): args = [ tweaked_toy_ec, '--sourcepath=%s' % sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--try-toolchain=gompi,2018a', '--robot=%s' % ecs_path, '--ignore-osdeps', @@ -2923,8 +2894,6 @@ def test_filter_deps(self): os.environ['MODULEPATH'] = os.path.join(test_dir, 'modules') args = [ ec_file, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--robot=%s' % os.path.join(test_dir, 'easyconfigs'), '--dry-run', ] @@ -3059,8 +3028,6 @@ def test_hide_deps(self): os.environ['MODULEPATH'] = os.path.join(test_dir, 'modules') args = [ ec_file, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--robot=%s' % os.path.join(test_dir, 'easyconfigs'), '--dry-run', ] @@ -3302,9 +3269,6 @@ def toy(extra_args=None): eb_file = os.path.join(os.path.dirname(__file__), 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') args = [ eb_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--force', '--debug', ] @@ -4376,9 +4340,6 @@ def test_extended_dry_run(self): ec_file = os.path.join(os.path.dirname(__file__), 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') args = [ ec_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--disable-rpath', ] @@ -6609,9 +6570,6 @@ def test_tmp_logdir(self): # check log message with --skip for existing module args = [ toy_ec, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--force', '--debug', '--tmp-logdir=%s' % tmp_logdir, diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index afe0b86bef..9a440319f2 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -170,9 +170,6 @@ def _test_toy_build(self, extra_args=None, ec_file=None, tmpdir=None, verify=Tru full_ver = '0.0%s' % versionsuffix args = [ ec_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--unittest-file=%s' % self.logfile, '--robot=%s' % os.pathsep.join([self.test_buildpath, os.path.dirname(__file__)]), ] @@ -315,9 +312,6 @@ def test_toy_tweaked(self): args = [ ec_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--force', ] @@ -393,9 +387,6 @@ def test_toy_build_formatv2(self): args = [ os.path.join(os.path.dirname(__file__), 'easyconfigs', 'v2.0', 'toy.eb'), - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--force', @@ -429,9 +420,6 @@ def test_toy_build_with_blocks(self): args = [ 'toy-0.0-multiple.eb', - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--force', @@ -465,9 +453,6 @@ def test_toy_build_formatv2_sections(self): for version, specs in versions.items(): args = [ os.path.join(os.path.dirname(__file__), 'easyconfigs', 'v2.0', 'toy-with-sections.eb'), - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--force', @@ -525,9 +510,6 @@ def test_toy_permissions(self): write_file(test_ec, test_ec_txt) args = [ - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--force', @@ -844,9 +826,6 @@ def test_toy_hierarchical(self): args = [ os.path.join(test_easyconfigs, 't', 'toy', 'toy-0.0.eb'), - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--force', @@ -1047,8 +1026,6 @@ def test_toy_hierarchical_subdir_user_modules(self): args = [ os.path.join(test_easyconfigs, 't', 'toy', 'toy-0.0-gompi-2018a.eb'), - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, '--installpath=%s' % home, '--unittest-file=%s' % self.logfile, '--force', @@ -1216,9 +1193,6 @@ def test_module_filepath_tweaking(self): eb_file = os.path.join(topdir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') args = [ eb_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--force', '--debug', '--suffix-modules-path=foobarbaz', @@ -1719,9 +1693,6 @@ def test_module_only(self): # sanity check fails without --force if software is not installed yet common_args = [ ec_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--robot=%s' % ec_files_path, @@ -2001,9 +1972,6 @@ def test_backup_modules(self): common_args = [ ec_file, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--robot=%s' % ec_files_path, @@ -2423,9 +2391,6 @@ def test_toy_sanity_check_commands(self): args = [ tweaked_toy_ec, - '--sourcepath=%s' % self.test_sourcepath, - '--buildpath=%s' % self.test_buildpath, - '--installpath=%s' % self.test_installpath, '--debug', '--unittest-file=%s' % self.logfile, '--force', From 9b4b7b319b18081591bb2b7d3e7d5542cac23f92 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 11 Sep 2024 16:52:29 +0200 Subject: [PATCH 2/5] Rename SOURCE_STEP to EXTRACT_STEP Adjust tests to take that into account and add tests to check that the old name causes a proper error. Add such error handling for the `skipsteps` EC parameter. Split up `test_skip` into more focused sub tests. --- easybuild/framework/easyblock.py | 11 +- easybuild/framework/easyconfig/easyconfig.py | 17 ++- easybuild/tools/hooks.py | 4 +- easybuild/tools/options.py | 4 +- test/framework/hooks.py | 25 +++-- test/framework/options.py | 103 ++++++++++++------- 6 files changed, 102 insertions(+), 62 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 88da255d1d..62db3a9c5b 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -86,10 +86,11 @@ from easybuild.tools.filetools import get_cwd, get_source_tarball_from_git, is_alt_pypi_url from easybuild.tools.filetools import is_binary, is_sha256_checksum, mkdir, move_file, move_logs, read_file, remove_dir from easybuild.tools.filetools import remove_file, remove_lock, verify_checksum, weld_paths, write_file, symlink -from easybuild.tools.hooks import BUILD_STEP, CLEANUP_STEP, CONFIGURE_STEP, EXTENSIONS_STEP, FETCH_STEP, INSTALL_STEP -from easybuild.tools.hooks import MODULE_STEP, MODULE_WRITE, PACKAGE_STEP, PATCH_STEP, PERMISSIONS_STEP, POSTITER_STEP -from easybuild.tools.hooks import POSTPROC_STEP, PREPARE_STEP, READY_STEP, SANITYCHECK_STEP, SOURCE_STEP -from easybuild.tools.hooks import SINGLE_EXTENSION, TEST_STEP, TESTCASES_STEP, load_hooks, run_hook +from easybuild.tools.hooks import ( + BUILD_STEP, CLEANUP_STEP, CONFIGURE_STEP, EXTENSIONS_STEP, EXTRACT_STEP, FETCH_STEP, INSTALL_STEP, MODULE_STEP, + MODULE_WRITE, PACKAGE_STEP, PATCH_STEP, PERMISSIONS_STEP, POSTITER_STEP, POSTPROC_STEP, PREPARE_STEP, READY_STEP, + SANITYCHECK_STEP, SINGLE_EXTENSION, TEST_STEP, TESTCASES_STEP, load_hooks, run_hook, +) from easybuild.tools.run import RunShellCmdError, raise_run_shell_cmd_error, run_shell_cmd from easybuild.tools.jenkins import write_to_xml from easybuild.tools.module_generator import ModuleGeneratorLua, ModuleGeneratorTcl, module_generator, dependencies_for @@ -4077,7 +4078,7 @@ def install_step_spec(initial): # format for step specifications: (step_name, description, list of functions, skippable) # core steps that are part of the iterated loop - extract_step_spec = (SOURCE_STEP, "unpacking", [lambda x: x.extract_step], True) + extract_step_spec = (EXTRACT_STEP, "unpacking", [lambda x: x.extract_step], True) patch_step_spec = (PATCH_STEP, 'patching', [lambda x: x.patch_step], True) prepare_step_spec = (PREPARE_STEP, 'preparing', [lambda x: x.prepare_step], False) configure_step_spec = (CONFIGURE_STEP, 'configuring', [lambda x: x.configure_step], True) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 0d54443517..71c771958c 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -72,7 +72,7 @@ from easybuild.tools.filetools import convert_name, copy_file, create_index, decode_class_name, encode_class_name from easybuild.tools.filetools import find_backup_name_candidate, find_easyconfigs, load_index from easybuild.tools.filetools import read_file, write_file -from easybuild.tools.hooks import PARSE, load_hooks, run_hook +from easybuild.tools.hooks import STEP_NAMES, PARSE, load_hooks, run_hook from easybuild.tools.module_naming_scheme.mns import DEVEL_MODULE_SUFFIX from easybuild.tools.module_naming_scheme.utilities import avail_module_naming_schemes, det_full_ec_version from easybuild.tools.module_naming_scheme.utilities import det_hidden_modname, is_valid_module_name @@ -860,9 +860,20 @@ def validate(self, check_osdeps=True): self.log.info("Not checking OS dependencies") self.log.info("Checking skipsteps") - if not isinstance(self._config['skipsteps'][0], (list, tuple,)): + skipsteps = self._config['skipsteps'][0] + if not isinstance(skipsteps, (list, tuple,)): raise EasyBuildError('Invalid type for skipsteps. Allowed are list or tuple, got %s (%s)', - type(self._config['skipsteps'][0]), self._config['skipsteps'][0]) + type(skipsteps), skipsteps) + unknown_step_names = [step for step in skipsteps if step not in STEP_NAMES] + if unknown_step_names: + error_lines = ["Found one or more unknown step names in skipsteps:"] + for step in unknown_step_names: + error_lines.append("* %s" % step) + # try to find close match, may be just a typo in the hook name + close_matches = difflib.get_close_matches(step, STEP_NAMES, 2, 0.8) + if close_matches: + error_lines[-1] += " (did you mean %s?)" % ', or '.join("'%s'" % s for s in close_matches) + raise EasyBuildError('\n'.join(error_lines)) self.log.info("Checking build option lists") self.validate_iterate_opts_lists() diff --git a/easybuild/tools/hooks.py b/easybuild/tools/hooks.py index b114224f95..aaeabfe314 100644 --- a/easybuild/tools/hooks.py +++ b/easybuild/tools/hooks.py @@ -55,7 +55,7 @@ PREPARE_STEP = 'prepare' READY_STEP = 'ready' SANITYCHECK_STEP = 'sanitycheck' -SOURCE_STEP = 'source' +EXTRACT_STEP = 'extract' TEST_STEP = 'test' TESTCASES_STEP = 'testcases' @@ -77,7 +77,7 @@ HOOK_SUFF = '_hook' # list of names for steps in installation procedure (in order of execution) -STEP_NAMES = [FETCH_STEP, READY_STEP, SOURCE_STEP, PATCH_STEP, PREPARE_STEP, CONFIGURE_STEP, BUILD_STEP, TEST_STEP, +STEP_NAMES = [FETCH_STEP, READY_STEP, EXTRACT_STEP, PATCH_STEP, PREPARE_STEP, CONFIGURE_STEP, BUILD_STEP, TEST_STEP, INSTALL_STEP, EXTENSIONS_STEP, POSTITER_STEP, POSTPROC_STEP, SANITYCHECK_STEP, CLEANUP_STEP, MODULE_STEP, PERMISSIONS_STEP, PACKAGE_STEP, TESTCASES_STEP] diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index a61daa6d1c..492015c5da 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -51,7 +51,7 @@ from easybuild.base import fancylogger # build_log should always stay there, to ensure EasyBuildLog from easybuild.base.fancylogger import setLogLevel from easybuild.base.generaloption import GeneralOption -from easybuild.framework.easyblock import MODULE_ONLY_STEPS, SOURCE_STEP, FETCH_STEP, EasyBlock +from easybuild.framework.easyblock import MODULE_ONLY_STEPS, EXTRACT_STEP, FETCH_STEP, EasyBlock from easybuild.framework.easyconfig import EASYCONFIGS_PKG_SUBDIR from easybuild.framework.easyconfig.easyconfig import HAVE_AUTOPEP8 from easybuild.framework.easyconfig.format.one import EB_FORMAT_EXTENSION @@ -292,7 +292,7 @@ def basic_options(self): 'skip': ("Skip existing software (useful for installing additional packages)", None, 'store_true', False, 'k'), 'stop': ("Stop the installation after certain step", - 'choice', 'store_or_None', SOURCE_STEP, 's', all_stops), + 'choice', 'store_or_None', EXTRACT_STEP, 's', all_stops), 'strict': ("Set strictness level", 'choice', 'store', WARN, strictness_options), }) diff --git a/test/framework/hooks.py b/test/framework/hooks.py index 0cc478f53e..6f005e3d93 100644 --- a/test/framework/hooks.py +++ b/test/framework/hooks.py @@ -29,6 +29,7 @@ """ import os import sys +import textwrap from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, init_config from unittest import TextTestRunner @@ -280,22 +281,24 @@ def test_verify_hooks(self): self.assertEqual(verify_hooks(hooks), None) test_broken_hooks_pymod = os.path.join(self.test_prefix, 'test_broken_hooks.py') - test_hooks_txt = '\n'.join([ - '', - 'def there_is_no_such_hook():', - ' pass', - 'def stat_hook(self):', - ' pass', - 'def post_source_hook(self):', - ' pass', - 'def install_hook(self):', - ' pass', - ]) + test_hooks_txt = textwrap.dedent(""" + def there_is_no_such_hook(): + pass + def stat_hook(self): + pass + def post_source_hook(self): + pass + def post_extract_hook(self): + pass + def install_hook(self): + pass + """) write_file(test_broken_hooks_pymod, test_hooks_txt) error_msg_pattern = r"Found one or more unknown hooks:\n" error_msg_pattern += r"\* install_hook \(did you mean 'pre_install_hook', or 'post_install_hook'\?\)\n" + error_msg_pattern += r"\* post_source_hook \(did you mean .*'\?\)\n" error_msg_pattern += r"\* stat_hook \(did you mean 'start_hook'\?\)\n" error_msg_pattern += r"\* there_is_no_such_hook\n\n" error_msg_pattern += r"Run 'eb --avail-hooks' to get an overview of known hooks" diff --git a/test/framework/options.py b/test/framework/options.py index 95ce5aa692..ef6e3ca860 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -293,7 +293,6 @@ def test_force(self): def test_skip(self): """Test skipping installation of module (--skip, -k).""" - # use toy-0.0.eb easyconfig file that comes with the tests topdir = os.path.abspath(os.path.dirname(__file__)) toy_ec = os.path.join(topdir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') @@ -332,13 +331,11 @@ def test_skip(self): with self.mocked_stdout_stderr(): outtxt = self.eb_main(args, do_build=True, verbose=True) - found_msg = "Module toy/1.2.3.4.5.6.7.8.9 found." - found = re.search(found_msg, outtxt) - self.assertTrue(not found, "Module found message not there with --skip for non-existing modules: %s" % outtxt) + self.assertNotIn("Module toy/1.2.3.4.5.6.7.8.9 found.", outtxt, + "Module found message should not be there with --skip for non-existing modules") - not_found_msg = "No module toy/1.2.3.4.5.6.7.8.9 found. Not skipping anything." - not_found = re.search(not_found_msg, outtxt) - self.assertTrue(not_found, "Module not found message there with --skip for non-existing modules: %s" % outtxt) + self.assertIn("No module toy/1.2.3.4.5.6.7.8.9 found. Not skipping anything.", outtxt, + "Module not found message should be there with --skip for non-existing modules") toy_mod_glob = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '*') for toy_mod in glob.glob(toy_mod_glob): @@ -357,29 +354,26 @@ def test_skip(self): '--force', ] error_pattern = "Sanity check failed: no file found at 'bin/nosuchfile'" - with self.mocked_stdout_stderr(): - self.assertErrorRegex(EasyBuildError, error_pattern, self.eb_main, args, do_build=True, raise_error=True) + self.assertErrorRegex(EasyBuildError, error_pattern, self.mocked_main, args, do_build=True, raise_error=True) - # check use of skipsteps to skip sanity check - test_ec_txt += "\nskipsteps = ['sanitycheck']\n" - write_file(test_ec, test_ec_txt) - with self.mocked_stdout_stderr(): - self.eb_main(args, do_build=True, raise_error=True) + def test_module_only_param(self): + """check use of module_only parameter""" + topdir = os.path.abspath(os.path.dirname(__file__)) + toy_ec = os.path.join(topdir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') - self.assertEqual(len(glob.glob(toy_mod_glob)), 1) + test_ec = os.path.join(self.test_prefix, 'test.eb') + test_ec_txt = read_file(toy_ec) + test_ec_txt += "\nmodule_only=True\n" + test_ec_txt += "\nskipsteps = ['sanitycheck']\n" # Software does not exist, so sanity check would fail + write_file(test_ec, test_ec_txt) - # check use of module_only parameter - remove_dir(os.path.join(self.test_installpath, 'modules', 'all', 'toy')) - remove_dir(os.path.join(self.test_installpath, 'software', 'toy', '0.0')) args = [ test_ec, '--rebuild', ] - test_ec_txt += "\nmodule_only = True\n" - write_file(test_ec, test_ec_txt) - with self.mocked_stdout_stderr(): - self.eb_main(args, do_build=True, raise_error=True) + self.mocked_main(args, do_build=True, raise_error=True) + toy_mod_glob = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '*') self.assertEqual(len(glob.glob(toy_mod_glob)), 1) # check that no software was installed @@ -388,6 +382,38 @@ def test_skip(self): easybuild_dir = os.path.join(installdir, 'easybuild') self.assertEqual(installdir_glob, [easybuild_dir]) + def test_skipsteps(self): + """Test skipping of steps using skipsteps.""" + # use toy-0.0.eb easyconfig file that comes with the tests + topdir = os.path.abspath(os.path.dirname(__file__)) + toy_ec = os.path.join(topdir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') + + # make sure that sanity check is *NOT* skipped + test_ec = os.path.join(self.test_prefix, 'test.eb') + test_ec_txt = read_file(toy_ec) + regex = re.compile(r"sanity_check_paths = \{(.|\n)*\}", re.M) + test_ec_txt = regex.sub("sanity_check_paths = {'files': ['bin/nosuchfile'], 'dirs': []}", test_ec_txt) + write_file(test_ec, test_ec_txt) + args = [ + test_ec, + '--rebuild', + ] + error_pattern = "Sanity check failed: no file found at 'bin/nosuchfile'" + self.assertErrorRegex(EasyBuildError, error_pattern, self.mocked_main, args, do_build=True, raise_error=True) + + # Verify a wrong step name is caught + test_ec_txt += "\nskipsteps = ['wrong-step-name']\n" + write_file(test_ec, test_ec_txt) + self.assertErrorRegex(EasyBuildError, 'wrong-step-name', self.eb_main, args, do_build=True, raise_error=True) + test_ec_txt += "\nskipsteps = ['source']\n" # Especially the old name -> Replaced by extract + write_file(test_ec, test_ec_txt) + self.assertErrorRegex(EasyBuildError, 'source', self.eb_main, args, do_build=True, raise_error=True) + + # check use of skipsteps to skip sanity check + test_ec_txt += "\nskipsteps = ['sanitycheck']\n" + write_file(test_ec, test_ec_txt) + self.mocked_main(args, do_build=True, raise_error=True) + def test_skip_test_step(self): """Test skipping testing the build (--skip-test-step).""" @@ -738,8 +764,8 @@ def test_avail_hooks(self): " post_fetch_hook", " pre_ready_hook", " post_ready_hook", - " pre_source_hook", - " post_source_hook", + " pre_extract_hook", + " post_extract_hook", " pre_patch_hook", " post_patch_hook", " pre_prepare_hook", @@ -1257,12 +1283,7 @@ def mocked_main(self, args, **kwargs): if not kwargs: kwargs = {'raise_error': True} - self.mock_stderr(True) - self.mock_stdout(True) - self.eb_main(args, **kwargs) - stderr, stdout = self.get_stderr(), self.get_stdout() - self.mock_stderr(False) - self.mock_stdout(False) + stdout, stderr = self._run_mock_eb(args, **kwargs) self.assertEqual(stderr, '') return stdout.strip() @@ -4443,14 +4464,14 @@ def _assert_regexs(self, regexs, txt, assert_true=True): self.assertFalse(regex.search(txt), "Pattern '%s' NOT found in: %s" % (regex.pattern, txt)) def _run_mock_eb(self, args, strip=False, **kwargs): - """Helper function to mock easybuild runs""" - self.mock_stdout(True) - self.mock_stderr(True) - self.eb_main(args, **kwargs) - stdout_txt = self.get_stdout() - stderr_txt = self.get_stderr() - self.mock_stdout(False) - self.mock_stderr(False) + """Helper function to mock easybuild runs + + Return (stdout, stderr) optionally stripped of whitespace at start/end + """ + with self.mocked_stdout_stderr() as (stdout, stderr): + self.eb_main(args, **kwargs) + stdout_txt = stdout.getvalue() + stderr_txt = stderr.getvalue() if strip: stdout_txt = stdout_txt.strip() stderr_txt = stderr_txt.strip() @@ -5458,6 +5479,10 @@ def test_stop(self): regex = re.compile(r"COMPLETED: Installation STOPPED successfully \(took .* secs?\)", re.M) self.assertTrue(regex.search(txt), "Pattern '%s' found in: %s" % (regex.pattern, txt)) + args = ['toy-0.0.eb', '--force', '--stop=source'] + _, stderr = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False, strip=True) + self.assertIn("option --stop: invalid choice", stderr) + def test_fetch(self): options = EasyBuildOptions(go_args=['--fetch']) @@ -6462,7 +6487,7 @@ def test_enforce_checksums(self): args = [ test_ec, - '--stop=source', + '--stop=fetch', '--enforce-checksums', ] From 65b61be1aa761c80a3b9903bcf19ce488683dc8b Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Sep 2024 09:25:35 +0200 Subject: [PATCH 3/5] Keep test path config when restoring environment in tests This fixes `test_toy_multi_deps` which restores the environment in the middle of the test and then misses the --sourcepath modification done at test setup. --- test/framework/utilities.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/framework/utilities.py b/test/framework/utilities.py index c20de50624..dac5cced2c 100644 --- a/test/framework/utilities.py +++ b/test/framework/utilities.py @@ -49,7 +49,7 @@ from easybuild.framework.easyblock import EasyBlock from easybuild.main import main from easybuild.tools import config -from easybuild.tools.config import GENERAL_CLASS, Singleton, module_classes, update_build_option +from easybuild.tools.config import GENERAL_CLASS, Singleton, module_classes from easybuild.tools.configobj import ConfigObj from easybuild.tools.environment import modify_env from easybuild.tools.filetools import copy_dir, mkdir, read_file, which @@ -106,8 +106,8 @@ def setUp(self): os.close(fd) self.cwd = os.getcwd() - # keep track of original environment to restore - self.orig_environ = copy.deepcopy(os.environ) + # keep track of original environment to restore after tests + self._initial_environ = copy.deepcopy(os.environ) # keep track of original environment/Python search path to restore self.orig_sys_path = sys.path[:] @@ -131,17 +131,19 @@ def setUp(self): if eb_path is not None: os.environ['EB_SCRIPT_PATH'] = eb_path + # disable progress bars when running the tests, + # since it messes with test suite progress output when test installations are performed + os.environ['EASYBUILD_DISABLE_SHOW_PROGRESS_BAR'] = '1' + + # Store the environment as setup (including the above paths) for tests to restore + self.orig_environ = copy.deepcopy(os.environ) + # make sure no deprecated behaviour is being triggered (unless intended by the test) self.orig_current_version = eb_build_log.CURRENT_VERSION self.disallow_deprecated_behaviour() init_config() - # disable progress bars when running the tests, - # since it messes with test suite progress output when test installations are performed - os.environ['EASYBUILD_DISABLE_SHOW_PROGRESS_BAR'] = '1' - update_build_option('show_progress_bar', False) - import easybuild # try to import easybuild.easyblocks(.generic) packages # it's OK if it fails here, but important to import first before fiddling with sys.path From f41ef2864c3bdc59fd2154a353ee47517cbd3eba Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 4 Dec 2024 15:09:16 +0100 Subject: [PATCH 4/5] trivial code style changes in EasyConfig.validate + tweaks in tests for skipsteps and --stop --- easybuild/framework/easyconfig/easyconfig.py | 11 ++++++----- test/framework/options.py | 18 ++++++++++++++---- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 85a7d5d9ae..ff6b307a6e 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -72,7 +72,7 @@ from easybuild.tools.filetools import convert_name, copy_file, create_index, decode_class_name, encode_class_name from easybuild.tools.filetools import find_backup_name_candidate, find_easyconfigs, load_index from easybuild.tools.filetools import read_file, write_file -from easybuild.tools.hooks import STEP_NAMES, PARSE, load_hooks, run_hook +from easybuild.tools.hooks import PARSE, STEP_NAMES, load_hooks, run_hook from easybuild.tools.module_naming_scheme.mns import DEVEL_MODULE_SUFFIX from easybuild.tools.module_naming_scheme.utilities import avail_module_naming_schemes, det_full_ec_version from easybuild.tools.module_naming_scheme.utilities import det_hidden_modname, is_valid_module_name @@ -866,13 +866,14 @@ def validate(self, check_osdeps=True): type(skipsteps), skipsteps) unknown_step_names = [step for step in skipsteps if step not in STEP_NAMES] if unknown_step_names: - error_lines = ["Found one or more unknown step names in skipsteps:"] + error_lines = ["Found one or more unknown step names in 'skipsteps' easyconfig parameter:"] for step in unknown_step_names: - error_lines.append("* %s" % step) - # try to find close match, may be just a typo in the hook name + error_line = "* %s" % step + # try to find close match, may be just a typo in the step name close_matches = difflib.get_close_matches(step, STEP_NAMES, 2, 0.8) if close_matches: - error_lines[-1] += " (did you mean %s?)" % ', or '.join("'%s'" % s for s in close_matches) + error_line += " (did you mean %s?)" % ', or '.join("'%s'" % s for s in close_matches) + error_lines.append(error_line) raise EasyBuildError('\n'.join(error_lines)) self.log.info("Checking build option lists") diff --git a/test/framework/options.py b/test/framework/options.py index 756dc774ec..4671cbb809 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -404,16 +404,24 @@ def test_skipsteps(self): # Verify a wrong step name is caught test_ec_txt += "\nskipsteps = ['wrong-step-name']\n" write_file(test_ec, test_ec_txt) - self.assertErrorRegex(EasyBuildError, 'wrong-step-name', self.eb_main, args, do_build=True, raise_error=True) - test_ec_txt += "\nskipsteps = ['source']\n" # Especially the old name -> Replaced by extract + error_pattern = "Found one or more unknown step names in 'skipsteps' easyconfig parameter:\n" + error_pattern += r"\* wrong-step-name" + self.assertErrorRegex(EasyBuildError, error_pattern, self.eb_main, args, do_build=True, raise_error=True) + # 'source' step was renamed to 'extract' in EasyBuild 5.0, + # see https://github.com/easybuilders/easybuild-framework/pull/4629 + test_ec_txt += "\nskipsteps = ['source']\n" write_file(test_ec, test_ec_txt) - self.assertErrorRegex(EasyBuildError, 'source', self.eb_main, args, do_build=True, raise_error=True) + error_pattern = error_pattern.replace('wrong-step-name', 'source') + self.assertErrorRegex(EasyBuildError, error_pattern, self.eb_main, args, do_build=True, raise_error=True) # check use of skipsteps to skip sanity check test_ec_txt += "\nskipsteps = ['sanitycheck']\n" write_file(test_ec, test_ec_txt) self.mocked_main(args, do_build=True, raise_error=True) + toy_mod_glob = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '*') + self.assertEqual(len(glob.glob(toy_mod_glob)), 1) + def test_skip_test_step(self): """Test skipping testing the build (--skip-test-step).""" @@ -5509,9 +5517,11 @@ def test_stop(self): regex = re.compile(r"COMPLETED: Installation STOPPED successfully \(took .* secs?\)", re.M) self.assertTrue(regex.search(txt), "Pattern '%s' found in: %s" % (regex.pattern, txt)) + # 'source' step was renamed to 'extract' in EasyBuild 5.0, + # see https://github.com/easybuilders/easybuild-framework/pull/4629 args = ['toy-0.0.eb', '--force', '--stop=source'] _, stderr = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False, strip=True) - self.assertIn("option --stop: invalid choice", stderr) + self.assertIn("option --stop: invalid choice: 'source' (choose from", stderr) def test_fetch(self): options = EasyBuildOptions(go_args=['--fetch']) From a41de82fb63c92aa247022a62929c1ea86e4a48e Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 4 Dec 2024 16:43:33 +0100 Subject: [PATCH 5/5] suggest 'extract' as as correct step name instead of 'source' when used in skipsteps --- easybuild/framework/easyconfig/easyconfig.py | 6 +++++- easybuild/tools/hooks.py | 2 +- test/framework/options.py | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index ff6b307a6e..3f6181df18 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -72,7 +72,7 @@ from easybuild.tools.filetools import convert_name, copy_file, create_index, decode_class_name, encode_class_name from easybuild.tools.filetools import find_backup_name_candidate, find_easyconfigs, load_index from easybuild.tools.filetools import read_file, write_file -from easybuild.tools.hooks import PARSE, STEP_NAMES, load_hooks, run_hook +from easybuild.tools.hooks import PARSE, EXTRACT_STEP, STEP_NAMES, load_hooks, run_hook from easybuild.tools.module_naming_scheme.mns import DEVEL_MODULE_SUFFIX from easybuild.tools.module_naming_scheme.utilities import avail_module_naming_schemes, det_full_ec_version from easybuild.tools.module_naming_scheme.utilities import det_hidden_modname, is_valid_module_name @@ -871,6 +871,10 @@ def validate(self, check_osdeps=True): error_line = "* %s" % step # try to find close match, may be just a typo in the step name close_matches = difflib.get_close_matches(step, STEP_NAMES, 2, 0.8) + # 'source' step was renamed to 'extract' in EasyBuild 5.0, see provide a useful suggestion in that case; + # see https://github.com/easybuilders/easybuild-framework/pull/4629 + if not close_matches and step == 'source': + close_matches.append(EXTRACT_STEP) if close_matches: error_line += " (did you mean %s?)" % ', or '.join("'%s'" % s for s in close_matches) error_lines.append(error_line) diff --git a/easybuild/tools/hooks.py b/easybuild/tools/hooks.py index aaeabfe314..8f50bcc75b 100644 --- a/easybuild/tools/hooks.py +++ b/easybuild/tools/hooks.py @@ -44,6 +44,7 @@ CLEANUP_STEP = 'cleanup' CONFIGURE_STEP = 'configure' EXTENSIONS_STEP = 'extensions' +EXTRACT_STEP = 'extract' FETCH_STEP = 'fetch' INSTALL_STEP = 'install' MODULE_STEP = 'module' @@ -55,7 +56,6 @@ PREPARE_STEP = 'prepare' READY_STEP = 'ready' SANITYCHECK_STEP = 'sanitycheck' -EXTRACT_STEP = 'extract' TEST_STEP = 'test' TESTCASES_STEP = 'testcases' diff --git a/test/framework/options.py b/test/framework/options.py index 4671cbb809..342c11e83e 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -411,7 +411,7 @@ def test_skipsteps(self): # see https://github.com/easybuilders/easybuild-framework/pull/4629 test_ec_txt += "\nskipsteps = ['source']\n" write_file(test_ec, test_ec_txt) - error_pattern = error_pattern.replace('wrong-step-name', 'source') + error_pattern = error_pattern.replace('wrong-step-name', r"source \(did you mean 'extract'\?\)") self.assertErrorRegex(EasyBuildError, error_pattern, self.eb_main, args, do_build=True, raise_error=True) # check use of skipsteps to skip sanity check