From cabfe725e313e2f16fe13447e8d6a6da3cf7e57c 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 | 8 ++-- setup.cfg | 3 +- test/framework/easyconfig.py | 91 ++++++++++++++++++------------------ 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 2982c5eb2e..d9535409a5 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -52,7 +52,7 @@ from easybuild.tools.py2vs3 import ascii_letters, create_base_metaclass, string_type try: - import rich # noqa + import rich # noqa # pylint:disable=unused-import HAVE_RICH = True except ImportError: HAVE_RICH = False @@ -603,12 +603,12 @@ def init_build_options(build_options=None, cmdline_options=None): # seed in defaults to make sure all build options are defined, and that build_option() doesn't fail on valid keys bo = {} for build_options_by_default in [BUILD_OPTIONS_CMDLINE, BUILD_OPTIONS_OTHER]: - for default in build_options_by_default: + for default, default_opts in build_options_by_default.items(): if default == EMPTY_LIST: - for opt in build_options_by_default[default]: + for opt in default_opts: bo[opt] = [] else: - bo.update({opt: default for opt in build_options_by_default[default]}) + bo.update({opt: default for opt in default_opts}) bo.update(active_build_options) # BuildOptions is a singleton, so any future calls to BuildOptions will yield the same instance 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 09801bde50..27f339e257 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -84,10 +84,10 @@ from test.framework.utilities import find_full_path try: - import pycodestyle # noqa + import pycodestyle # noqa # pylint:disable=unused-import except ImportError: try: - import pep8 # noqa + import pep8 # noqa # pylint:disable=unused-import except ImportError: pass @@ -379,7 +379,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) @@ -2225,8 +2225,8 @@ def test_external_dependencies_templates(self): 'pyshortver': '3.6', 'pyver': '3.6.5', } - for key in expected_template_values: - self.assertEqual(ec.template_values[key], expected_template_values[key]) + for key, expected_value in expected_template_values.items(): + self.assertEqual(ec.template_values[key], expected_value) self.assertEqual(ec['versionsuffix'], '-Python-3.6.5-Perl-5.30') @@ -2302,8 +2302,8 @@ def test_quote_str(self): 'foo\\bar': '"foo\\bar"', } - for t in teststrings: - self.assertEqual(quote_str(t), teststrings[t]) + for t, expected in teststrings.items(): + self.assertEqual(quote_str(t), expected) # test escape_newline self.assertEqual(quote_str("foo\nbar", escape_newline=False), '"foo\nbar"') @@ -2574,13 +2574,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""" @@ -3072,46 +3073,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 = { @@ -3774,7 +3775,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 @@ -4989,8 +4990,8 @@ def test_get_cuda_cc_template_value(self): update_build_option('cuda_compute_capabilities', ['6.5', '7.0']) ec = EasyConfig(self.eb_file) - for key in cuda_template_values: - self.assertEqual(ec.get_cuda_cc_template_value(key), cuda_template_values[key]) + for key, expected in cuda_template_values.items(): + self.assertEqual(ec.get_cuda_cc_template_value(key), expected) update_build_option('cuda_compute_capabilities', None) ec = EasyConfig(self.eb_file) @@ -5002,8 +5003,8 @@ def test_get_cuda_cc_template_value(self): self.prep() ec = EasyConfig(self.eb_file) - for key in cuda_template_values: - self.assertEqual(ec.get_cuda_cc_template_value(key), cuda_template_values[key]) + for key, expected in cuda_template_values.items(): + self.assertEqual(ec.get_cuda_cc_template_value(key), expected) def test_count_files(self): """Tests for EasyConfig.count_files method.""" From 538d6665cd17a965ac3bc62e51f198bd32ede636 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 7c1b5acaba..eae570f179 100644 --- a/easybuild/framework/easyconfig/default.py +++ b/easybuild/framework/easyconfig/default.py @@ -183,7 +183,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 944e3ede1121591552ddeb96aab8341b8619fd4e 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 c442b7f782..b6a624d5a5 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -1013,7 +1013,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 c7ec26d6c1198c878ac96526bd8c0db091bd289e 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 | 31 ++++++++++++------------ test/framework/easyconfig.py | 41 +++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 21a85f21a0..8cf23206a1 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -2792,8 +2792,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) @@ -2846,8 +2848,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) @@ -2906,21 +2911,17 @@ def extensions_step(self, fetch=False, install=True): if install: self.log.info("Installing extensions") - # we really need a default class - if not self.cfg['exts_defaultclass'] and fake_mod_data: - self.clean_up_fake_module(fake_mod_data) - raise EasyBuildError("ERROR: No default extension class set for %s", self.name) - - self.init_ext_instances() - - if self.skip: - self.skip_extensions() + try: + self.init_ext_instances() - self.install_extensions(install=install) + if self.skip: + self.skip_extensions() - # cleanup (unload fake module, remove fake module dir) - if fake_mod_data: - self.clean_up_fake_module(fake_mod_data) + self.install_extensions(install=install) + finally: + # cleanup (unload fake module, remove fake module dir) + if fake_mod_data: + self.clean_up_fake_module(fake_mod_data) stop_progress_bar(PROGRESS_BAR_EXTENSIONS, visible=False) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 27f339e257..60abd7fa82 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -527,6 +527,46 @@ def test_exts_list(self): regex = re.compile('EBEXTSLISTPI.*ext1-1.0,ext2-2.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.""" @@ -549,7 +589,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 = [',