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.

Author: Flamefire

easybuild/framework/easyblock.py

@@ -85,10 +85,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
@@ -4051,7 +4052,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)

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
@@ -858,9 +858,21 @@ 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()

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]
 

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),
         })
 

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"

test/framework/options.py

@@ -296,7 +296,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')
@@ -335,13 +334,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):
@@ -360,29 +357,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
@@ -391,6 +385,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)."""
 
@@ -717,8 +743,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",
@@ -1236,12 +1262,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()
 
@@ -4422,14 +4443,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()
@@ -5437,6 +5458,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'])
 
@@ -6446,7 +6471,7 @@ def test_enforce_checksums(self):
 
         args = [
             test_ec,
-            '--stop=source',
+            '--stop=fetch',
             '--enforce-checksums',
         ]