From 418fabf17b3189b54a124506e611349ca5865ca7 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 27 Feb 2021 14:01:16 +0100 Subject: [PATCH 1/8] Rename _rfm_init() --- reframe/core/pipeline.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 41d052dc5c..e03e0e18ab 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -138,6 +138,7 @@ class RegressionMixin(metaclass=RegressionTestMeta): .. versionadded:: 3.4.2 ''' + def __getattribute__(self, name): try: return super().__getattribute__(name) @@ -771,7 +772,7 @@ def __new__(cls, *args, _rfm_use_params=False, **kwargs): os.path.dirname(inspect.getfile(cls)) ) - obj._rfm_init(name, prefix) + obj.__rfm_init__(name, prefix) return obj def __init__(self): @@ -796,7 +797,7 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False, **kwargs): os.path.dirname(inspect.getfile(cls)) ) - def _rfm_init(self, name=None, prefix=None): + def __rfm_init__(self, name=None, prefix=None): if name is not None: self.name = name From 98c418c02d8e7ab6e93be575867a154745b7ab1c Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sun, 28 Feb 2021 23:20:13 +0100 Subject: [PATCH 2/8] Revisit naming scheme of tests --- reframe/core/decorators.py | 40 +++++- reframe/core/meta.py | 3 +- reframe/core/namespaces.py | 10 ++ reframe/core/parameters.py | 49 ++++++-- reframe/core/pipeline.py | 117 +++++++++++------- reframe/core/schedulers/__init__.py | 13 +- reframe/frontend/loader.py | 26 ++-- reframe/utility/__init__.py | 6 +- unittests/resources/checks/frontend_checks.py | 6 +- unittests/test_dependencies.py | 108 ++++++++-------- unittests/test_loader.py | 2 +- unittests/test_pipeline.py | 105 ++++------------ 12 files changed, 270 insertions(+), 215 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 72daa9a2d4..bfaf6fea8d 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -22,6 +22,7 @@ import reframe.utility.osext as osext from reframe.core.exceptions import ReframeSyntaxError, user_frame from reframe.core.logging import getlogger +from reframe.core.parameters import TestParam from reframe.core.pipeline import RegressionTest from reframe.utility.versioning import VersionValidator @@ -56,9 +57,11 @@ def _instantiate_all(): ret.append(_instantiate(cls, args)) except Exception: frame = user_frame(*sys.exc_info()) - msg = "skipping test due to errors: %s: " % cls.__name__ - msg += "use `-v' for more information\n" - msg += " FILE: %s:%s" % (frame.filename, frame.lineno) + msg = f"skipping test due to errors: {cls.__qualname__}: " + msg += f"use `-v' for more information\n" + if frame: + msg += f' FILE: {frame.filename}:{frame.lineno}' + getlogger().warning(msg) getlogger().verbose(traceback.format_exc()) @@ -117,6 +120,28 @@ def parameterized_test(*inst): This decorator does not instantiate any test. It only registers them. The actual instantiation happens during the loading phase of the test. ''' + def _extend_param_space(cls): + if not inst: + return + + argnames = inspect.getfullargspec(cls.__init__).args[1:] + params = {} + for args in inst: + if isinstance(args, collections.abc.Sequence): + for i, v in enumerate(args): + params.setdefault(argnames[i], ()) + params[argnames[i]] += (v,) + elif isinstance(args, collections.abc.Mapping): + for k, v in args.items(): + params.setdefault(k, ()) + params[k] += (v,) + + # argvalues = zip(*inst) + # for name, values in zip(argnames, argvalues): + # cls.param_space.params[name] = values + + cls.param_space.params.update(params) + def _do_register(cls): _validate_test(cls) if not cls.param_space.is_empty(): @@ -124,6 +149,15 @@ def _do_register(cls): f'{cls.__qualname__!r} is already a parameterized test' ) + # NOTE: We need to mark that the test is using the old parameterized + # test syntax, so that any derived test does not extend its + # parameterization space with these parameters. This is to keep + # compatibility with the original behavior of this decorator, when it + # was not implemented using the new parameter machinery. + cls._rfm_compat_parameterized = True + + _extend_param_space(cls) + cls.param_space.set_iter_fn(zip) for args in inst: _register_test(cls, args) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 16058cd8a6..9247f7f58c 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -18,6 +18,7 @@ class RegressionTestMeta(type): class MetaNamespace(namespaces.LocalNamespace): '''Custom namespace to control the cls attribute assignment.''' + def __setitem__(self, key, value): if isinstance(value, variables.VarDirective): # Insert the attribute in the variable namespace @@ -159,7 +160,7 @@ def __call__(cls, *args, **kwargs): to perform specific reframe-internal actions. This gives extra control over the class instantiation process, allowing reframe to instantiate the regression test class differently if this class was registered or - not (e.g. when deep-copying a regression test object). These interal + not (e.g. when deep-copying a regression test object). These internal arguments must be intercepted before the object initialization, since these would otherwise affect the __init__ method's signature, and these internal mechanisms must be fully transparent to the user. diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index 13ce9dc62c..1e402f60e2 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -135,11 +135,21 @@ def assert_target_cls(self, cls): assert isinstance(getattr(cls, self.local_namespace_name), LocalNamespace) + def allow_inheritance(self, cls, base): + '''Return true if cls is allowed to inherit the namespace of base. + + Subclasses may override that to customize the behavior. + ''' + return True + def inherit(self, cls): '''Inherit the Namespaces from the bases.''' for base in filter(lambda x: hasattr(x, self.namespace_name), cls.__bases__): + if not self.allow_inheritance(cls, base): + continue + assert isinstance(getattr(base, self.namespace_name), type(self)) self.join(getattr(base, self.namespace_name), cls) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index bbb7643fd6..515c5fd78c 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -27,24 +27,30 @@ class TestParam: ''' def __init__(self, values=None, - inherit_params=False, filter_params=None): + inherit_params=False, filter_params=None, fmtval=None): if values is None: values = [] - # By default, filter out all the parameter values defined in the - # base classes. if not inherit_params: + # By default, filter out all the parameter values defined in the + # base classes. def filter_params(x): return () - - # If inherit_params==True, inherit all the parameter values from the - # base classes as default behaviour. elif filter_params is None: + # If inherit_params==True, inherit all the parameter values from the + # base classes as default behaviour. def filter_params(x): return x + if not callable(filter_params): + raise TypeError('filter_params must be a callable') + + if fmtval and not callable(fmtval): + raise TypeError('fmtval must be a callable') + self.values = tuple(values) self.filter_params = filter_params + self.format_value = fmtval def __set_name__(self, owner, name): self.name = name @@ -90,7 +96,16 @@ def __init__(self, target_cls=None, target_namespace=None): super().__init__(target_cls, target_namespace) # Internal parameter space usage tracker - self.__unique_iter = iter(self) + self.set_iter_fn(itertools.product) + # self.__unique_iter = iter(self) + + # Alternative value formatting functions + self.__format_fn = {} + + def allow_inheritance(self, cls, base): + # Do not allow inheritance of the parameterization space in case of + # the old @parameterized_test decorator. + return not hasattr(cls, '_rfm_compat_parameterized') def join(self, other, cls): '''Join other parameter space into the current one. @@ -103,6 +118,7 @@ def join(self, other, cls): :param other: instance of the ParamSpace class. :param cls: the target class. ''' + for key in other.params: # With multiple inheritance, a single parameter # could be doubly defined and lead to repeated @@ -128,6 +144,8 @@ def extend(self, cls): self.params[name] = ( p.filter_params(self.params.get(name, ())) + p.values ) + if p.format_value: + self.__format_fn[name] = p.format_value # If any previously declared parameter was defined in the class body # by directly assigning it a value, raise an error. Parameters must be @@ -158,21 +176,27 @@ def inject(self, obj, cls=None, use_params=False): parameter values defined in the parameter space. ''' + # Set the values of the test parameters (if any) if use_params and self.params: try: # Consume the parameter space iterator + injected = [] param_values = next(self.unique_iter) for index, key in enumerate(self.params): setattr(obj, key, param_values[index]) + format_fn = self.__format_fn.get(key) + injected.append( + (key, param_values[index], format_fn) + ) + obj.params_inserted(injected) except StopIteration as no_params: raise RuntimeError( f'exhausted parameter space: all possible parameter value' f' combinations have been used for ' f'{obj.__class__.__qualname__}' ) from None - else: # Otherwise init the params as None for key in self.params: @@ -186,10 +210,17 @@ def __iter__(self): :return: generator object to iterate over the parameter space. ''' - yield from itertools.product( + # yield from itertools.product( + # *(copy.deepcopy(p) for p in self.params.values()) + # ) + yield from self.__iterator( *(copy.deepcopy(p) for p in self.params.values()) ) + def set_iter_fn(self, fn): + self.__iterator = fn + self.__unique_iter = iter(self) + @property def params(self): return self._namespace diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index e03e0e18ab..583b474c75 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -15,6 +15,7 @@ import functools import glob +import hashlib import inspect import itertools import numbers @@ -195,7 +196,7 @@ def pipeline_hooks(cls): #: The name of the test. #: #: :type: string that can contain any character except ``/`` - name = variable(typ.Str[r'[^\/]+']) + name = variable(str) #: List of programming environments supported by this test. #: @@ -740,51 +741,36 @@ def pipeline_hooks(cls): #: :type: boolean : :default: :class:`True` build_locally = variable(bool, value=True) + _param_hash = variable(str, type(None), value=None) + def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) + obj.name = cls.__qualname__ # Insert the var & param spaces cls._rfm_var_space.inject(obj, cls) cls._rfm_param_space.inject(obj, cls, _rfm_use_params) - # Create a test name from the class name and the constructor's - # arguments - name = cls.__qualname__ - name += obj._append_parameters_to_name() - - # or alternatively, if the parameterized test was defined the old way. - if args or kwargs: - arg_names = map(lambda x: util.toalphanum(str(x)), - itertools.chain(args, kwargs.values())) - name += '_' + '_'.join(arg_names) - # Determine the prefix try: - prefix = cls._rfm_custom_prefix + obj._prefix = cls._rfm_custom_prefix except AttributeError: if osext.is_interactive(): - prefix = os.getcwd() + obj._prefix = os.getcwd() else: try: - prefix = cls._rfm_pinned_prefix + obj._prefix = cls._rfm_pinned_prefix except AttributeError: - prefix = os.path.abspath( + obj._prefix = os.path.abspath( os.path.dirname(inspect.getfile(cls)) ) - obj.__rfm_init__(name, prefix) + obj.__rfm_init__() return obj def __init__(self): pass - def _append_parameters_to_name(self): - if self._rfm_param_space.params: - return '_' + '_'.join([util.toalphanum(str(self.__dict__[key])) - for key in self._rfm_param_space.params]) - else: - return '' - @classmethod def __init_subclass__(cls, *, special=False, pin_prefix=False, **kwargs): super().__init_subclass__(**kwargs) @@ -797,17 +783,13 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False, **kwargs): os.path.dirname(inspect.getfile(cls)) ) - def __rfm_init__(self, name=None, prefix=None): - if name is not None: - self.name = name - + def __rfm_init__(self): self.descr = self.name self.executable = os.path.join('.', self.name) self._perfvalues = {} # Static directories of the regression check - self._prefix = os.path.abspath(prefix) - if not os.path.isdir(os.path.join(self._prefix, self.sourcesdir)): + if not os.path.isdir(os.path.join(self.prefix, self.sourcesdir)): self.sourcesdir = None # Runtime information of the test @@ -847,7 +829,46 @@ def __rfm_init__(self, name=None, prefix=None): # Just an empty environment self._cdt_environ = env.Environment('__rfm_cdt_environ') - # Export read-only views to interesting fields + @property + def unique_id(self): + ret = type(self).__qualname__ + if self._param_hash: + ret += '_' + self._param_hash[:8] + + return ret + + def _set_param_hash(self, params): + obj = {} + for name, value, _ in params: + obj[name] = value + + h = hashlib.sha256() + h.update(bytes(jsonext.dumps(obj), encoding='utf-8')) + self._param_hash = h.hexdigest() + + def params_inserted(self, params): + '''Callback that is called when all parameters have been injected in + this test. + + This is used to generate the human-readable name of the test as well + as the parameter hash + + :arg params: A list of tuples ``(name, value, fmtval)``. + + :meta private: + ''' + + # Calculate the parameter hash + self._set_param_hash(params) + + # Generate the test name + self.name = type(self).__qualname__ + for name, value, fmtval in params: + if fmtval: + value = fmtval(value) + + self.name += f'%{name}={value}' + @property def current_environ(self): '''The programming environment that the regression test is currently @@ -1040,16 +1061,16 @@ def _setup_paths(self): runtime = rt.runtime() self._stagedir = runtime.make_stagedir( self.current_system.name, self._current_partition.name, - self._current_environ.name, self.name + self._current_environ.name, self.unique_id ) self._outputdir = runtime.make_outputdir( self.current_system.name, self._current_partition.name, - self._current_environ.name, self.name + self._current_environ.name, self.unique_id ) except OSError as e: raise PipelineError('failed to set up paths') from e - def _setup_job(self, name, force_local=False, **job_opts): + def _setup_job(self, name, build_job=False, force_local=False, **job_opts): '''Setup the job related to this check.''' if force_local: @@ -1064,9 +1085,11 @@ def _setup_job(self, name, force_local=False, **job_opts): f'(scheduler: {scheduler.registered_name!r}, ' f'launcher: {launcher.registered_name!r})' ) + filename = '_rfm_build.sh' if build_job else '_rfm_run.sh' return Job.create(scheduler, launcher, name=name, + script_filename=filename, workdir=self._stagedir, max_pending_time=self.max_pending_time, sched_access=self._current_partition.access, @@ -1105,11 +1128,12 @@ def setup(self, partition, environ, **job_opts): self._current_partition = partition self._current_environ = environ self._setup_paths() - self._job = self._setup_job(f'rfm_{self.name}_job', - self.local, + self._job = self._setup_job(f'rfm_runjob_{self.name}', + force_local=self.local, **job_opts) - self._build_job = self._setup_job(f'rfm_{self.name}_build', - self.local or self.build_locally, + self._build_job = self._setup_job(f'rfm_buildjob_{self.name}', + build_job=True, + force_local=self.local or self.build_locally, **job_opts) def _copy_to_stagedir(self, path): @@ -1168,7 +1192,7 @@ def compile(self): if osext.is_url(self.sourcesdir): self._clone_to_stagedir(self.sourcesdir) else: - self._copy_to_stagedir(os.path.join(self._prefix, + self._copy_to_stagedir(os.path.join(self.prefix, self.sourcesdir)) # Verify the sourcepath and determine the sourcepath in the stagedir @@ -1805,10 +1829,10 @@ def __eq__(self, other): if not isinstance(other, RegressionTest): return NotImplemented - return self.name == other.name + return self.unique_id == other.unique_id def __hash__(self): - return hash(self.name) + return hash(self.unique_id) def __rfm_json_decode__(self, json): # 'tags' are decoded as list, so we convert them to a set @@ -1832,8 +1856,8 @@ def setup(self, partition, environ, **job_opts): self._current_partition = partition self._current_environ = environ self._setup_paths() - self._job = self._setup_job(f'rfm_{self.name}_job', - self.local, + self._job = self._setup_job(f'rfm_runjob_{self.name}', + force_local=self.local, **job_opts) def compile(self): @@ -1859,7 +1883,7 @@ def run(self): if osext.is_url(self.sourcesdir): self._clone_to_stagedir(self.sourcesdir) else: - self._copy_to_stagedir(os.path.join(self._prefix, + self._copy_to_stagedir(os.path.join(self.prefix, self.sourcesdir)) super().run.__wrapped__(self) @@ -1889,8 +1913,9 @@ def setup(self, partition, environ, **job_opts): self._current_partition = partition self._current_environ = environ self._setup_paths() - self._build_job = self._setup_job(f'rfm_{self.name}_build', - self.local or self.build_locally, + self._build_job = self._setup_job(f'rfm_buildjob_{self.name}', + build_job=True, + force_local=self.local or self.build_locally, **job_opts) @property diff --git a/reframe/core/schedulers/__init__.py b/reframe/core/schedulers/__init__.py index d4872a541e..f30a6d242e 100644 --- a/reframe/core/schedulers/__init__.py +++ b/reframe/core/schedulers/__init__.py @@ -8,6 +8,7 @@ # import abc +import os import time import reframe.core.fields as fields @@ -168,7 +169,7 @@ def __init__(self, stderr=None, max_pending_time=None, sched_flex_alloc_nodes=None, - sched_access=[], + sched_access=None, sched_exclusive_access=None, sched_options=None): @@ -185,14 +186,16 @@ def __init__(self, self._name = name self._workdir = workdir - self._script_filename = script_filename or '%s.sh' % name - self._stdout = stdout or '%s.out' % name - self._stderr = stderr or '%s.err' % name + self._script_filename = script_filename or f'{name}.sh' + + basename = os.path.splitext(self._script_filename)[0] + self._stdout = stdout or f'{basename}.out' + self._stderr = stderr or f'{basename}.err' self._max_pending_time = max_pending_time # Backend scheduler related information self._sched_flex_alloc_nodes = sched_flex_alloc_nodes - self._sched_access = sched_access + self._sched_access = list(sched_access) if sched_access else [] self._sched_exclusive_access = sched_exclusive_access # Live job information; to be filled during job's lifetime by the diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index 9305d493af..d7d3a4c9b4 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -167,33 +167,37 @@ def load_from_module(self, module): getlogger().debug(f' > Loaded {len(ret)} test(s)') return ret - def load_from_file(self, filename, **check_args): + def load_from_file(self, filename, force=False): if not self._validate_source(filename): return [] - return self.load_from_module(util.import_module_from_file(filename)) + return self.load_from_module( + util.import_module_from_file(filename, force) + ) - def load_from_dir(self, dirname, recurse=False): + def load_from_dir(self, dirname, recurse=False, force=False): checks = [] for entry in os.scandir(dirname): if recurse and entry.is_dir(): - checks.extend( - self.load_from_dir(entry.path, recurse) - ) + checks += self.load_from_dir(entry.path, recurse, force) if (entry.name.startswith('.') or not entry.name.endswith('.py') or not entry.is_file()): continue - checks.extend(self.load_from_file(entry.path)) + checks += self.load_from_file(entry.path, force) return checks - def load_all(self): + def load_all(self, force=False): '''Load all checks in self._load_path. - If a prefix exists, it will be prepended to each path.''' + If a prefix exists, it will be prepended to each path. + + :arg force: Force reloading of test files. + :returns: The list of loaded tests. + ''' checks = [] for d in self._load_path: getlogger().debug(f'Looking for tests in {d!r}') @@ -201,8 +205,8 @@ def load_all(self): continue if os.path.isdir(d): - checks.extend(self.load_from_dir(d, self._recurse)) + checks += self.load_from_dir(d, self._recurse, force) else: - checks.extend(self.load_from_file(d)) + checks += self.load_from_file(d, force) return checks diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index df8eb20d9c..1f71d77cb8 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -67,10 +67,11 @@ def _do_import_module_from_file(filename, module_name=None): return module -def import_module_from_file(filename): +def import_module_from_file(filename, force=False): '''Import module from file. :arg filename: The path to the filename of a Python module. + :arg force: Force reload of module in case it is already loaded :returns: The loaded Python module. ''' @@ -94,6 +95,9 @@ def import_module_from_file(filename): if match: module_name = _get_module_name(match['rel_filename']) + if force: + sys.modules.pop(module_name, None) + return importlib.import_module(module_name) diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index ac2004b16a..80e9ef0f17 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -161,7 +161,11 @@ class SleepCheck(BaseFrontendCheck): def __init__(self, sleep_time): super().__init__() - self.name = '%s_%s' % (self.name, SleepCheck._next_id) + + # Simulate a parameterized test, so as to get a proper unique test id + self.params_inserted([('id', self._next_id, None), + ('sleep_time', sleep_time, None)]) + print(self.name, self.unique_id) self.sourcesdir = None self.sleep_time = sleep_time self.executable = 'python3' diff --git a/unittests/test_dependencies.py b/unittests/test_dependencies.py index e1db9c54e4..325ad1de8a 100644 --- a/unittests/test_dependencies.py +++ b/unittests/test_dependencies.py @@ -106,7 +106,7 @@ def loader(): def test_eq_hash(loader, exec_ctx): - cases = executors.generate_testcases(loader.load_all()) + cases = executors.generate_testcases(loader.load_all(force=True)) case0 = find_case('Test0', 'e0', 'p0', cases) case1 = find_case('Test0', 'e1', 'p0', cases) case0_copy = case0.clone() @@ -384,11 +384,13 @@ def __init__(self, kind): def test_build_deps(loader, exec_ctx): - checks = loader.load_all() + # We need to force reload the tests in order to properly populate the + # parameter space in every test + checks = loader.load_all(force=True) cases = executors.generate_testcases(checks) # Test calling getdep() before having built the graph - t = find_check('Test1_fully', checks) + t = find_check('Test1%kind=fully', checks) with pytest.raises(DependencyError): t.getdep('Test0', 'e0', 'p0') @@ -397,110 +399,110 @@ def test_build_deps(loader, exec_ctx): dependencies.validate_deps(deps) # Check dependencies for fully connected graph - assert num_deps(deps, 'Test1_fully') == 16 + assert num_deps(deps, 'Test1%kind=fully') == 16 for p0 in ['sys0:p0', 'sys0:p1']: for p1 in ['sys0:p0', 'sys0:p1']: for e0 in ['e0', 'e1']: for e1 in ['e0', 'e1']: assert has_edge(deps, - Node('Test1_fully', p0, e0), + Node('Test1%kind=fully', p0, e0), Node('Test0', p1, e1)) # Check dependencies with same partition - assert num_deps(deps, 'Test1_by_part') == 8 + assert num_deps(deps, 'Test1%kind=by_part') == 8 for p in ['sys0:p0', 'sys0:p1']: for e0 in ['e0', 'e1']: for e1 in ['e0', 'e1']: assert has_edge(deps, - Node('Test1_by_part', p, e0), + Node('Test1%kind=by_part', p, e0), Node('Test0', p, e1)) # Check dependencies with same partition environment - assert num_deps(deps, 'Test1_by_case') == 4 - assert num_deps(deps, 'Test1_default') == 4 + assert num_deps(deps, 'Test1%kind=by_case') == 4 + assert num_deps(deps, 'Test1%kind=default') == 4 for p in ['sys0:p0', 'sys0:p1']: for e in ['e0', 'e1']: assert has_edge(deps, - Node('Test1_by_case', p, e), + Node('Test1%kind=by_case', p, e), Node('Test0', p, e)) assert has_edge(deps, - Node('Test1_default', p, e), + Node('Test1%kind=default', p, e), Node('Test0', p, e)) - assert num_deps(deps, 'Test1_any') == 12 + assert num_deps(deps, 'Test1%kind=any') == 12 for p0 in ['sys0:p0', 'sys0:p1']: for p1 in ['sys0:p0', 'sys0:p1']: for e0 in ['e0', 'e1']: for e1 in ['e0', 'e1']: if (p0 == 'sys0:p0' or e1 == 'e1'): assert has_edge(deps, - Node('Test1_any', p0, e0), + Node('Test1%kind=any', p0, e0), Node('Test0', p1, e1)) - assert num_deps(deps, 'Test1_all') == 2 + assert num_deps(deps, 'Test1%kind=all') == 2 for p0 in ['sys0:p0', 'sys0:p1']: for p1 in ['sys0:p0', 'sys0:p1']: for e0 in ['e0', 'e1']: for e1 in ['e0', 'e1']: if (p0 == 'sys0:p0' and p1 == 'sys0:p0' and e1 == 'e1'): assert has_edge(deps, - Node('Test1_any', p0, e0), + Node('Test1%kind=any', p0, e0), Node('Test0', p1, e1)) # Check custom dependencies - assert num_deps(deps, 'Test1_custom') == 1 + assert num_deps(deps, 'Test1%kind=custom') == 1 assert has_edge(deps, - Node('Test1_custom', 'sys0:p0', 'e0'), + Node('Test1%kind=custom', 'sys0:p0', 'e0'), Node('Test0', 'sys0:p1', 'e1')) - # Check dependencies of Test1_nodeps - assert num_deps(deps, 'Test1_nodeps') == 0 + # Check dependencies of Test1%kind=nodeps + assert num_deps(deps, 'Test1%kind=nodeps') == 0 # Check in-degree of Test0 - # 4 from Test1_fully, - # 2 from Test1_by_part, - # 1 from Test1_by_case, - # 2 from Test1_any, - # 2 from Test1_all, - # 0 from Test1_custom, - # 1 from Test1_default - # 0 from Test1_nodeps + # 4 from Test1%kind=fully, + # 2 from Test1%kind=by_part, + # 1 from Test1%kind=by_case, + # 2 from Test1%kind=any, + # 2 from Test1%kind=all, + # 0 from Test1%kind=custom, + # 1 from Test1%kind=default + # 0 from Test1%kind=nodeps assert in_degree(deps, Node('Test0', 'sys0:p0', 'e0')) == 12 - # 4 from Test1_fully, - # 2 from Test1_by_part, - # 1 from Test1_by_case, - # 2 from Test1_any, - # 0 from Test1_all, - # 0 from Test1_custom, - # 1 from Test1_default - # 0 from Test1_nodeps + # 4 from Test1%kind=fully, + # 2 from Test1%kind=by_part, + # 1 from Test1%kind=by_case, + # 2 from Test1%kind=any, + # 0 from Test1%kind=all, + # 0 from Test1%kind=custom, + # 1 from Test1%kind=default + # 0 from Test1%kind=nodeps assert in_degree(deps, Node('Test0', 'sys0:p1', 'e0')) == 10 - # 4 from Test1_fully, - # 2 from Test1_by_part, - # 1 from Test1_by_case, - # 4 from Test1_any, - # 0 from Test1_all, - # 0 from Test1_custom, - # 1 from Test1_default - # 0 from Test1_nodeps + # 4 from Test1%kind=fully, + # 2 from Test1%kind=by_part, + # 1 from Test1%kind=by_case, + # 4 from Test1%kind=any, + # 0 from Test1%kind=all, + # 0 from Test1%kind=custom, + # 1 from Test1%kind=default + # 0 from Test1%kind=nodeps assert in_degree(deps, Node('Test0', 'sys0:p0', 'e1')) == 12 - # 4 from Test1_fully, - # 2 from Test1_by_part, - # 1 from Test1_by_case, - # 4 from Test1_any, - # 0 from Test1_all, - # 1 from Test1_custom, - # 1 from Test1_default - # 0 from Test1_nodeps + # 4 from Test1%kind=fully, + # 2 from Test1%kind=by_part, + # 1 from Test1%kind=by_case, + # 4 from Test1%kind=any, + # 0 from Test1%kind=all, + # 1 from Test1%kind=custom, + # 1 from Test1%kind=default + # 0 from Test1%kind=nodeps assert in_degree(deps, Node('Test0', 'sys0:p1', 'e1')) == 13 # Pick a check to test getdep() - check_e0 = find_case('Test1_by_part', 'e0', 'p0', cases).check - check_e1 = find_case('Test1_by_part', 'e1', 'p0', cases).check + check_e0 = find_case('Test1%kind=by_part', 'e0', 'p0', cases).check + check_e1 = find_case('Test1%kind=by_part', 'e1', 'p0', cases).check with pytest.raises(DependencyError): check_e0.getdep('Test0', 'p0') diff --git a/unittests/test_loader.py b/unittests/test_loader.py index 3bda4dcadd..e22b40faa9 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -50,7 +50,7 @@ def test_load_all(loader_with_path): def test_load_new_syntax(loader): checks = loader.load_from_file( - 'unittests/resources/checks_unlisted/good.py' + 'unittests/resources/checks_unlisted/good.py', force=True ) assert 13 == len(checks) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 3d4af3cfcc..c733c91fea 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -10,6 +10,7 @@ import reframe as rfm import reframe.core.runtime as rt +import reframe.utility as util import reframe.utility.osext as osext import reframe.utility.sanity as sn import unittests.fixtures as fixtures @@ -29,10 +30,18 @@ def _run(test, partition, prgenv): @pytest.fixture -def HelloTest(): - from unittests.resources.checks.hellocheck import HelloTest - yield HelloTest - del sys.modules['unittests.resources.checks.hellocheck'] +def module_import(): + def _do_import(filename): + mod = util.import_module_from_file(filename, force=True) + return mod + + return _do_import + + +@pytest.fixture +def HelloTest(module_import): + mod = module_import('unittests/resources/checks/hellocheck.py') + return mod.HelloTest @pytest.fixture @@ -41,17 +50,15 @@ def hellotest(HelloTest): @pytest.fixture -def hellomaketest(): - from unittests.resources.checks.hellocheck_make import HelloMakeTest - yield HelloMakeTest - del sys.modules['unittests.resources.checks.hellocheck_make'] +def hellomaketest(module_import): + mod = module_import('unittests/resources/checks/hellocheck_make.py') + return mod.HelloMakeTest @pytest.fixture -def pinnedtest(): - from unittests.resources.checks.pinnedcheck import PinnedTest - yield PinnedTest - del sys.modules['unittests.resources.checks.pinnedcheck'] +def pinnedtest(module_import): + mod = module_import('unittests/resources/checks/pinnedcheck.py') + return mod.PinnedTest @pytest.fixture @@ -154,10 +161,6 @@ def __init__(self): self.name = 'T0' t0, t1 = T0(), T1() - assert t0 == t1 - assert hash(t0) == hash(t1) - - t1.name = 'T1' assert t0 != t1 assert hash(t0) != hash(t1) @@ -758,74 +761,8 @@ def setz(self, T0): assert t.z == 3 -def test_regression_test_name(): - class MyTest(rfm.RegressionTest): - def __init__(self, a, b): - self.a = a - self.b = b - - test = MyTest(1, 2) - assert os.path.abspath(os.path.dirname(__file__)) == test.prefix - assert 'test_regression_test_name..MyTest_1_2' == test.name - - -def test_strange_test_names(): - class C: - def __init__(self, a): - self.a = a - - def __repr__(self): - return 'C(%s)' % self.a - - class MyTest(rfm.RegressionTest): - def __init__(self, a, b): - self.a = a - self.b = b - - test = MyTest('(a*b+c)/12', C(33)) - assert ('test_strange_test_names..MyTest__a_b_c__12_C_33_' == - test.name) - - -def test_name_user_inheritance(): - class MyBaseTest(rfm.RegressionTest): - def __init__(self, a, b): - self.a = a - self.b = b - - class MyTest(MyBaseTest): - def __init__(self): - super().__init__(1, 2) - - test = MyTest() - assert 'test_name_user_inheritance..MyTest' == test.name - - -def test_name_runonly_test(): - class MyTest(rfm.RunOnlyRegressionTest): - def __init__(self, a, b): - self.a = a - self.b = b - - test = MyTest(1, 2) - assert os.path.abspath(os.path.dirname(__file__)) == test.prefix - assert 'test_name_runonly_test..MyTest_1_2' == test.name - - -def test_name_compileonly_test(): - class MyTest(rfm.CompileOnlyRegressionTest): - def __init__(self, a, b): - self.a = a - self.b = b - - test = MyTest(1, 2) - assert os.path.abspath(os.path.dirname(__file__)) == test.prefix - assert 'test_name_compileonly_test..MyTest_1_2' == test.name - - -def test_registration_of_tests(): - import unittests.resources.checks_unlisted.good as mod - +def test_registration_of_tests(module_import): + mod = module_import('unittests/resources/checks_unlisted/good.py') checks = mod._rfm_gettests() assert 13 == len(checks) assert [mod.MyBaseTest(0, 0), From f5c7ce9ada3673ea1eafd7a6d1dfef76e0e0c429 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 4 Mar 2021 22:29:11 +0100 Subject: [PATCH 3/8] Remove unused imports --- reframe/core/decorators.py | 1 - unittests/test_pipeline.py | 1 - 2 files changed, 2 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index bfaf6fea8d..6406b691c8 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -22,7 +22,6 @@ import reframe.utility.osext as osext from reframe.core.exceptions import ReframeSyntaxError, user_frame from reframe.core.logging import getlogger -from reframe.core.parameters import TestParam from reframe.core.pipeline import RegressionTest from reframe.utility.versioning import VersionValidator diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index c733c91fea..12fb4494a2 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -6,7 +6,6 @@ import os import pytest import re -import sys import reframe as rfm import reframe.core.runtime as rt From 87ad77bb74b0d93efc9a0b08c61e60cbad77143b Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 4 Mar 2021 22:33:02 +0100 Subject: [PATCH 4/8] Update default executable name --- reframe/core/pipeline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 583b474c75..ca175a9c9b 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -785,7 +785,7 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False, **kwargs): def __rfm_init__(self): self.descr = self.name - self.executable = os.path.join('.', self.name) + self.executable = os.path.join('.', self.unique_id) self._perfvalues = {} # Static directories of the regression check From 5854ea5455a93fc1da0eed6b12078138a78b6a4c Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 4 Mar 2021 23:58:44 +0100 Subject: [PATCH 5/8] Show parameter hash in test listings --- reframe/core/pipeline.py | 19 +++++++++++++++---- reframe/frontend/cli.py | 12 ++++++++++-- reframe/frontend/filters.py | 5 ++++- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index ca175a9c9b..e7b12971a2 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -829,11 +829,19 @@ def __rfm_init__(self): # Just an empty environment self._cdt_environ = env.Environment('__rfm_cdt_environ') + @property + def param_hash(self): + return self._param_hash + + @property + def param_hash_short(self): + return self._param_hash[:8] if self.param_hash else None + @property def unique_id(self): ret = type(self).__qualname__ - if self._param_hash: - ret += '_' + self._param_hash[:8] + if self.param_hash: + ret += '_' + self.param_hash_short return ret @@ -1018,11 +1026,14 @@ def info(self): method may be called at any point of the test's lifetime. ''' ret = self.name + if self.param_hash: + ret += f' (/{self.param_hash_short})' + if self.current_partition: - ret += ' on %s' % self.current_partition.fullname + ret += f' on {self.current_partition.fullname}' if self.current_environ: - ret += ' using %s' % self.current_environ.name + ret += f' using {self.current_environ.name}' return ret diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index c6a14459bd..f70275dd8c 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -55,9 +55,16 @@ def fmt_deps(): else: return '' + def fmt_name(check): + ret = f'- {check.name}' + if check.param_hash: + ret += f' (parameter hash: {check.param_hash_short})' + + return ret + location = inspect.getfile(type(check)) if not detailed: - return f'- {check.name} (found in {location!r})' + return fmt_name(check) if check.num_tasks > 0: node_alloc_scheme = (f'standard ({check.num_tasks} task(s) -- ' @@ -85,7 +92,8 @@ def fmt_deps(): ), 'Dependencies (actual)': fmt_deps() } - lines = [f'- {check.name}:'] + + lines = [fmt_name(check) + ':'] for prop, val in check_info.items(): lines.append(f' {prop}:') if isinstance(val, dict): diff --git a/reframe/frontend/filters.py b/reframe/frontend/filters.py index 3584e3d276..4fc425906a 100644 --- a/reframe/frontend/filters.py +++ b/reframe/frontend/filters.py @@ -19,7 +19,10 @@ def have_name(patt): regex = re_compile(patt) def _fn(case): - return regex.match(case.check.name) + if case.check.unique_id == patt: + return True + else: + return regex.match(case.check.name) return _fn From 25879fe14b1a15caa2f96628294c34b3f4d618fb Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 5 Mar 2021 12:57:31 +0100 Subject: [PATCH 6/8] WIP: Hash the whole test name --- reframe/core/pipeline.py | 8 ++++---- reframe/frontend/cli.py | 34 +++++++++++++++++++++++++++++----- reframe/frontend/filters.py | 5 +---- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index e7b12971a2..a7cc56f7b8 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -846,10 +846,10 @@ def unique_id(self): return ret def _set_param_hash(self, params): - obj = {} - for name, value, _ in params: - obj[name] = value - + obj = { + 'test': type(self).__qualname__, + 'params': {name: value for name, value, _ in params} + } h = hashlib.sha256() h.update(bytes(jsonext.dumps(obj), encoding='utf-8')) self._param_hash = h.hexdigest() diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index f70275dd8c..a1b8577590 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -58,7 +58,7 @@ def fmt_deps(): def fmt_name(check): ret = f'- {check.name}' if check.param_hash: - ret += f' (parameter hash: {check.param_hash_short})' + ret += f' (test hash: {check.param_hash_short})' return ret @@ -705,14 +705,38 @@ def print_infoline(param, value): testcases = testcases_all printer.verbose(f'Generated {len(testcases)} test case(s)') + def split_names(nameopt): + '''Splits names specified in `-n` and `-x` options to actual names + or hashes''' + names, hashes = [], [] + for n in nameopt: + if n.startswith('/'): + hashes.append(n[1:]) + else: + names.append(n) + + return names, hashes + + exclude_names, exclude_hashes = split_names(options.exclude_names) + names, hashes = split_names(options.names) + + # Filter test cases by hash + if exclude_hashes: + for h in exclude_hashes: + testcases = filter(filters.have_not_hash(h), testcases) + + if hashes: + for h in hashes: + testcases = filter(filters.have_hash(h), testcases) + # Filter test cases by name - if options.exclude_names: - for name in options.exclude_names: + if exclude_names: + for name in exclude_names: testcases = filter(filters.have_not_name(name), testcases) - if options.names: + if names: testcases = filter( - filters.have_name('|'.join(options.names)), testcases + filters.have_name('|'.join(names)), testcases ) testcases = list(testcases) diff --git a/reframe/frontend/filters.py b/reframe/frontend/filters.py index 4fc425906a..3584e3d276 100644 --- a/reframe/frontend/filters.py +++ b/reframe/frontend/filters.py @@ -19,10 +19,7 @@ def have_name(patt): regex = re_compile(patt) def _fn(case): - if case.check.unique_id == patt: - return True - else: - return regex.match(case.check.name) + return regex.match(case.check.name) return _fn From 23321f5922ec60008f13b2c8efaf568f8c10c327 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 5 Mar 2021 18:26:31 +0100 Subject: [PATCH 7/8] Fix unit tests --- reframe/core/parameters.py | 4 +- reframe/core/pipeline.py | 60 ++++----- reframe/frontend/cli.py | 9 +- reframe/frontend/filters.py | 4 + unittests/resources/checks/frontend_checks.py | 13 +- unittests/test_policies.py | 115 ++++++++++++++---- 6 files changed, 133 insertions(+), 72 deletions(-) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 515c5fd78c..f3c903db40 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -178,10 +178,10 @@ def inject(self, obj, cls=None, use_params=False): ''' # Set the values of the test parameters (if any) + injected = [] if use_params and self.params: try: # Consume the parameter space iterator - injected = [] param_values = next(self.unique_iter) for index, key in enumerate(self.params): setattr(obj, key, param_values[index]) @@ -202,6 +202,8 @@ def inject(self, obj, cls=None, use_params=False): for key in self.params: setattr(obj, key, None) + obj.params_inserted(injected) + def __iter__(self): '''Create a generator object to iterate over the parameter space diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index a7cc56f7b8..78b6774268 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -21,6 +21,7 @@ import numbers import os import shutil +import sys import reframe.core.environments as env import reframe.core.fields as fields @@ -741,7 +742,9 @@ def pipeline_hooks(cls): #: :type: boolean : :default: :class:`True` build_locally = variable(bool, value=True) - _param_hash = variable(str, type(None), value=None) + # The unique ID of the test: a SHA256 hash of the class name and the + # test's parameters if any + _unique_id = variable(bytes, type(None), value=None) def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) @@ -785,7 +788,7 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False, **kwargs): def __rfm_init__(self): self.descr = self.name - self.executable = os.path.join('.', self.unique_id) + self.executable = os.path.join('.', self.unique_name) self._perfvalues = {} # Static directories of the regression check @@ -829,30 +832,17 @@ def __rfm_init__(self): # Just an empty environment self._cdt_environ = env.Environment('__rfm_cdt_environ') - @property - def param_hash(self): - return self._param_hash - - @property - def param_hash_short(self): - return self._param_hash[:8] if self.param_hash else None - - @property - def unique_id(self): - ret = type(self).__qualname__ - if self.param_hash: - ret += '_' + self.param_hash_short - - return ret + def _set_unique_id(self, params): + if self._unique_id is not None: + return - def _set_param_hash(self, params): obj = { 'test': type(self).__qualname__, 'params': {name: value for name, value, _ in params} } h = hashlib.sha256() h.update(bytes(jsonext.dumps(obj), encoding='utf-8')) - self._param_hash = h.hexdigest() + self._unique_id = h.digest() def params_inserted(self, params): '''Callback that is called when all parameters have been injected in @@ -866,8 +856,8 @@ def params_inserted(self, params): :meta private: ''' - # Calculate the parameter hash - self._set_param_hash(params) + # Calculate the unique id + self._set_unique_id(params) # Generate the test name self.name = type(self).__qualname__ @@ -877,6 +867,22 @@ def params_inserted(self, params): self.name += f'%{name}={value}' + @property + def hash_long(self): + return self._unique_id.hex() + + @property + def hash_short(self): + return self.hash_long[:8] + + @property + def unique_name(self): + ret = type(self).__qualname__ + if not type(self).param_space.is_empty(): + ret += f'_{self.hash_short}' + + return ret + @property def current_environ(self): '''The programming environment that the regression test is currently @@ -1025,10 +1031,8 @@ def info(self): you use the :class:`RegressionTest`'s attributes, because this method may be called at any point of the test's lifetime. ''' - ret = self.name - if self.param_hash: - ret += f' (/{self.param_hash_short})' + ret = f'{self.name} (/{self.hash_short})' if self.current_partition: ret += f' on {self.current_partition.fullname}' @@ -1072,11 +1076,11 @@ def _setup_paths(self): runtime = rt.runtime() self._stagedir = runtime.make_stagedir( self.current_system.name, self._current_partition.name, - self._current_environ.name, self.unique_id + self._current_environ.name, self.unique_name ) self._outputdir = runtime.make_outputdir( self.current_system.name, self._current_partition.name, - self._current_environ.name, self.unique_id + self._current_environ.name, self.unique_name ) except OSError as e: raise PipelineError('failed to set up paths') from e @@ -1840,10 +1844,10 @@ def __eq__(self, other): if not isinstance(other, RegressionTest): return NotImplemented - return self.unique_id == other.unique_id + return self._unique_id == other._unique_id def __hash__(self): - return hash(self.unique_id) + return int.from_bytes(self._unique_id[:4], sys.byteorder) def __rfm_json_decode__(self, json): # 'tags' are decoded as list, so we convert them to a set diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index a1b8577590..73d11104f5 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -56,11 +56,7 @@ def fmt_deps(): return '' def fmt_name(check): - ret = f'- {check.name}' - if check.param_hash: - ret += f' (test hash: {check.param_hash_short})' - - return ret + return f'- {check.name} (/{check.hash_short})' location = inspect.getfile(type(check)) if not detailed: @@ -726,8 +722,7 @@ def split_names(nameopt): testcases = filter(filters.have_not_hash(h), testcases) if hashes: - for h in hashes: - testcases = filter(filters.have_hash(h), testcases) + testcases = filter(filters.have_hash(hashes), testcases) # Filter test cases by name if exclude_names: diff --git a/reframe/frontend/filters.py b/reframe/frontend/filters.py index 3584e3d276..68c0e3983b 100644 --- a/reframe/frontend/filters.py +++ b/reframe/frontend/filters.py @@ -15,6 +15,10 @@ def re_compile(patt): raise ReframeError(f'invalid regex: {patt!r}') +def have_hash(hashes): + pass + + def have_name(patt): regex = re_compile(patt) diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index 80e9ef0f17..9a6bc34df5 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -157,20 +157,14 @@ def fail(self): class SleepCheck(BaseFrontendCheck): - _next_id = 0 + sleep_time = parameter() - def __init__(self, sleep_time): + def __init__(self): super().__init__() - - # Simulate a parameterized test, so as to get a proper unique test id - self.params_inserted([('id', self._next_id, None), - ('sleep_time', sleep_time, None)]) - print(self.name, self.unique_id) self.sourcesdir = None - self.sleep_time = sleep_time self.executable = 'python3' self.executable_opts = [ - '-c "from time import sleep; sleep(%s)"' % sleep_time + '-c "from time import sleep; sleep(%s)"' % self.sleep_time ] print_timestamp = ( "python3 -c \"from datetime import datetime; " @@ -180,7 +174,6 @@ def __init__(self, sleep_time): self.sanity_patterns = sn.assert_found(r'.*', self.stdout) self.valid_systems = ['*'] self.valid_prog_environs = ['*'] - SleepCheck._next_id += 1 class SleepCheckPollFail(SleepCheck, special=True): diff --git a/unittests/test_policies.py b/unittests/test_policies.py index 5afc19197b..3b02779577 100644 --- a/unittests/test_policies.py +++ b/unittests/test_policies.py @@ -80,6 +80,25 @@ def _make_loader(check_search_path): return _make_loader +@pytest.fixture +def make_sleep_check(): + test_count = 0 + + def _do_make(cls, duration): + nonlocal test_count + + # We use a dummy parameter in the test, which we change with every + # instantiation, in order to make each test different from each other + class _SleepCheck(cls): + sleep_time = parameter([duration]) + dummy = parameter([test_count]) + + test_count += 1 + return _SleepCheck(_rfm_use_params=True) + + return _do_make + + @pytest.fixture def common_exec_ctx(temp_runtime): yield from temp_runtime(fixtures.TEST_CONFIG_FILE, 'generic') @@ -508,7 +527,8 @@ def _read_timestamps(tasks): return begin_stamps, end_stamps -def test_concurrency_unlimited(async_runner, make_cases, make_async_exec_ctx): +def test_concurrency_unlimited(async_runner, make_cases, + make_async_exec_ctx, make_sleep_check): num_checks = 3 # Trigger evaluation of the execution context @@ -516,7 +536,11 @@ def test_concurrency_unlimited(async_runner, make_cases, make_async_exec_ctx): next(ctx) runner, monitor = async_runner - runner.runall(make_cases([SleepCheck(.5) for i in range(num_checks)])) + runner.runall( + make_cases([ + make_sleep_check(SleepCheck, 0.5) for i in range(num_checks) + ]) + ) # Ensure that all tests were run and without failures. assert num_checks == runner.stats.num_cases() @@ -537,14 +561,19 @@ def test_concurrency_unlimited(async_runner, make_cases, make_async_exec_ctx): pytest.skip('the system seems too much loaded.') -def test_concurrency_limited(async_runner, make_cases, make_async_exec_ctx): +def test_concurrency_limited(async_runner, make_cases, + make_async_exec_ctx, make_sleep_check): # The number of checks must be <= 2*max_jobs. num_checks, max_jobs = 5, 3 ctx = make_async_exec_ctx(max_jobs) next(ctx) runner, monitor = async_runner - runner.runall(make_cases([SleepCheck(.5) for i in range(num_checks)])) + runner.runall( + make_cases([ + make_sleep_check(SleepCheck, 0.5) for i in range(num_checks) + ]) + ) # Ensure that all tests were run and without failures. assert num_checks == runner.stats.num_cases() @@ -580,13 +609,18 @@ def test_concurrency_limited(async_runner, make_cases, make_async_exec_ctx): pytest.skip('the system seems too loaded.') -def test_concurrency_none(async_runner, make_cases, make_async_exec_ctx): +def test_concurrency_none(async_runner, make_cases, + make_async_exec_ctx, make_sleep_check): num_checks = 3 ctx = make_async_exec_ctx(1) next(ctx) runner, monitor = async_runner - runner.runall(make_cases([SleepCheck(.5) for i in range(num_checks)])) + runner.runall( + make_cases([ + make_sleep_check(SleepCheck, 0.5) for i in range(num_checks) + ]) + ) # Ensure that all tests were run and without failures. assert num_checks == runner.stats.num_cases() @@ -603,6 +637,7 @@ def test_concurrency_none(async_runner, make_cases, make_async_exec_ctx): # (e.g. begin[1] > end[0]). begin_after_end = (b > e for b, e in zip(begin_stamps[1:], end_stamps[:-1])) + print(list(zip(begin_stamps, end_stamps))) assert all(begin_after_end) @@ -622,22 +657,26 @@ def assert_interrupted_run(runner): def test_kbd_interrupt_in_wait_with_concurrency(async_runner, make_cases, - make_async_exec_ctx): + make_async_exec_ctx, + make_sleep_check): ctx = make_async_exec_ctx(4) next(ctx) runner, _ = async_runner with pytest.raises(KeyboardInterrupt): runner.runall(make_cases([ - KeyboardInterruptCheck(), SleepCheck(10), - SleepCheck(10), SleepCheck(10) + KeyboardInterruptCheck(), + make_sleep_check(SleepCheck, 10), + make_sleep_check(SleepCheck, 10), + make_sleep_check(SleepCheck, 10) ])) assert_interrupted_run(runner) def test_kbd_interrupt_in_wait_with_limited_concurrency( - async_runner, make_cases, make_async_exec_ctx): + async_runner, make_cases, make_async_exec_ctx, make_sleep_check +): # The general idea for this test is to allow enough time for all the # four checks to be submitted and at the same time we need the # KeyboardInterruptCheck to finish first (the corresponding wait should @@ -649,22 +688,27 @@ def test_kbd_interrupt_in_wait_with_limited_concurrency( runner, _ = async_runner with pytest.raises(KeyboardInterrupt): runner.runall(make_cases([ - KeyboardInterruptCheck(), SleepCheck(10), - SleepCheck(10), SleepCheck(10) + KeyboardInterruptCheck(), + make_sleep_check(SleepCheck, 10), + make_sleep_check(SleepCheck, 10), + make_sleep_check(SleepCheck, 10) ])) assert_interrupted_run(runner) def test_kbd_interrupt_in_setup_with_concurrency(async_runner, make_cases, - make_async_exec_ctx): + make_async_exec_ctx, + make_sleep_check): ctx = make_async_exec_ctx(4) next(ctx) runner, _ = async_runner with pytest.raises(KeyboardInterrupt): runner.runall(make_cases([ - SleepCheck(1), SleepCheck(1), SleepCheck(1), + make_sleep_check(SleepCheck, 1), + make_sleep_check(SleepCheck, 1), + make_sleep_check(SleepCheck, 1), KeyboardInterruptCheck(phase='setup') ])) @@ -672,14 +716,17 @@ def test_kbd_interrupt_in_setup_with_concurrency(async_runner, make_cases, def test_kbd_interrupt_in_setup_with_limited_concurrency( - async_runner, make_cases, make_async_exec_ctx): + async_runner, make_cases, make_async_exec_ctx, make_sleep_check +): ctx = make_async_exec_ctx(2) next(ctx) runner, _ = async_runner with pytest.raises(KeyboardInterrupt): runner.runall(make_cases([ - SleepCheck(1), SleepCheck(1), SleepCheck(1), + make_sleep_check(SleepCheck, 1), + make_sleep_check(SleepCheck, 1), + make_sleep_check(SleepCheck, 1), KeyboardInterruptCheck(phase='setup') ])) @@ -687,14 +734,18 @@ def test_kbd_interrupt_in_setup_with_limited_concurrency( def test_run_complete_fails_main_loop(async_runner, make_cases, - make_async_exec_ctx): + make_async_exec_ctx, + make_sleep_check): ctx = make_async_exec_ctx(1) next(ctx) runner, _ = async_runner num_checks = 3 - runner.runall(make_cases([SleepCheckPollFail(10), - SleepCheck(0.1), SleepCheckPollFail(10)])) + runner.runall(make_cases([ + make_sleep_check(SleepCheckPollFail, 10), + make_sleep_check(SleepCheck, 0.1), + make_sleep_check(SleepCheckPollFail, 10), + ])) assert_runall(runner) stats = runner.stats assert stats.num_cases() == num_checks @@ -707,14 +758,18 @@ def test_run_complete_fails_main_loop(async_runner, make_cases, def test_run_complete_fails_busy_loop(async_runner, make_cases, - make_async_exec_ctx): + make_async_exec_ctx, + make_sleep_check): ctx = make_async_exec_ctx(1) next(ctx) runner, _ = async_runner num_checks = 3 - runner.runall(make_cases([SleepCheckPollFailLate(1), - SleepCheck(0.1), SleepCheckPollFailLate(0.5)])) + runner.runall(make_cases([ + make_sleep_check(SleepCheckPollFailLate, 1), + make_sleep_check(SleepCheck, 0.1), + make_sleep_check(SleepCheckPollFailLate, 0.5), + ])) assert_runall(runner) stats = runner.stats assert stats.num_cases() == num_checks @@ -727,13 +782,17 @@ def test_run_complete_fails_busy_loop(async_runner, make_cases, def test_compile_fail_reschedule_main_loop(async_runner, make_cases, - make_async_exec_ctx): + make_async_exec_ctx, + make_sleep_check): ctx = make_async_exec_ctx(1) next(ctx) runner, _ = async_runner num_checks = 2 - runner.runall(make_cases([SleepCheckPollFail(.1), CompileFailureCheck()])) + runner.runall(make_cases([ + make_sleep_check(SleepCheckPollFail, .1), + CompileFailureCheck() + ])) stats = runner.stats assert num_checks == stats.num_cases() @@ -742,14 +801,18 @@ def test_compile_fail_reschedule_main_loop(async_runner, make_cases, def test_compile_fail_reschedule_busy_loop(async_runner, make_cases, - make_async_exec_ctx): + make_async_exec_ctx, + make_sleep_check): ctx = make_async_exec_ctx(1) next(ctx) runner, _ = async_runner num_checks = 2 runner.runall( - make_cases([SleepCheckPollFailLate(1.5), CompileFailureCheck()]) + make_cases([ + make_sleep_check(SleepCheckPollFailLate, 1.5), + CompileFailureCheck() + ]) ) stats = runner.stats assert num_checks == stats.num_cases() From 44bd2dbbb3f5860513314581c2b8c4f48fc68e54 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 6 Mar 2021 00:47:12 +0100 Subject: [PATCH 8/8] Select tests by hash --- reframe/frontend/cli.py | 5 ++++- reframe/frontend/filters.py | 9 +++++++-- unittests/resources/checks/hellocheck.py | 1 - unittests/test_cli.py | 14 +++++++++++++- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 73d11104f5..6fd4f71697 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -72,6 +72,7 @@ def fmt_name(check): check_info = { 'Description': check.descr, + 'Hash': check.hash_long, 'Environment modules': fmt_list(check.modules), 'Location': location, 'Maintainers': fmt_list(check.maintainers), @@ -722,7 +723,9 @@ def split_names(nameopt): testcases = filter(filters.have_not_hash(h), testcases) if hashes: - testcases = filter(filters.have_hash(hashes), testcases) + testcases = filter( + filters.have_hash('|'.join(f'^{h}' for h in hashes)), testcases + ) # Filter test cases by name if exclude_names: diff --git a/reframe/frontend/filters.py b/reframe/frontend/filters.py index 68c0e3983b..7db2edd7e1 100644 --- a/reframe/frontend/filters.py +++ b/reframe/frontend/filters.py @@ -15,8 +15,13 @@ def re_compile(patt): raise ReframeError(f'invalid regex: {patt!r}') -def have_hash(hashes): - pass +def have_hash(patt): + regex = re_compile(patt) + + def _fn(case): + return regex.match(case.check.hash_long) + + return _fn def have_name(patt): diff --git a/unittests/resources/checks/hellocheck.py b/unittests/resources/checks/hellocheck.py index a78efd8023..abb73a93c0 100644 --- a/unittests/resources/checks/hellocheck.py +++ b/unittests/resources/checks/hellocheck.py @@ -10,7 +10,6 @@ @rfm.simple_test class HelloTest(rfm.RegressionTest): def __init__(self): - self.name = 'hellocheck' self.descr = 'C Hello World test' # All available systems are supported diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 5f789c443e..8306efb116 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -553,7 +553,7 @@ def test_filtering_multiple_criteria(run_reframe): returncode, stdout, stderr = run_reframe( checkpath=['unittests/resources/checks'], action='list', - more_options=['-t', 'foo', '-n', 'hellocheck', + more_options=['-t', 'foo', '-n', 'HelloTest', '--ignore-check-conflicts'] ) assert 'Traceback' not in stdout @@ -562,6 +562,18 @@ def test_filtering_multiple_criteria(run_reframe): assert returncode == 0 +def test_filtering_by_hash(run_reframe): + returncode, stdout, stderr = run_reframe( + checkpath=['unittests/resources/checks'], + action='list', + more_options=['-n', '/fc3f678d'] + ) + assert 'Traceback' not in stdout + assert 'Traceback' not in stderr + assert 'Found 1 check(s)' in stdout + assert returncode == 0 + + def test_show_config_all(run_reframe): # Just make sure that this option does not make the frontend crash returncode, stdout, stderr = run_reframe(