From 07cddae4be8101c3c7bd4be817b80fec7fd6514a Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 11 Aug 2022 16:59:13 +0300 Subject: [PATCH 1/7] Set `_rfm_unique_name` in the metaclass also for standard tests --- reframe/core/meta.py | 2 ++ reframe/core/pipeline.py | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 3abae0df09..f3e151b825 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -423,6 +423,8 @@ def __call__(cls, *args, **kwargs): obj._rfm_unique_name = fixt_name obj._rfm_fixt_data = fixt_data obj._rfm_is_fixture = True + else: + obj._rfm_unique_name = cls.variant_name(variant_num) # Set the variables passed to the constructor for k, v in fixt_vars.items(): diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index f189ef3cc5..d7003f6778 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -959,9 +959,6 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False, @deferrable def __rfm_init__(self, prefix=None): - if not self.is_fixture() and not hasattr(self, '_rfm_unique_name'): - self._rfm_unique_name = type(self).variant_name(self.variant_num) - # Pass if descr is a required variable. if not hasattr(self, 'descr'): self.descr = self.display_name From 94d10aa7c0c33475e28fe169f2ab244452744c6a Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 11 Aug 2022 17:12:27 +0300 Subject: [PATCH 2/7] Change default value of the `descr` variable --- reframe/core/pipeline.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index d7003f6778..2d05843031 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -281,8 +281,11 @@ def pipeline_hooks(cls): #: A detailed description of the test. #: #: :type: :class:`str` - #: :default: ``self.display_name`` - descr = variable(str, loggable=True) + #: :default: ``''`` + #: + #: .. versionchanged:: 4.0 + #: The default value is now the empty string. + descr = variable(str, value='', loggable=True) #: The path to the source file or source directory of the test. #: @@ -959,10 +962,6 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False, @deferrable def __rfm_init__(self, prefix=None): - # Pass if descr is a required variable. - if not hasattr(self, 'descr'): - self.descr = self.display_name - self._perfvalues = {} # Static directories of the regression check From 12920c116eb65c5939bdff1e44e93e3a3945290b Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 12 Aug 2022 17:12:59 +0300 Subject: [PATCH 3/7] Introduce name hashes --- reframe/core/logging.py | 2 +- reframe/core/pipeline.py | 42 +++++++++++++++++++++----- reframe/frontend/cli.py | 12 ++++---- reframe/frontend/executors/__init__.py | 3 +- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/reframe/core/logging.py b/reframe/core/logging.py index 25e7d014c8..3a878a132d 100644 --- a/reframe/core/logging.py +++ b/reframe/core/logging.py @@ -148,7 +148,7 @@ def emit(self, record): raise LoggingError('logging failed') from e self.baseFilename = os.path.join(dirname, - f'{record.__rfm_check__.name}.log') + f'{record.__rfm_check__.fs_name}.log') self.stream = self._streams.get(self.baseFilename, None) super().emit(record) self._streams[self.baseFilename] = self.stream diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 2d05843031..358da41f5e 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -14,6 +14,7 @@ import glob +import hashlib import inspect import itertools import numbers @@ -1085,9 +1086,9 @@ def unique_name(self): def name(self): '''The name of the test. - This is an alias of :attr:`unique_name`. + This is an alias of :attr:`display_name`. ''' - return self.unique_name + return self.display_name @loggable @property @@ -1142,6 +1143,32 @@ def _format_params(cls, info, prefix=' %'): return self._rfm_display_name + @loggable + @property + def hashcode(self): + if hasattr(self, '_rfm_hashcode'): + return self._rfm_hashcode + + m = hashlib.sha256() + if self.is_fixture: + m.update(self.unique_name.encode('utf-8')) + else: + basename, *params = self.display_name.split(' %') + m.update(basename.encode('utf-8')) + for p in sorted(params): + m.update(p.encode('utf-8')) + + self._rfm_hashcode = m.hexdigest()[:8] + return self._rfm_hashcode + + @loggable + @property + def fs_name(self): + if self.unique_name != self.display_name: + return f'{type(self).__name__}_{self.hashcode}' + else: + return self.unique_name + @property def current_environ(self): '''The programming environment that the regression test is currently @@ -1393,7 +1420,7 @@ def info(self): method may be called at any point of the test's lifetime. ''' - ret = self.display_name + ret = f'{self.display_name} /{self.hashcode}' if self.current_partition: ret += f' @{self.current_partition.fullname}' @@ -1504,11 +1531,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_name + self._current_environ.name, self.fs_name ) self._outputdir = runtime.make_outputdir( self.current_system.name, self._current_partition.name, - self._current_environ.name, self.unique_name + self._current_environ.name, self.fs_name ) except OSError as e: raise PipelineError('failed to set up paths') from e @@ -1536,13 +1563,12 @@ def _create_job(self, name, force_local=False, **job_opts): **job_opts) def _setup_build_job(self, **job_opts): - self._build_job = self._create_job(f'rfm_{self.unique_name}_build', + self._build_job = self._create_job(f'rfm_build', self.local or self.build_locally, **job_opts) def _setup_run_job(self, **job_opts): - self._job = self._create_job(f'rfm_{self.unique_name}_job', - self.local, **job_opts) + self._job = self._create_job(f'rfm_job', self.local, **job_opts) def _setup_perf_logging(self): self._perf_logger = logging.getperflogger(self) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 0412fd6140..b475453241 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -73,6 +73,7 @@ def dep_lines(u, *, prefix, depth=0, lines=None, printed=None): unique_checks.add(v.check.unique_name) if depth: + name_info = f'{u.check.display_name} /{u.check.hashcode}' tc_info = '' details = '' if concretized: @@ -80,17 +81,16 @@ def dep_lines(u, *, prefix, depth=0, lines=None, printed=None): location = inspect.getfile(type(u.check)) if detailed: - details = f' [id: {u.check.unique_name}, file: {location!r}]' + details = f' [variant: {u.check.variant_num}, file: {location!r}]' - lines.append( - f'{prefix}^{u.check.display_name}{tc_info}{details}' - ) + lines.append(f'{prefix}^{name_info}{tc_info}{details}') return lines # We need the leaf test cases to be printed at the leftmost leaf_testcases = list(t for t in testcases if t.in_degree == 0) for t in leaf_testcases: + name_info = f'{t.check.display_name} /{t.check.hashcode}' tc_info = '' details = '' if concretized: @@ -98,12 +98,12 @@ def dep_lines(u, *, prefix, depth=0, lines=None, printed=None): location = inspect.getfile(type(t.check)) if detailed: - details = f' [id: {t.check.unique_name}, file: {location!r}]' + details = f' [variant: {t.check.variant_num}, file: {location!r}]' # if not concretized and t.check.name not in unique_checks: if concretized or (not concretized and t.check.unique_name not in unique_checks): - printer.info(f'- {t.check.display_name}{tc_info}{details}') + printer.info(f'- {name_info}{tc_info}{details}') if not t.check.is_fixture(): unique_checks.add(t.check.unique_name) diff --git a/reframe/frontend/executors/__init__.py b/reframe/frontend/executors/__init__.py index 267c95c837..b06a1abb99 100644 --- a/reframe/frontend/executors/__init__.py +++ b/reframe/frontend/executors/__init__.py @@ -416,9 +416,10 @@ def abort(self, cause=None): def info(self): '''Return an info string about this task.''' name = self.check.display_name + hashcode = self.check.hashcode part = self.testcase.partition.fullname env = self.testcase.environ.name - return f'{name} @{part}+{env}' + return f'{name} /{hashcode} @{part}+{env}' class TaskEventListener(abc.ABC): From 2abe370c600ea96735b8ddcdafb94d747bfb3a65 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 12 Aug 2022 18:01:30 +0300 Subject: [PATCH 4/7] Fix unit tests --- unittests/test_dependencies.py | 22 +++++++++++----------- unittests/test_fixtures.py | 4 +++- unittests/test_logging.py | 2 +- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/unittests/test_dependencies.py b/unittests/test_dependencies.py index e7378220ea..dec8ef5690 100644 --- a/unittests/test_dependencies.py +++ b/unittests/test_dependencies.py @@ -37,7 +37,7 @@ def __eq__(self, other): self.ename == other.ename) if isinstance(other, executors.TestCase): - return (self.cname == other.check.name and + return (self.cname == other.check.unique_name and self.pname == other.partition.fullname and self.ename == other.environ.name) @@ -450,9 +450,9 @@ def test_build_deps(loader, default_exec_ctx): check_e0._current_environ = Environment('e0') check_e1._current_environ = Environment('e1') - assert check_e0.getdep('Test0', 'e0', 'p0').name == 'Test0' - assert check_e0.getdep('Test0', 'e1', 'p0').name == 'Test0' - assert check_e1.getdep('Test0', 'e1', 'p0').name == 'Test0' + assert check_e0.getdep('Test0', 'e0', 'p0').unique_name == 'Test0' + assert check_e0.getdep('Test0', 'e1', 'p0').unique_name == 'Test0' + assert check_e1.getdep('Test0', 'e1', 'p0').unique_name == 'Test0' with pytest.raises(DependencyError): check_e0.getdep('TestX_deprecated', 'e0', 'p0') @@ -605,7 +605,7 @@ def test_skip_unresolved_deps(make_exec_ctx): ) assert len(skipped_cases) == 6 - skipped_tests = {c.check.name for c in skipped_cases} + skipped_tests = {c.check.unique_name for c in skipped_cases} assert skipped_tests == {'t1', 't2', 't3'} @@ -615,9 +615,9 @@ def assert_topological_order(cases, graph): tests = util.OrderedSet() for c in cases: check, part, env = c - cases_order.append((check.name, part.fullname, env.name)) - tests.add(check.name) - visited_tests.add(check.name) + cases_order.append((check.unique_name, part.fullname, env.name)) + tests.add(check.unique_name) + visited_tests.add(check.unique_name) # Assert that all dependencies of c have been visited before for d in graph[c]: @@ -625,7 +625,7 @@ def assert_topological_order(cases, graph): # dependency points outside the subgraph continue - assert d.check.name in visited_tests + assert d.check.unique_name in visited_tests # Check the order of systems and prog. environments # We are checking against all possible orderings @@ -739,7 +739,7 @@ def test_toposort(default_exec_ctx): cases_by_level = {} for c in cases: cases_by_level.setdefault(c.level, set()) - cases_by_level[c.level].add(c.check.name) + cases_by_level[c.level].add(c.check.unique_name) assert cases_by_level[0] == {'t0', 't5'} assert cases_by_level[1] == {'t1', 't6', 't7'} @@ -784,7 +784,7 @@ def test_toposort_subgraph(default_exec_ctx): cases_by_level = {} for c in cases: cases_by_level.setdefault(c.level, set()) - cases_by_level[c.level].add(c.check.name) + cases_by_level[c.level].add(c.check.unique_name) assert cases_by_level[1] == {'t3'} assert cases_by_level[2] == {'t4'} diff --git a/unittests/test_fixtures.py b/unittests/test_fixtures.py index 6563fb7ab8..f9160a5ac7 100644 --- a/unittests/test_fixtures.py +++ b/unittests/test_fixtures.py @@ -531,7 +531,9 @@ def test_overlapping_registries(ctx_sys, simple_test, # correctly. assert len(inst) == 1 assert inst[0].v == 2 - assert inst[0].name == list(diff_reg[simple_fixture().cls].keys())[0] + assert inst[0].unique_name == list( + diff_reg[simple_fixture().cls].keys() + )[0] assert inst[0].valid_systems == ['sys1:p0'] assert inst[0].valid_prog_environs == ['e0'] diff --git a/unittests/test_logging.py b/unittests/test_logging.py index 551b6b009c..60acf61aed 100644 --- a/unittests/test_logging.py +++ b/unittests/test_logging.py @@ -467,7 +467,7 @@ def test_logging_context_check(default_exec_ctx, logfile, fake_check): rlog.getlogger().error('error outside context') assert _found_in_logfile( - f'_FakeCheck_1: {sys.argv[0]}: error from context', logfile + f'_FakeCheck %param=10: {sys.argv[0]}: error from context', logfile ) assert _found_in_logfile( f'reframe: {sys.argv[0]}: error outside context', logfile From 2e5b6d47f726765086c9b26d7ed1a58484e32b7a Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 18 Aug 2022 13:42:34 +0300 Subject: [PATCH 5/7] Rename `fs_name` to `short_name` --- reframe/core/logging.py | 5 +++-- reframe/core/pipeline.py | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/reframe/core/logging.py b/reframe/core/logging.py index 3a878a132d..4dc5519351 100644 --- a/reframe/core/logging.py +++ b/reframe/core/logging.py @@ -147,8 +147,9 @@ def emit(self, record): except OSError as e: raise LoggingError('logging failed') from e - self.baseFilename = os.path.join(dirname, - f'{record.__rfm_check__.fs_name}.log') + self.baseFilename = os.path.join( + dirname, f'{record.__rfm_check__.short_name}.log' + ) self.stream = self._streams.get(self.baseFilename, None) super().emit(record) self._streams[self.baseFilename] = self.stream diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 358da41f5e..4911cff8df 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -1163,7 +1163,17 @@ def hashcode(self): @loggable @property - def fs_name(self): + def short_name(self): + '''A short version of the test's display name. + + The shortened version coincides with the :attr:`unique_name` for + simple tests and combines the test's class name and a hash code for + parameterised tests. + + .. versionadded:: 4.0.0 + + ''' + if self.unique_name != self.display_name: return f'{type(self).__name__}_{self.hashcode}' else: @@ -1531,11 +1541,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.fs_name + self._current_environ.name, self.short_name ) self._outputdir = runtime.make_outputdir( self.current_system.name, self._current_partition.name, - self._current_environ.name, self.fs_name + self._current_environ.name, self.short_name ) except OSError as e: raise PipelineError('failed to set up paths') from e From 177e567d2db3f2360f754edbb02576a629e2ca06 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 18 Aug 2022 14:09:36 +0300 Subject: [PATCH 6/7] Select tests by their hash --- reframe/frontend/filters.py | 16 ++++++++++++---- unittests/test_filters.py | 4 ++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/reframe/frontend/filters.py b/reframe/frontend/filters.py index efdb95c9ec..20441a18c5 100644 --- a/reframe/frontend/filters.py +++ b/reframe/frontend/filters.py @@ -41,13 +41,16 @@ def _fn(case): def have_any_name(names): rt = runtime() - exact_matches = [] + variant_matches = [] + hash_matches = [] regex_matches = [] for n in names: if '@' in n: test, _, variant = n.rpartition('@') if variant.isdigit(): - exact_matches.append((test, int(variant))) + variant_matches.append((test, int(variant))) + elif n.startswith('/'): + hash_matches.append(n[1:]) else: regex_matches.append(n) @@ -57,12 +60,17 @@ def have_any_name(names): regex = None def _fn(case): - # Check if we have an exact match - for m in exact_matches: + # Check the variant matches + for m in variant_matches: cls_name = type(case.check).__name__ if (cls_name, case.check.variant_num) == m: return True + # Check hash matches + for m in hash_matches: + if m == case.check.hashcode: + return True + display_name = case.check.display_name.replace(' ', '') if regex: return regex.match(display_name) diff --git a/unittests/test_filters.py b/unittests/test_filters.py index e052061eff..3ebbbd4013 100644 --- a/unittests/test_filters.py +++ b/unittests/test_filters.py @@ -65,6 +65,8 @@ def test_have_any_name(sample_cases): sample_cases) assert 2 == count_checks(filters.have_any_name(['(?i)check1|CHECK2']), sample_cases) + assert 1 == count_checks(filters.have_any_name(['/e2ae5cc6']), + sample_cases) def test_have_any_name_param_test(sample_param_cases): @@ -81,6 +83,8 @@ def test_have_any_name_param_test(sample_param_cases): sample_param_cases) assert 0 == count_checks(filters.have_any_name(['_X@12']), sample_param_cases) + assert 2 == count_checks(filters.have_any_name(['/023313dc', '/efddbc6c']), + sample_param_cases) assert 2 == count_checks(filters.have_any_name(['_X@0', '_X@1']), sample_param_cases) assert 12 == count_checks(filters.have_any_name(['_X@0', '_X.*']), From 6ccf6f3a5ad93b5edf0263dea1b727a7cc5d880e Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 18 Aug 2022 20:08:09 +0300 Subject: [PATCH 7/7] Document the test hash codes --- docs/config_reference.rst | 4 +-- docs/manpage.rst | 54 +++++++++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/docs/config_reference.rst b/docs/config_reference.rst index 7e70e4f367..9e1b6d3561 100644 --- a/docs/config_reference.rst +++ b/docs/config_reference.rst @@ -1007,9 +1007,9 @@ The additional properties for the ``filelog`` handler are the following: {basedir}/ system1/ partition1/ - test_name.log + test_short_name.log partition2/ - test_name.log + test_short_name.log ... system2/ ... diff --git a/docs/manpage.rst b/docs/manpage.rst index 8ee598740e..e4fbad1a63 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -125,6 +125,8 @@ This happens recursively so that if test ``T1`` depends on ``T2`` and ``T2`` dep If the special notation ``@`` is passed as the ``NAME`` argument, then an exact match will be performed selecting the variant ``variant_num`` of the test ``test_name``. + You may also select a test by its hash code using the notation ``/`` for the ``NAME`` argument. + .. note:: Fixtures cannot be selected. @@ -133,6 +135,11 @@ This happens recursively so that if test ``T1`` depends on ``T2`` and ``T2`` dep The option's behaviour was adapted and extended in order to work with the updated test naming scheme. + .. versionchanged:: 4.0.0 + + Support selecting tests by their hash code. + + .. option:: -p, --prgenv=NAME Filter tests by programming environment. @@ -953,26 +960,26 @@ Here is how this test is listed where the various components of the display name .. code-block:: console - - TestA %x=4 %l.foo=10 %t.p=2 - ^MyFixture %p=1 ~TestA_4_1 - ^MyFixture %p=2 ~TestA_4_1 - ^X %foo=10 ~generic:default+builtin - - TestA %x=3 %l.foo=10 %t.p=2 - ^MyFixture %p=1 ~TestA_3_1 - ^MyFixture %p=2 ~TestA_3_1 - ^X %foo=10 ~generic:default+builtin - - TestA %x=4 %l.foo=10 %t.p=1 - ^MyFixture %p=2 ~TestA_4_0 - ^MyFixture %p=1 ~TestA_4_0 - ^X %foo=10 ~generic:default+builtin - - TestA %x=3 %l.foo=10 %t.p=1 - ^MyFixture %p=2 ~TestA_3_0 - ^MyFixture %p=1 ~TestA_3_0 - ^X %foo=10 ~generic:default+builtin + - TestA %x=4 %l.foo=10 %t.p=2 /1c51609b + ^Myfixture %p=1 ~TestA_3 /f027ee75 + ^MyFixture %p=2 ~TestA_3 /830323a4 + ^X %foo=10 ~generic:default+builtin /7dae3cc5 + - TestA %x=3 %l.foo=10 %t.p=2 /707b752c + ^MyFixture %p=1 ~TestA_2 /02368516 + ^MyFixture %p=2 ~TestA_2 /854b99b5 + ^X %foo=10 ~generic:default+builtin /7dae3cc5 + - TestA %x=4 %l.foo=10 %t.p=1 /c65657d5 + ^MyFixture %p=2 ~TestA_1 /f0383f7f + ^MyFixture %p=1 ~TestA_1 /d07f4281 + ^X %foo=10 ~generic:default+builtin /7dae3cc5 + - TestA %x=3 %l.foo=10 %t.p=1 /1b9f44df + ^MyFixture %p=2 ~TestA_0 /b894ab05 + ^MyFixture %p=1 ~TestA_0 /ca376ca8 + ^X %foo=10 ~generic:default+builtin /7dae3cc5 Found 4 check(s) Display names may not always be unique. -In the following example: +Assume the following test: .. code-block:: python @@ -982,6 +989,19 @@ In the following example: This generates three different tests with different unique names, but their display name is the same for all: ``MyTest %p=1``. Notice that this example leads to a name conflict with the old naming scheme, since all tests would be named ``MyTest_1``. +Each test is also associated with a hash code that is derived from the test name, its parameters and their values. +As in the example listing above, the hash code of each test is printed with the :option:`-l` option and individual tests can be selected by their hash using the :option:`-n` option, e.g., ``-n /1c51609b``. +The stage and output directories, as well as the performance log file of the ``filelog`` `performance log handler `__ will use the hash code for the test-specific directories and files. +This might lead to conflicts for tests as the one above when executing them with the asynchronous execution policy, but ensures consistency of performance record files when parameter values are added to or deleted from a test parameter. +More specifically, the test's hash will not change if a new parameter value is added or deleted or even if the parameter values are shuffled. +Test variants on the other side are more volatile and can change with such changes. +Also users should not rely on how the variant numbers are assigned to a test, as this is an implementation detail. + + +.. versionchanged:: 4.0.0 + + A hash code is associated with each test. + -------------------------------------- Differences from the old naming scheme