From 697ccf53d4cde99212136d1dbdbb7e969c1c6c9d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 7 Mar 2025 15:21:14 +0100 Subject: [PATCH 1/4] Handle some linter issues - Some places could use `dict.items`. - Pylint supressions added - Ignore G002 (Logging statements should not use % formatting for their first argument) - Early return for skipped tests to avoid indent --- easybuild/tools/config.py | 2 +- setup.cfg | 3 +- test/framework/easyconfig.py | 73 ++++++++++++++++++------------------ 3 files changed, 40 insertions(+), 38 deletions(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 071c01421f..dfec325d52 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -53,7 +53,7 @@ from easybuild.tools.build_log import EasyBuildError, EasyBuildExit try: - import rich # noqa + import rich # noqa # pylint:disable=unused-import HAVE_RICH = True except ImportError: HAVE_RICH = False diff --git a/setup.cfg b/setup.cfg index 430d761b59..d5aab88e40 100644 --- a/setup.cfg +++ b/setup.cfg @@ -19,4 +19,5 @@ builtins = # ignore "Black would make changes" produced by flake8-black # see also https://github.com/houndci/hound/issues/1769 -extend-ignore = BLK100 +# Also: "Logging statements should not use % formatting for their first argument" +extend-ignore = BLK100, G002 diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 7e3eebbfdd..7ffde4a0c1 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -83,7 +83,7 @@ from test.framework.utilities import find_full_path try: - import pycodestyle # noqa + import pycodestyle # noqa # pylint:disable=unused-import except ImportError: pass @@ -386,7 +386,7 @@ def test_false_dep_version(self): # only non-POWER arch, dependency is retained for arch in (AARCH64, X86_64): - st.get_cpu_architecture = lambda: arch + st.get_cpu_architecture = lambda: arch # pylint: disable=cell-var-from-loop eb = EasyConfig(self.eb_file) deps = eb.dependencies() self.assertEqual(len(deps), 1) @@ -2743,13 +2743,14 @@ def test_dump_order(self): def test_dump_autopep8(self): """Test dump() with autopep8 usage enabled (only if autopep8 is available).""" try: - import autopep8 # noqa - os.environ['EASYBUILD_DUMP_AUTOPEP8'] = '1' - init_config() - self.test_dump() - del os.environ['EASYBUILD_DUMP_AUTOPEP8'] + import autopep8 # noqa # pylint:disable=unused-import except ImportError: print("Skipping test_dump_autopep8, since autopep8 is not available") + return + os.environ['EASYBUILD_DUMP_AUTOPEP8'] = '1' + init_config() + self.test_dump() + del os.environ['EASYBUILD_DUMP_AUTOPEP8'] def test_dump_extra(self): """Test EasyConfig's dump() method for files containing extra values""" @@ -3241,46 +3242,46 @@ def test_to_template_str(self): def test_dep_graph(self): """Test for dep_graph.""" try: - import pygraph # noqa - - test_easyconfigs = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs') - build_options = { - 'external_modules_metadata': ConfigObj(), - 'valid_module_classes': module_classes(), - 'robot_path': [test_easyconfigs], - 'silent': True, - } - init_config(build_options=build_options) + import pygraph # noqa # pylint:disable=unused-import + except ImportError: + print("Skipping test_dep_graph, since pygraph is not available") + return - ec_file = os.path.join(test_easyconfigs, 't', 'toy', 'toy-0.0-deps.eb') - ec_files = [(ec_file, False)] - ecs, _ = parse_easyconfigs(ec_files) + test_easyconfigs = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs') + build_options = { + 'external_modules_metadata': ConfigObj(), + 'valid_module_classes': module_classes(), + 'robot_path': [test_easyconfigs], + 'silent': True, + } + init_config(build_options=build_options) - dot_file = os.path.join(self.test_prefix, 'test.dot') - ordered_ecs = resolve_dependencies(ecs, self.modtool, retain_all_deps=True) - dep_graph(dot_file, ordered_ecs) + ec_file = os.path.join(test_easyconfigs, 't', 'toy', 'toy-0.0-deps.eb') + ec_files = [(ec_file, False)] + ecs, _ = parse_easyconfigs(ec_files) - # hard check for expect .dot file contents - # 3 nodes should be there: 'GCC/6.4.0-2.28 (EXT)', 'toy', and 'intel/2018a' - # and 2 edges: 'toy -> intel' and 'toy -> "GCC/6.4.0-2.28 (EXT)"' - dottxt = read_file(dot_file) + dot_file = os.path.join(self.test_prefix, 'test.dot') + ordered_ecs = resolve_dependencies(ecs, self.modtool, retain_all_deps=True) + dep_graph(dot_file, ordered_ecs) - self.assertTrue(dottxt.startswith('digraph graphname {')) + # hard check for expect .dot file contents + # 3 nodes should be there: 'GCC/6.4.0-2.28 (EXT)', 'toy', and 'intel/2018a' + # and 2 edges: 'toy -> intel' and 'toy -> "GCC/6.4.0-2.28 (EXT)"' + dottxt = read_file(dot_file) - # compare sorted output, since order of lines can change - ordered_dottxt = '\n'.join(sorted(dottxt.split('\n'))) - ordered_expected = '\n'.join(sorted(EXPECTED_DOTTXT_TOY_DEPS.split('\n'))) - self.assertEqual(ordered_dottxt, ordered_expected) + self.assertTrue(dottxt.startswith('digraph graphname {')) - except ImportError: - print("Skipping test_dep_graph, since pygraph is not available") + # compare sorted output, since order of lines can change + ordered_dottxt = '\n'.join(sorted(dottxt.split('\n'))) + ordered_expected = '\n'.join(sorted(EXPECTED_DOTTXT_TOY_DEPS.split('\n'))) + self.assertEqual(ordered_dottxt, ordered_expected) def test_dep_graph_multi_deps(self): """ Test for dep_graph using easyconfig that uses multi_deps. """ try: - import pygraph # noqa + import pygraph # noqa # pylint:disable=unused-import test_easyconfigs = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs') build_options = { @@ -3942,7 +3943,7 @@ def test_det_subtoolchain_version(self): """Test det_subtoolchain_version function""" _, all_tc_classes = search_toolchain('') subtoolchains = {tc_class.NAME: getattr(tc_class, 'SUBTOOLCHAIN', None) for tc_class in all_tc_classes} - optional_toolchains = set(tc_class.NAME for tc_class in all_tc_classes if getattr(tc_class, 'OPTIONAL', False)) + optional_toolchains = {tc_class.NAME for tc_class in all_tc_classes if getattr(tc_class, 'OPTIONAL', False)} current_tc = {'name': 'fosscuda', 'version': '2018a'} # missing gompic and double golfc should both give exceptions From ebc0aab142cd2955a2d699f191e879c604da9af0 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 7 Mar 2025 15:28:48 +0100 Subject: [PATCH 2/4] Fix description of exts_defaultclass --- easybuild/framework/easyconfig/default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/default.py b/easybuild/framework/easyconfig/default.py index 323312e5cc..81c2e2a4ef 100644 --- a/easybuild/framework/easyconfig/default.py +++ b/easybuild/framework/easyconfig/default.py @@ -188,7 +188,7 @@ # EXTENSIONS easyconfig parameters 'exts_download_dep_fail': [False, "Fail if downloaded dependencies are detected for extensions", EXTENSIONS], 'exts_classmap': [{}, "Map of extension name to class for handling build and installation.", EXTENSIONS], - 'exts_defaultclass': [None, "List of module for and name of the default extension class", EXTENSIONS], + 'exts_defaultclass': [None, "Name of default easyblock for extensions", EXTENSIONS], 'exts_default_options': [{}, "List of default options for extensions", EXTENSIONS], 'exts_filter': [None, ("Extension filter details: template for cmd and input to cmd " "(templates for ext_name, ext_version and src)."), EXTENSIONS], From 895f5113461f06e331027e1d52e4df830d8b45f5 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 7 Mar 2025 15:29:12 +0100 Subject: [PATCH 3/4] Remove redundant assertion --- test/framework/easyblock.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index ec3eab2200..8d06192dd8 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -1298,7 +1298,6 @@ def test_extensions_step(self): # test for proper error message without the exts_defaultclass set eb = EasyBlock(EasyConfig(self.eb_file)) eb.installdir = config.install_path() - self.assertRaises(EasyBuildError, eb.extensions_step, fetch=True) self.assertErrorRegex(EasyBuildError, "No default extension class set", eb.extensions_step, fetch=True) # test if everything works fine if set From 90b378959fb970de8ab912cd80c37ddb050d61f5 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 7 Mar 2025 15:29:14 +0100 Subject: [PATCH 4/4] Make specifying `exts_defaultclass` optional It is NOT required when all extensions have either custom easyblocks or an `easyblock` key in their option dict. In those cases `exts_defaultclass` is redundant making it harder to write the EasyConfig. Check for a missing easyblock when actually using it so the error will be just a bit later but still (almost) the same. --- easybuild/framework/easyblock.py | 13 +++++----- test/framework/easyconfig.py | 41 +++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index c714443d1f..38a23c05fb 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -3086,8 +3086,10 @@ def init_ext_instances(self): # proper way: derive module path from specified class name default_class = exts_defaultclass default_class_modpath = get_module_path(default_class, generic=True) + elif exts_defaultclass is None: + default_class = default_class_modpath = None else: - error_msg = "Improper default extension class specification, should be string: %s (%s)" + error_msg = "Improper default extension class specification, should be string or None: %s (%s)" raise EasyBuildError(error_msg, exts_defaultclass, type(exts_defaultclass)) exts_cnt = len(self.exts) @@ -3139,8 +3141,11 @@ def init_ext_instances(self): "for extension %s: %s", class_name, mod_path, ext_name, err) - # fallback attempt: use default class + # fallback attempt: use default class if any if inst is None: + if not default_class: + raise EasyBuildError("ERROR: No default extension class set for %s and no explicit or custom " + "easyblock found for extension %s", self.name, ext_name) try: cls = get_class_for(default_class_modpath, default_class) self.log.debug("Obtained class %s for installing extension %s", cls, ext_name) @@ -3180,10 +3185,6 @@ def extensions_step(self, fetch=False, install=True): self.prepare_for_extensions() - # we really need a default class - if not self.cfg['exts_defaultclass'] and install: - raise EasyBuildError("ERROR: No default extension class set for %s", self.name) - if fetch: self.update_exts_progress_bar("fetching extension sources/patches") self.exts = self.collect_exts_file_info(fetch_files=True) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 7ffde4a0c1..35eca0228e 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -540,6 +540,46 @@ def test_exts_list(self): regex = re.compile('EBEXTSLISTPI.*"ext1-1.0,ext2-2.0,ext-PI-3.14,ext-pi-3.0') self.assertTrue(regex.search(modtxt), "Pattern '%s' found in: %s" % (regex.pattern, modtxt)) + def test_extensions_default_class(self): + """Test that exts_defaultclass doesn't need to be specified if explicit one is present.""" + + init_config(build_options={'silent': True}) + + self.contents = textwrap.dedent(""" + easyblock = "ConfigureMake" + name = "PI" + version = "3.14" + homepage = "http://example.com" + description = "test easyconfig" + toolchain = SYSTEM + exts_list = [ + ("toy", "0.0"), # Custom block by name + ("bar", "0.0", { # Explicit + 'easyblock': 'EB_toy', + 'sources': ['toy-%(version)s.tar.gz'], + }), + ] + """) + self.prep() + # Ensure source is found + toy_tar_gz = os.path.join(self.test_sourcepath, 'toy', 'toy-0.0.tar.gz') + copy_file(toy_tar_gz, self.test_prefix) + os.environ['EASYBUILD_SOURCEPATH'] = self.test_prefix + init_config(build_options={'silent': True}) + + ec = EasyConfig(self.eb_file) + eb = EasyBlock(ec) + eb.fetch_step() + with self.mocked_stdout_stderr(): + eb.extensions_step() + + pi_installdir = os.path.join(self.test_installpath, 'software', 'PI', '3.14') + + # check whether files expected to be installed for both extensions are in place + self.assertExists(os.path.join(pi_installdir, 'bin', 'toy')) + self.assertExists(os.path.join(pi_installdir, 'lib', 'libtoy.a')) + self.assertExists(os.path.join(pi_installdir, 'lib', 'libbar.a')) + def test_extensions_templates(self): """Test whether templates used in exts_list are resolved properly.""" @@ -562,7 +602,6 @@ def test_extensions_templates(self): 'description = "test easyconfig"', 'toolchain = SYSTEM', 'dependencies = [("Python", "3.6.6")]', - 'exts_defaultclass = "EB_Toy"', # bogus, but useful to check whether this get resolved 'exts_default_options = {"source_urls": [PYPI_SOURCE]}', 'exts_list = [',