From d1d5c4b01c4ab3e5830f16314b1540bdb6fcc95c Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 21 Oct 2022 22:51:50 +0200 Subject: [PATCH 1/5] Deprecate `variables` in favour of `env_vars` The deprecation is valid everywhere: from the configuration to the tests. --- reframe/core/config.py | 39 ++++++++++++++++++++++++++++++++ reframe/core/environments.py | 40 ++++++++++++++++++++++----------- reframe/core/logging.py | 13 ++++++----- reframe/core/pipeline.py | 20 +++++++++++++---- reframe/core/runtime.py | 4 ++-- reframe/core/systems.py | 19 ++++++++-------- reframe/frontend/autodetect.py | 4 ++-- reframe/frontend/cli.py | 5 +++-- reframe/frontend/runreport.py | 5 +++-- reframe/frontend/statistics.py | 10 +++++++-- reframe/schemas/config.json | 12 +++++++++- unittests/resources/settings.py | 4 ++-- unittests/test_config.py | 40 ++++++++++++++++++++++++++++----- unittests/test_environments.py | 24 ++++++++++---------- unittests/test_pipeline.py | 20 +++++++++++++++-- 15 files changed, 195 insertions(+), 64 deletions(-) diff --git a/reframe/core/config.py b/reframe/core/config.py index 2ae93f8f15..75cd0afe75 100644 --- a/reframe/core/config.py +++ b/reframe/core/config.py @@ -347,6 +347,45 @@ def validate(self): partition_names.add(partname) + def _warn_variables(config, opt_path): + opt_path = '/'.join(opt_path + ['variables']) + if 'env_vars' in config and 'variables' in config: + getlogger().warning( + f"configuration option {opt_path!r}: " + f"both 'env_vars' and 'variables' are defined; " + f"'variables' will be ignored" + ) + elif 'variables' in config: + getlogger().warning( + f"configuration option {opt_path!r}: " + f"'variables' is deprecated; please use 'env_vars' instead" + ) + config['env_vars'] = config['variables'] + + # Warn about the deprecated `variables` and convert them internally to + # `env_vars` + for system in self._site_config['systems']: + sysname = system['name'] + opt_path = ['systems', f'@{sysname}'] + _warn_variables(system, opt_path) + for part in system['partitions']: + partname = part['name'] + opt_path += ['partitions', f'@{partname}'] + _warn_variables(part, opt_path) + for i, cp in enumerate(part.get('container_platforms', [])): + opt_path += ['container_platforms', str(i)] + _warn_variables(cp, opt_path) + opt_path.pop() + opt_path.pop() + + opt_path.pop() + opt_path.pop() + + for env in self._site_config['environments']: + envname = env['name'] + opt_path = ['environments', f'@{envname}'] + _warn_variables(env, opt_path) + def select_subconfig(self, system_fullname=None, ignore_resolve_errors=False): # First look for the current subconfig in the cache; if not found, diff --git a/reframe/core/environments.py b/reframe/core/environments.py index 3df845a2e2..4c663145b6 100644 --- a/reframe/core/environments.py +++ b/reframe/core/environments.py @@ -10,6 +10,7 @@ import reframe.utility as util import reframe.utility.jsonext as jsonext import reframe.utility.typecheck as typ +from reframe.core.warnings import user_deprecation_warning def normalize_module_list(modules): @@ -37,14 +38,14 @@ class Environment(jsonext.JSONSerializable): Users may not create :class:`Environment` objects directly. ''' - def __init__(self, name, modules=None, variables=None, + def __init__(self, name, modules=None, env_vars=None, extras=None, features=None): modules = modules or [] - variables = variables or [] + env_vars = env_vars or [] self._name = name self._modules = normalize_module_list(modules) self._module_names = [m['name'] for m in self._modules] - self._variables = collections.OrderedDict(variables) + self._env_vars = collections.OrderedDict(env_vars) self._extras = extras or {} self._features = features or [] @@ -85,12 +86,25 @@ def modules_detailed(self): return util.SequenceView(self._modules) @property - def variables(self): + def env_vars(self): '''The environment variables associated with this environment. :type: :class:`OrderedDict[str, str]` + + .. versionadded:: 4.0.0 + ''' + return util.MappingView(self._env_vars) + + @property + def variables(self): + '''The environment variables associated with this environment. + + .. deprecated:: 4.0.0 + Please :attr:`env_vars` instead. ''' - return util.MappingView(self._variables) + user_deprecation_warning("the 'variables' attribute is deprecated; " + "please use the 'env_vars' instead") + return util.MappingView(self._env_vars) @property def extras(self): @@ -119,7 +133,7 @@ def __eq__(self, other): return (self.name == other.name and set(self.modules) == set(other.modules) and - self.variables == other.variables) + self.env_vars == other.env_vars) def __str__(self): return self.name @@ -128,7 +142,7 @@ def __repr__(self): return (f'{type(self).__name__}(' f'name={self._name!r}, ' f'modules={self._modules!r}, ' - f'variables={list(self._variables.items())!r}, ' + f'env_vars={list(self._env_vars.items())!r}, ' f'extras={self._extras!r}, features={self._features!r})') @@ -141,15 +155,15 @@ def __init__(self, name='env_snapshot'): def restore(self): '''Restore this environment snapshot.''' os.environ.clear() - os.environ.update(self._variables) + os.environ.update(self._env_vars) def __eq__(self, other): if not isinstance(other, Environment): return NotImplemented - # Order of variables is not important when comparing snapshots - for k, v in self.variables.items(): - if other.variables[k] != v: + # Order of env. variables is not important when comparing snapshots + for k, v in self.env_vars.items(): + if other.env_vars[k] != v: return False return self.name == other.name @@ -185,7 +199,7 @@ class ProgEnvironment(Environment): def __init__(self, name, modules=None, - variables=None, + env_vars=None, extras=None, features=None, cc='cc', @@ -198,7 +212,7 @@ def __init__(self, fflags=None, ldflags=None, **kwargs): - super().__init__(name, modules, variables, extras, features) + super().__init__(name, modules, env_vars, extras, features) self._cc = cc self._cxx = cxx self._ftn = ftn diff --git a/reframe/core/logging.py b/reframe/core/logging.py index 12057d3b35..60af63c4ea 100644 --- a/reframe/core/logging.py +++ b/reframe/core/logging.py @@ -20,6 +20,7 @@ import reframe.utility.jsonext as jsonext import reframe.utility.osext as osext from reframe.core.exceptions import ConfigError, LoggingError +from reframe.core.warnings import suppress_deprecations from reframe.utility.profile import TimeProfiler @@ -396,8 +397,8 @@ def __init__(self, url, extras=None, ignore_keys=None): def _record_to_json(self, record): def _can_send(key): return not ( - key.startswith('_') or key in HTTPJSONHandler.LOG_ATTRS - or (self._ignore_keys and key in self._ignore_keys) + key.startswith('_') or key in HTTPJSONHandler.LOG_ATTRS or + (self._ignore_keys and key in self._ignore_keys) ) json_record = { @@ -577,9 +578,11 @@ def _update_check_extras(self): for attr, alt_name in check_type.loggable_attrs(): extra_name = alt_name or attr - # In case of AttributeError, i.e., the variable is undefined, we - # set the value to None - val = getattr(self.check, attr, None) + with suppress_deprecations(): + # In case of AttributeError, i.e., the variable is undefined, + # we set the value to None + val = getattr(self.check, attr, None) + if attr in check_type.raw_params: # Attribute is parameter, so format it val = check_type.raw_params[attr].format(val) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 80b3fdd727..133ccc1c9d 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -783,9 +783,21 @@ def pipeline_hooks(cls): #: #: These variables will be set during the :func:`setup` phase. #: - #: :type: :class:`Dict[str, str]` + #: :type: :class:`Dict[str, object]` #: :default: ``{}`` - variables = variable(typ.Dict[str, str], value={}, loggable=True) + #: + #: .. versionadded:: 4.0.0 + env_vars = variable(typ.Dict[str, object], value={}, loggable=True) + + #: Environment variables to be set before running this test. + #: + #: This is an alias of :attr:`env_vars`. + #: + #: .. deprecated:: 4.0.0 + #: Please use :attr:`env_vars` instead. + variables = deprecate(variable(alias=env_vars, loggable=True), + f"the use of 'variables' is deprecated; " + f"please use 'env_vars' instead") #: Time limit for this test. #: @@ -1746,7 +1758,7 @@ def compile(self): self.build_system.executable = self.executable user_environ = Environment(self.unique_name, - self.modules, self.variables.items()) + self.modules, self.env_vars.items()) environs = [self._current_partition.local_env, self._current_environ, user_environ, self._cdt_environ] self._build_job.time_limit = ( @@ -1880,7 +1892,7 @@ def _get_cp_env(): *self.postrun_cmds ] user_environ = Environment(self.unique_name, - self.modules, self.variables.items()) + self.modules, self.env_vars.items()) environs = [ self._current_partition.local_env, self._current_environ, diff --git a/reframe/core/runtime.py b/reframe/core/runtime.py index 1c34455d3c..893c0e39c4 100644 --- a/reframe/core/runtime.py +++ b/reframe/core/runtime.py @@ -235,7 +235,7 @@ def _load_cmds_tracked(**module): else: commands += modules_system.emit_load_commands(**mod) - for k, v in env.variables.items(): + for k, v in env.env_vars.items(): os.environ[k] = osext.expandvars(v) commands.append(f'export {k}={v}') @@ -264,7 +264,7 @@ def is_env_loaded(environ): is_module_loaded = runtime().modules_system.is_module_loaded return (all(map(is_module_loaded, environ.modules)) and all(os.environ.get(k, None) == osext.expandvars(v) - for k, v in environ.variables.items())) + for k, v in environ.env_vars.items())) def _is_valid_part(part, valid_systems): diff --git a/reframe/core/systems.py b/reframe/core/systems.py index d8d25bd8e4..c07e04bca8 100644 --- a/reframe/core/systems.py +++ b/reframe/core/systems.py @@ -432,13 +432,12 @@ def json(self): { 'type': ctype, 'modules': [m for m in cpenv.modules], - 'variables': [[n, v] for n, v in cpenv.variables.items()] + 'env_vars': [[n, v] for n, v in cpenv.env_vars.items()] } for ctype, cpenv in self._container_environs.items() ], 'modules': [m for m in self._local_env.modules], - 'variables': [[n, v] - for n, v in self._local_env.variables.items()], + 'env_vars': [[n, v] for n, v in self._local_env.env_vars.items()], 'environs': [e.name for e in self._environs], 'max_jobs': self._max_jobs, 'resources': [ @@ -513,8 +512,8 @@ def create(cls, site_config): modules=site_config.get( f'{partid}/container_platforms/{i}/modules' ), - variables=site_config.get( - f'{partid}/container_platforms/{i}/variables' + env_vars=site_config.get( + f'{partid}/container_platforms/{i}/env_vars' ) ) if p.get('default', None): @@ -529,7 +528,7 @@ def create(cls, site_config): ProgEnvironment( name=e, modules=site_config.get(f'environments/@{e}/modules'), - variables=site_config.get(f'environments/@{e}/variables'), + env_vars=site_config.get(f'environments/@{e}/env_vars'), extras=site_config.get(f'environments/@{e}/extras'), features=site_config.get(f'environments/@{e}/features'), cc=site_config.get(f'environments/@{e}/cc'), @@ -558,7 +557,7 @@ def create(cls, site_config): local_env=Environment( name=f'__rfm_env_{part_name}', modules=site_config.get(f'{partid}/modules'), - variables=site_config.get(f'{partid}/variables') + env_vars=site_config.get(f'{partid}/env_vars') ), max_jobs=site_config.get(f'{partid}/max_jobs'), prepare_cmds=site_config.get(f'{partid}/prepare_cmds'), @@ -582,7 +581,7 @@ def create(cls, site_config): preload_env=Environment( name=f'__rfm_env_{sysname}', modules=site_config.get('systems/0/modules'), - variables=site_config.get('systems/0/variables') + env_vars=site_config.get('systems/0/env_vars') ), prefix=site_config.get('systems/0/prefix'), outputdir=site_config.get('systems/0/outputdir'), @@ -696,9 +695,9 @@ def json(self): 'hostnames': self._hostnames, 'modules_system': self._modules_system.name, 'modules': [m for m in self._preload_env.modules], - 'variables': [ + 'env_vars': [ [name, value] - for name, value in self._preload_env.variables.items() + for name, value in self._preload_env.env_vars.items() ], 'prefix': self._prefix, 'outputdir': self._outputdir, diff --git a/reframe/frontend/autodetect.py b/reframe/frontend/autodetect.py index 21d50d9d2e..6bebce878a 100644 --- a/reframe/frontend/autodetect.py +++ b/reframe/frontend/autodetect.py @@ -231,10 +231,10 @@ def detect_topology(): # No topology found, try to auto-detect it getlogger().debug(f'> no topology file found; auto-detecting...') modules = list(rt.system.preload_environ.modules) - vars = dict(rt.system.preload_environ.variables.items()) + vars = dict(rt.system.preload_environ.env_vars.items()) if _is_part_local(part): modules += part.local_env.modules - vars.update(part.local_env.variables) + vars.update(part.local_env.env_vars) # Unconditionally detect the system for fully local partitions with runtime.temp_environment(modules=modules, variables=vars): diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 0a12eb4c9d..12f8f0b5fd 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -155,8 +155,9 @@ def describe_checks(testcases, printer): # List all required variables required = [] - for var in tc.check._rfm_var_space: - if not tc.check._rfm_var_space[var].is_defined(): + var_space = type(tc.check).var_space + for var in var_space: + if not var_space[var].is_defined(): required.append(var) rec['@required'] = required diff --git a/reframe/frontend/runreport.py b/reframe/frontend/runreport.py index ee11323024..65ab80c8f9 100644 --- a/reframe/frontend/runreport.py +++ b/reframe/frontend/runreport.py @@ -13,7 +13,7 @@ import reframe as rfm import reframe.core.exceptions as errors import reframe.utility.jsonext as jsonext - +from reframe.core.warnings import suppress_deprecations # The schema data version # Major version bumps are expected to break the validation of previous schemas @@ -46,7 +46,8 @@ def __getitem__(self, key): return self._report[key] def __getattr__(self, name): - return getattr(self._report, name) + with suppress_deprecations(): + return getattr(self._report, name) def add_fallback(self, report): self._fallbacks.append(report) diff --git a/reframe/frontend/statistics.py b/reframe/frontend/statistics.py index 8d9194e330..03e0d14cba 100644 --- a/reframe/frontend/statistics.py +++ b/reframe/frontend/statistics.py @@ -10,6 +10,12 @@ import reframe.core.runtime as rt import reframe.core.exceptions as errors import reframe.utility as util +from reframe.core.warnings import suppress_deprecations + + +def _getattr(obj, attr): + with suppress_deprecations(): + return getattr(obj, attr) class TestStats: @@ -199,7 +205,7 @@ def json(self, force=False): for name, var in test_cls.var_space.items(): if var.is_loggable(): try: - entry['check_vars'][name] = getattr(check, name) + entry['check_vars'][name] = _getattr(check, name) except AttributeError: entry['check_vars'][name] = '' @@ -207,7 +213,7 @@ def json(self, force=False): test_cls = type(check) for name, param in test_cls.param_space.items(): if param.is_loggable(): - entry['check_params'][name] = getattr(check, name) + entry['check_params'][name] = _getattr(check, name) testcases.append(entry) diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json index 0145cf4678..eae799bb78 100644 --- a/reframe/schemas/config.json +++ b/reframe/schemas/config.json @@ -235,6 +235,7 @@ "lmod", "nomod", "spack"] }, "modules": {"$ref": "#/defs/modules_list"}, + "env_vars": {"$ref": "#/defs/envvar_list"}, "variables": {"$ref": "#/defs/envvar_list"}, "prefix": {"type": "string"}, "stagedir": {"type": "string"}, @@ -274,6 +275,9 @@ "modules": { "$ref": "#/defs/modules_list" }, + "env_vars": { + "$ref": "#/defs/envvar_list" + }, "variables": { "$ref": "#/defs/envvar_list" }, @@ -283,8 +287,9 @@ } }, "modules": {"$ref": "#/defs/modules_list"}, - "time_limit": {"type": ["string", "null"]}, + "env_vars": {"$ref": "#/defs/envvar_list"}, "variables": {"$ref": "#/defs/envvar_list"}, + "time_limit": {"type": ["string", "null"]}, "max_jobs": {"type": "number"}, "prepare_cmds": { "type": "array", @@ -335,6 +340,7 @@ "properties": { "name": {"$ref": "#/defs/alphanum_ext_string"}, "modules": {"$ref": "#/defs/modules_list"}, + "env_vars": {"$ref": "#/defs/envvar_list"}, "variables": {"$ref": "#/defs/envvar_list"}, "cc": {"type": "string"}, "cxx": {"type": "string"}, @@ -499,6 +505,7 @@ "additionalProperties": false, "defaults": { "environments/modules": [], + "environments/env_vars": [], "environments/variables": [], "environments/cc": "cc", "environments/cxx": "CC", @@ -565,6 +572,7 @@ "systems/max_local_jobs": 8, "systems/modules_system": "nomod", "systems/modules": [], + "systems/env_vars": [], "systems/variables": [], "systems/prefix": ".", "systems/outputdir": "", @@ -576,11 +584,13 @@ "systems/partitions/container_runtime": null, "systems/partitions/container_platforms": [], "systems/partitions/container_platforms/*modules": [], + "systems/partitions/container_platforms/*env_vars": [], "systems/partitions/container_platforms/*variables": [], "systems/partitions/features": [], "systems/partitions/resources": [], "systems/partitions/resources/options": [], "systems/partitions/modules": [], + "systems/partitions/env_vars": [], "systems/partitions/variables": [], "systems/partitions/max_jobs": 8, "systems/partitions/prepare_cmds": [], diff --git a/unittests/resources/settings.py b/unittests/resources/settings.py index 8ded35dc48..4aa611f0b1 100644 --- a/unittests/resources/settings.py +++ b/unittests/resources/settings.py @@ -51,7 +51,7 @@ 'prefix': '.rfm_testing', 'resourcesdir': '.rfm_testing/resources', 'modules': ['foo/1.0'], - 'variables': [['FOO_CMD', 'foobar']], + 'env_vars': [['FOO_CMD', 'foobar']], 'partitions': [ { 'name': 'login', @@ -76,7 +76,7 @@ 'modules': [ {'name': 'foogpu', 'collection': False, 'path': '/foo'} ], - 'variables': [['FOO_GPU', 'yes']], + 'env_vars': [['FOO_GPU', 'yes']], 'resources': [ { 'name': 'gpu', diff --git a/unittests/test_config.py b/unittests/test_config.py index 57fc912f6c..fb343cbea7 100644 --- a/unittests/test_config.py +++ b/unittests/test_config.py @@ -224,7 +224,7 @@ def test_select_subconfig(): assert site_config.get('systems/0/modules') == [{'name': 'foo/1.0', 'collection': False, 'path': None}] - assert site_config.get('systems/0/variables') == [['FOO_CMD', 'foobar']] + assert site_config.get('systems/0/env_vars') == [['FOO_CMD', 'foobar']] assert site_config.get('systems/0/modules_system') == 'nomod' assert site_config.get('systems/0/outputdir') == '' assert site_config.get('systems/0/stagedir') == '' @@ -255,7 +255,7 @@ def test_select_subconfig(): {'type': 'Singularity'} ] assert site_config.get('systems/0/partitions/0/modules') == [] - assert site_config.get('systems/0/partitions/0/variables') == [] + assert site_config.get('systems/0/partitions/0/env_vars') == [] assert site_config.get('systems/0/partitions/0/max_jobs') == 8 assert len(site_config['environments']) == 7 assert site_config.get('environments/@PrgEnv-gnu/cc') == 'gcc' @@ -287,7 +287,7 @@ def test_select_subconfig(): assert site_config.get('systems/0/partitions/0/modules') == [ {'name': 'foogpu', 'collection': False, 'path': '/foo'} ] - assert (site_config.get('systems/0/partitions/0/variables') == + assert (site_config.get('systems/0/partitions/0/env_vars') == [['FOO_GPU', 'yes']]) assert site_config.get('systems/0/partitions/0/max_jobs') == 10 assert site_config.get('environments/@PrgEnv-gnu/cc') == 'cc' @@ -347,7 +347,7 @@ def test_system_create(): assert system.hostnames == ['testsys'] assert system.modules_system.name == 'nomod' assert system.preload_environ.modules == ['foo/1.0'] - assert system.preload_environ.variables == {'FOO_CMD': 'foobar'} + assert system.preload_environ.env_vars == {'FOO_CMD': 'foobar'} assert system.prefix == '.rfm_testing' assert system.stagedir == '' assert system.outputdir == '' @@ -367,7 +367,7 @@ def test_system_create(): assert partition.local_env.modules_detailed == [{ 'name': 'foogpu', 'collection': False, 'path': '/foo' }] - assert partition.local_env.variables == {'FOO_GPU': 'yes'} + assert partition.local_env.env_vars == {'FOO_GPU': 'yes'} assert partition.max_jobs == 10 assert partition.time_limit is None assert len(partition.environs) == 2 @@ -405,6 +405,36 @@ def test_system_create(): assert system.partitions[0].container_runtime == 'Docker' +def test_variables(tmp_path): + # Test that the old syntax using `variables` instead of `env_vars` still + # works + config_file = tmp_path / 'settings.py' + with open(config_file, 'w') as fout: + with open('unittests/resources/settings.py') as fin: + fout.write(fin.read().replace('env_vars', 'variables')) + + site_config = config.load_config(config_file) + site_config.validate() + site_config.select_subconfig('testsys') + assert site_config.get('systems/0/variables') == [['FOO_CMD', 'foobar']] + assert site_config.get('systems/0/env_vars') == [['FOO_CMD', 'foobar']] + + site_config.select_subconfig('testsys:login') + assert site_config.get('systems/0/partitions/0/variables') == [] + assert site_config.get('systems/0/partitions/0/env_vars') == [] + + site_config.select_subconfig('testsys:gpu') + assert (site_config.get('systems/0/partitions/0/variables') == + [['FOO_GPU', 'yes']]) + assert (site_config.get('systems/0/partitions/0/env_vars') == + [['FOO_GPU', 'yes']]) + + # Test that system is created correctly + system = System.create(site_config) + assert system.preload_environ.env_vars == {'FOO_CMD': 'foobar'} + assert system.partitions[0].local_env.env_vars == {'FOO_GPU': 'yes'} + + def test_hostname_autodetection(): # This exercises only the various execution paths diff --git a/unittests/test_environments.py b/unittests/test_environments.py index caf593db97..8992f5347f 100644 --- a/unittests/test_environments.py +++ b/unittests/test_environments.py @@ -71,12 +71,12 @@ def assert_modules_loaded(modules): def test_env_construction(base_environ, env0): assert len(env0.modules) == 1 assert 'testmod_foo' in env0.modules - assert len(env0.variables.keys()) == 3 - assert env0.variables['_var0'] == 'val1' + assert len(env0.env_vars.keys()) == 3 + assert env0.env_vars['_var0'] == 'val1' # No variable expansion, if environment is not loaded - assert env0.variables['_var2'] == '$_var0' - assert env0.variables['_var3'] == '${_var1}' + assert env0.env_vars['_var2'] == '$_var0' + assert env0.env_vars['_var3'] == '${_var1}' # Assert extras assert env0.extras == {'foo': 1, 'bar': 2} @@ -85,11 +85,11 @@ def test_env_construction(base_environ, env0): def test_progenv_construction(): environ = env.ProgEnvironment('myenv', modules=['modfoo'], - variables=[('var', 'val')], + env_vars=[('var', 'val')], extras={'foo': 'bar'}) assert environ.name == 'myenv' assert environ.modules == ['modfoo'] - assert environ.variables == {'var': 'val'} + assert environ.env_vars == {'var': 'val'} assert environ.extras == {'foo': 'bar'} @@ -138,16 +138,16 @@ def test_env_load_already_present(base_environ, user_runtime, def test_env_load_non_overlapping(base_environ): - e0 = env.Environment(name='e0', variables=[('a', '1'), ('b', '2')]) - e1 = env.Environment(name='e1', variables=[('c', '3'), ('d', '4')]) + e0 = env.Environment(name='e0', env_vars=[('a', '1'), ('b', '2')]) + e1 = env.Environment(name='e1', env_vars=[('c', '3'), ('d', '4')]) rt.loadenv(e0, e1) assert rt.is_env_loaded(e0) assert rt.is_env_loaded(e1) def test_load_overlapping(base_environ): - e0 = env.Environment(name='e0', variables=[('a', '1'), ('b', '2')]) - e1 = env.Environment(name='e1', variables=[('b', '3'), ('c', '4')]) + e0 = env.Environment(name='e0', env_vars=[('a', '1'), ('b', '2')]) + e1 = env.Environment(name='e1', env_vars=[('b', '3'), ('c', '4')]) rt.loadenv(e0, e1) assert not rt.is_env_loaded(e0) assert rt.is_env_loaded(e1) @@ -166,8 +166,8 @@ def test_env_not_equal(base_environ): assert env1 != env2 # Variables are ordered, because they might depend on each other - env1 = env.Environment('env1', variables=[('a', 1), ('b', 2)]) - env2 = env.Environment('env1', variables=[('b', 2), ('a', 1)]) + env1 = env.Environment('env1', env_vars=[('a', 1), ('b', 2)]) + env2 = env.Environment('env1', env_vars=[('b', 2), ('a', 1)]) assert env1 != env2 diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 18be91d1c1..4e8eafdd32 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -20,6 +20,7 @@ PerformanceError, SanityError, SkipTestError, ReframeSyntaxError) from reframe.core.meta import make_test +from reframe.core.warnings import ReframeDeprecationWarning def _run(test, partition, prgenv): @@ -159,9 +160,9 @@ def test_eq(): def test_environ_setup(hellotest, local_exec_ctx): # Use test environment for the regression check - hellotest.variables = {'_FOO_': '1', '_BAR_': '2'} + hellotest.env_vars = {'_FOO_': '1', '_BAR_': '2'} hellotest.setup(*local_exec_ctx) - for k in hellotest.variables.keys(): + for k in hellotest.env_vars.keys(): assert k not in os.environ @@ -1889,3 +1890,18 @@ def set_defaults(self): x = _X() assert x.foo == 10 assert x.bar == 100 + + +def _test_variables_deprecation(): + with pytest.warns(ReframeDeprecationWarning): + class _X(rfm.RunOnlyRegressionTest): + variables = {'FOO': '1'} + + test = _X() + print('===') + assert test.env_vars['FOO'] == '1' + + with pytest.warns(ReframeDeprecationWarning): + test.variables['BAR'] == '2' + + assert test.env_vars['BAR'] == '2' From 0e5e5dfbd8ce97881abc679d85579ecebef7c587 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 29 Oct 2022 14:08:35 +0200 Subject: [PATCH 2/5] Update configuration docs --- docs/config_reference.rst | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/docs/config_reference.rst b/docs/config_reference.rst index a9b5b7d36c..8b6df26203 100644 --- a/docs/config_reference.rst +++ b/docs/config_reference.rst @@ -142,7 +142,7 @@ System Configuration These modules modify the ReFrame environment. This is useful in cases where a particular module is needed, for example, to submit jobs on a specific system. -.. js:attribute:: .systems[].variables +.. js:attribute:: .systems[].env_vars :required: No :default: ``[]`` @@ -154,6 +154,14 @@ System Configuration ReFrame will expand its value. Variables are set after the environment modules are loaded. + .. versionadded:: 4.0.0 + +.. js:attribute:: .systems[].variables + + .. deprecated:: 4.0.0 + Please use :js:attr:`env_vars` instead. + If specified in conjunction with :js:attr:`env_vars`, it will be ignored. + .. js:attribute:: .systems[].prefix :required: No @@ -374,7 +382,7 @@ System Partition Configuration When the value is ``null``, no time limit is applied. -.. js:attribute:: .systems[].partitions[].variables +.. js:attribute:: .systems[].partitions[].env_vars :required: No :default: ``[]`` @@ -385,6 +393,13 @@ System Partition Configuration ReFrame will expand its value. Variables are set after the environment modules are loaded. + .. versionadded:: 4.0.0 + +.. js:attribute:: .systems[].partitions[].variables + + .. deprecated:: 4.0.0 + Please use :js:attr:`env_vars` instead. + If specified in conjunction with :js:attr:`env_vars`, it will be ignored. .. js:attribute:: .systems[].partitions[].max_jobs @@ -500,7 +515,7 @@ ReFrame can launch containerized applications, but you need to configure properl A list of `environment module objects <#module-objects>`__ to be loaded when running containerized tests using this container platform. -.. js:attribute:: .systems[].partitions[].container_platforms[].variables +.. js:attribute:: .systems[].partitions[].container_platforms[].env_vars :required: No :default: ``[]`` @@ -511,6 +526,14 @@ ReFrame can launch containerized applications, but you need to configure properl ReFrame will expand its value. Variables are set after the environment modules are loaded. + .. versionadded:: 4.0.0 + +.. js:attribute:: .systems[].partitions[].container_platforms[].variables + + .. deprecated:: 4.0.0 + Please use :js:attr:`env_vars` instead. + If specified in conjunction with :js:attr:`env_vars`, it will be ignored. + Custom Job Scheduler Resources ============================== @@ -619,7 +642,7 @@ They are associated with `system partitions <#system-partition-configuration>`__ A list of `environment module objects <#module-objects>`__ to be loaded when this environment is loaded. -.. js:attribute:: .environments[].variables +.. js:attribute:: .environments[].env_vars :required: No :default: ``[]`` @@ -630,6 +653,14 @@ They are associated with `system partitions <#system-partition-configuration>`__ ReFrame will expand its value. Variables are set after the environment modules are loaded. + .. versionadded:: 4.0.0 + +.. js:attribute:: .environments[].variables + + .. deprecated:: 4.0.0 + Please use :js:attr:`env_vars` instead. + If specified in conjunction with :js:attr:`env_vars`, it will be ignored. + .. js:attribute:: .environments[].features From 5273851f4e3814bd92e5ae2189c2ab02513dc30d Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sun, 30 Oct 2022 22:32:19 +0100 Subject: [PATCH 3/5] Revert `env_vars` type --- reframe/core/pipeline.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 133ccc1c9d..6baa654c57 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -783,11 +783,11 @@ def pipeline_hooks(cls): #: #: These variables will be set during the :func:`setup` phase. #: - #: :type: :class:`Dict[str, object]` + #: :type: :class:`Dict[str, str]` #: :default: ``{}`` #: #: .. versionadded:: 4.0.0 - env_vars = variable(typ.Dict[str, object], value={}, loggable=True) + env_vars = variable(typ.Dict[str, str], value={}, loggable=True) #: Environment variables to be set before running this test. #: From 385e5addd077f8be1cd44479e88adefa96c198b3 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sun, 30 Oct 2022 22:39:21 +0100 Subject: [PATCH 4/5] Remove inaccurate statement about `env_vars` --- reframe/core/pipeline.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 6baa654c57..a034f13d9c 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -781,8 +781,6 @@ def pipeline_hooks(cls): #: Environment variables to be set before running this test. #: - #: These variables will be set during the :func:`setup` phase. - #: #: :type: :class:`Dict[str, str]` #: :default: ``{}`` #: From 30284d71b8bba66191dda66607fe9be2b3187682 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 31 Oct 2022 00:25:08 +0100 Subject: [PATCH 5/5] Update tutorial and the docs --- docs/config_reference.rst | 5 +++-- docs/tutorial_basics.rst | 2 +- tutorials/advanced/containers/container_test.py | 2 +- tutorials/advanced/parameterized/stream.py | 6 +++--- tutorials/basics/stream/stream1.py | 2 +- tutorials/basics/stream/stream2.py | 2 +- tutorials/basics/stream/stream3.py | 2 +- tutorials/basics/stream/stream4.py | 4 ++-- tutorials/cscs-webinar-2022/tests/stream4.py | 2 +- tutorials/cscs-webinar-2022/tests/stream5.py | 2 +- tutorials/cscs-webinar-2022/tests/stream6.py | 2 +- tutorials/cscs-webinar-2022/tests/stream7.py | 2 +- tutorials/cscs-webinar-2022/tests/stream8.py | 4 ++-- tutorials/cscs-webinar-2022/tests/stream9.py | 4 ++-- 14 files changed, 21 insertions(+), 20 deletions(-) diff --git a/docs/config_reference.rst b/docs/config_reference.rst index 8b6df26203..c3c9054b1c 100644 --- a/docs/config_reference.rst +++ b/docs/config_reference.rst @@ -899,7 +899,8 @@ All logging handlers share the following set of common attributes: ``%(check_descr)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.descr` attribute. ``%(check_display_name)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.display_name` attribute. ``%(check_environ)s``, The name of the test's :attr:`~reframe.core.pipeline.RegressionTest.current_environ`. - ``%(check_exclusive_access)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.exclusive_access` attribute. + ``%(check_env_vars)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.env_vars` attribute. + ``%(check_exclusive_access)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.exclusive_access` attribute. ``%(check_executable)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.executable` attribute. ``%(check_executable_opts)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.executable_opts` attribute. ``%(check_extra_resources)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.extra_resources` attribute. @@ -949,7 +950,7 @@ All logging handlers share the following set of common attributes: ``%(check_use_multithreading)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.use_multithreading` attribute. ``%(check_valid_prog_environs)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.valid_prog_environs` attribute. ``%(check_valid_systems)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.valid_systems` attribute. - ``%(check_variables)s``, The value of the :attr:`~reframe.core.pipeline.RegressionTest.variables` attribute. + ``%(check_variables)s``, DEPRECATED: Please use ``%(check_env_vars)s`` instead. ``%(osuser)s``, The name of the OS user running ReFrame. ``%(osgroup)s``, The name of the OS group running ReFrame. ``%(version)s``, The ReFrame version. diff --git a/docs/tutorial_basics.rst b/docs/tutorial_basics.rst index 180114cad5..ea7b043c23 100644 --- a/docs/tutorial_basics.rst +++ b/docs/tutorial_basics.rst @@ -430,7 +430,7 @@ The next thing to notice is the :attr:`~reframe.core.pipeline.RegressionTest.pre These commands will be executed from the test's stage directory. In this case, we just fetch the source code of the benchmark. For running the benchmark, we need to set the OpenMP number of threads and pin them to the right CPUs through the ``OMP_NUM_THREADS`` and ``OMP_PLACES`` environment variables. -You can set environment variables in a ReFrame test through the :attr:`~reframe.core.pipeline.RegressionTest.variables` dictionary. +You can set environment variables in a ReFrame test through the :attr:`~reframe.core.pipeline.RegressionTest.env_vars` dictionary. What makes a ReFrame test a performance test is the definition of at least one :ref:`performance function`. Similarly to a test's :func:`@sanity_function`, a performance function is a member function decorated with the :attr:`@performance_function` decorator, which binds the decorated function to a given unit. diff --git a/tutorials/advanced/containers/container_test.py b/tutorials/advanced/containers/container_test.py index cb1b2f3403..5f7618f34e 100644 --- a/tutorials/advanced/containers/container_test.py +++ b/tutorials/advanced/containers/container_test.py @@ -15,7 +15,7 @@ class ContainerTest(rfm.RunOnlyRegressionTest): valid_prog_environs = ['builtin'] @run_before('run') - def set_container_variables(self): + def setup_container_platf(self): self.descr = f'Run commands inside a container using {self.platform}' image_prefix = 'docker://' if self.platform == 'Singularity' else '' self.container_platform = self.platform diff --git a/tutorials/advanced/parameterized/stream.py b/tutorials/advanced/parameterized/stream.py index 53fb293395..1593037905 100644 --- a/tutorials/advanced/parameterized/stream.py +++ b/tutorials/advanced/parameterized/stream.py @@ -21,7 +21,7 @@ class StreamMultiSysTest(rfm.RegressionTest): ] build_system = 'SingleSource' sourcepath = 'stream.c' - variables = { + env_vars = { 'OMP_NUM_THREADS': '4', 'OMP_PLACES': 'cores' } @@ -48,7 +48,7 @@ class StreamMultiSysTest(rfm.RegressionTest): }) @run_after('init') - def set_variables(self): + def setup_build(self): self.array_size = (self.num_bytes >> 3) // 3 self.ntimes = 100*1024*1024 // self.array_size self.descr = ( @@ -67,7 +67,7 @@ def set_compiler_flags(self): def set_num_threads(self): num_threads = self.cores.get(self.current_partition.fullname, 1) self.num_cpus_per_task = num_threads - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(num_threads), 'OMP_PLACES': 'cores' } diff --git a/tutorials/basics/stream/stream1.py b/tutorials/basics/stream/stream1.py index b818d9db1c..5cb74abbc5 100644 --- a/tutorials/basics/stream/stream1.py +++ b/tutorials/basics/stream/stream1.py @@ -17,7 +17,7 @@ class StreamTest(rfm.RegressionTest): ] build_system = 'SingleSource' sourcepath = 'stream.c' - variables = { + env_vars = { 'OMP_NUM_THREADS': '4', 'OMP_PLACES': 'cores' } diff --git a/tutorials/basics/stream/stream2.py b/tutorials/basics/stream/stream2.py index f31672941d..89e6a6a330 100644 --- a/tutorials/basics/stream/stream2.py +++ b/tutorials/basics/stream/stream2.py @@ -17,7 +17,7 @@ class StreamAltTest(rfm.RegressionTest): ] build_system = 'SingleSource' sourcepath = 'stream.c' - variables = { + env_vars = { 'OMP_NUM_THREADS': '4', 'OMP_PLACES': 'cores' } diff --git a/tutorials/basics/stream/stream3.py b/tutorials/basics/stream/stream3.py index 249a0a8094..20b5c23edf 100644 --- a/tutorials/basics/stream/stream3.py +++ b/tutorials/basics/stream/stream3.py @@ -17,7 +17,7 @@ class StreamWithRefTest(rfm.RegressionTest): ] build_system = 'SingleSource' sourcepath = 'stream.c' - variables = { + env_vars = { 'OMP_NUM_THREADS': '4', 'OMP_PLACES': 'cores' } diff --git a/tutorials/basics/stream/stream4.py b/tutorials/basics/stream/stream4.py index df7df1ceca..fa68616f20 100644 --- a/tutorials/basics/stream/stream4.py +++ b/tutorials/basics/stream/stream4.py @@ -17,7 +17,7 @@ class StreamMultiSysTest(rfm.RegressionTest): ] build_system = 'SingleSource' sourcepath = 'stream.c' - variables = { + env_vars = { 'OMP_NUM_THREADS': '4', 'OMP_PLACES': 'cores' } @@ -56,7 +56,7 @@ def set_compiler_flags(self): def set_num_threads(self): num_threads = self.cores.get(self.current_partition.fullname, 1) self.num_cpus_per_task = num_threads - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(num_threads), 'OMP_PLACES': 'cores' } diff --git a/tutorials/cscs-webinar-2022/tests/stream4.py b/tutorials/cscs-webinar-2022/tests/stream4.py index 6d2dbc3d56..46b5357148 100644 --- a/tutorials/cscs-webinar-2022/tests/stream4.py +++ b/tutorials/cscs-webinar-2022/tests/stream4.py @@ -29,7 +29,7 @@ def setup_build(self): def setup_omp_env(self): procinfo = self.current_partition.processor self.num_cpus_per_task = procinfo.num_cores - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(self.num_cpus_per_task), 'OMP_PLACES': 'cores' } diff --git a/tutorials/cscs-webinar-2022/tests/stream5.py b/tutorials/cscs-webinar-2022/tests/stream5.py index f0ba5614fd..151d3d6d18 100644 --- a/tutorials/cscs-webinar-2022/tests/stream5.py +++ b/tutorials/cscs-webinar-2022/tests/stream5.py @@ -32,7 +32,7 @@ def setup_build(self): def setup_omp_env(self): procinfo = self.current_partition.processor self.num_cpus_per_task = procinfo.num_cores - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(self.num_cpus_per_task), 'OMP_PLACES': 'cores' } diff --git a/tutorials/cscs-webinar-2022/tests/stream6.py b/tutorials/cscs-webinar-2022/tests/stream6.py index a78fc27c18..a16a9e8faf 100644 --- a/tutorials/cscs-webinar-2022/tests/stream6.py +++ b/tutorials/cscs-webinar-2022/tests/stream6.py @@ -34,7 +34,7 @@ def setup_build(self): def setup_omp_env(self): procinfo = self.current_partition.processor self.num_cpus_per_task = procinfo.num_cores - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(self.num_cpus_per_task), 'OMP_PLACES': 'cores' } diff --git a/tutorials/cscs-webinar-2022/tests/stream7.py b/tutorials/cscs-webinar-2022/tests/stream7.py index e3784b716e..8abda0b980 100644 --- a/tutorials/cscs-webinar-2022/tests/stream7.py +++ b/tutorials/cscs-webinar-2022/tests/stream7.py @@ -45,7 +45,7 @@ def setup_omp_env(self): self.executable = os.path.join(self.stream_binaries.stagedir, 'stream') procinfo = self.current_partition.processor self.num_cpus_per_task = procinfo.num_cores - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(self.num_cpus_per_task), 'OMP_PLACES': 'cores' } diff --git a/tutorials/cscs-webinar-2022/tests/stream8.py b/tutorials/cscs-webinar-2022/tests/stream8.py index ebe581539a..e8b6f6f91c 100644 --- a/tutorials/cscs-webinar-2022/tests/stream8.py +++ b/tutorials/cscs-webinar-2022/tests/stream8.py @@ -45,7 +45,7 @@ def setup_omp_env(self): self.executable = os.path.join(self.stream_binaries.stagedir, 'stream') procinfo = self.current_partition.processor self.num_cpus_per_task = procinfo.num_cores - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(self.num_cpus_per_task), 'OMP_PLACES': 'cores' } @@ -67,7 +67,7 @@ class stream_scale_test(stream_test): @run_before('run') def set_cpus_per_task(self): self.num_cpus_per_task = self.num_threads - self.variables['OMP_NUM_THREADS'] = str(self.num_cpus_per_task) + self.env_vars['OMP_NUM_THREADS'] = str(self.num_cpus_per_task) @run_after('setup') def skip_if_too_large(self): diff --git a/tutorials/cscs-webinar-2022/tests/stream9.py b/tutorials/cscs-webinar-2022/tests/stream9.py index aaad89d109..3beb5f1f3c 100644 --- a/tutorials/cscs-webinar-2022/tests/stream9.py +++ b/tutorials/cscs-webinar-2022/tests/stream9.py @@ -46,7 +46,7 @@ def setup_omp_env(self): self.executable = os.path.join(self.stream_binaries.stagedir, 'stream') procinfo = self.current_partition.processor self.num_cpus_per_task = procinfo.num_cores - self.variables = { + self.env_vars = { 'OMP_NUM_THREADS': str(self.num_cpus_per_task), 'OMP_PLACES': 'cores' } @@ -83,4 +83,4 @@ def setup_thread_config(self): @run_before('run') def set_cpus_per_task(self): self.num_cpus_per_task = self.num_threads - self.variables['OMP_NUM_THREADS'] = str(self.num_cpus_per_task) + self.env_vars['OMP_NUM_THREADS'] = str(self.num_cpus_per_task)