From 92bbaf602d398158bc5ce651204650e7b6e05ad9 Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Fri, 3 Apr 2020 16:00:56 +0000 Subject: [PATCH 01/13] adding the option to specify *: as a list of system --- reframe/core/pipeline.py | 8 ++++++-- unittests/test_pipeline.py | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 4748f00469..5f15fe797c 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -158,7 +158,7 @@ class RegressionTest(metaclass=RegressionTestMeta): typ.List[str]) #: List of systems supported by this test. - #: The general syntax for systems is ``[:[: is found + generic_partition_name = '*:%s' % (partition_name) - return partition_name in self.valid_systems + + return any(p in self.valid_systems for p in + [partition_name, generic_partition_name]) def supports_environ(self, env_name): if '*' in self.valid_prog_environs: diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index b5ce62c24f..f1ea85ff0c 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -287,6 +287,12 @@ def test_supports_system(self): assert not test.supports_system('testsys:gpu') assert not test.supports_system('testsys:login') + test.valid_systems = ['*:gpu'] + assert test.supports_system('testsys:gpu') + assert test.supports_system('foo:gpu') + assert not test.supports_system('testsys:cpu') + assert not test.supports_system('testsys:login') + def test_supports_environ(self): test = self.loader.load_from_file( 'unittests/resources/checks/hellocheck.py')[0] From cca75308e0883e692dea109a717c2c0eb942762c Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Fri, 3 Apr 2020 16:04:25 +0000 Subject: [PATCH 02/13] Deleted extra blank line --- reframe/core/pipeline.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 5f15fe797c..06b2acfad4 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -963,7 +963,6 @@ def supports_system(self, partition_name): # Check if *: is found generic_partition_name = '*:%s' % (partition_name) - return any(p in self.valid_systems for p in [partition_name, generic_partition_name]) From 32a487d84a5bf2c409998597d751562e33da34ea Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Fri, 3 Apr 2020 16:10:01 +0000 Subject: [PATCH 03/13] Rewritten to avoid generic_partition_name being used before assignment --- reframe/core/pipeline.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 06b2acfad4..1c3ca638c0 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -960,11 +960,11 @@ def supports_system(self, partition_name): if partition_name.find(':') == -1: partition_name = '%s:%s' % (self.current_system.name, partition_name) - # Check if *: is found - generic_partition_name = '*:%s' % (partition_name) - return any(p in self.valid_systems for p in - [partition_name, generic_partition_name]) + if '*:%s' % (partition_name) in self.valid_systems: + return True + + return partition_name in self.valid_systems def supports_environ(self, env_name): if '*' in self.valid_prog_environs: From 64445594489d29e36a9077622b3cbb012c99662a Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Fri, 3 Apr 2020 16:21:50 +0000 Subject: [PATCH 04/13] Refactored to cover all cases --- reframe/core/pipeline.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 1c3ca638c0..9f44ff13a6 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -957,14 +957,18 @@ def supports_system(self, partition_name): return True # Check if this is a relative name - if partition_name.find(':') == -1: - partition_name = '%s:%s' % (self.current_system.name, - partition_name) + if partition_name.find(':') != -1: + system_partition = partition_name.split(':') + system_name = system_partition[0] + partition_name = system_partition[1] + else: + system_name = self.current_system.name + partition_name = partition_name - if '*:%s' % (partition_name) in self.valid_systems: - return True + specific = '%s:%s' % (system_name, partition_name) + generic = '*:%s' % partition_name - return partition_name in self.valid_systems + return any(p in self.valid_systems for p in [specific, generic]) def supports_environ(self, env_name): if '*' in self.valid_prog_environs: From 831c7ea3fb00851fc0d30e51258b9283bdfbbfb0 Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Mon, 6 Apr 2020 17:36:11 +0000 Subject: [PATCH 05/13] addressing suggested changes --- reframe/core/pipeline.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 9f44ff13a6..7760fee8e5 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -958,9 +958,7 @@ def supports_system(self, partition_name): # Check if this is a relative name if partition_name.find(':') != -1: - system_partition = partition_name.split(':') - system_name = system_partition[0] - partition_name = system_partition[1] + system_partition, partition_name = partition_name.split(':') else: system_name = self.current_system.name partition_name = partition_name @@ -968,7 +966,7 @@ def supports_system(self, partition_name): specific = '%s:%s' % (system_name, partition_name) generic = '*:%s' % partition_name - return any(p in self.valid_systems for p in [specific, generic]) + return specific in self.valid_systems or generic in self.valid_systems def supports_environ(self, env_name): if '*' in self.valid_prog_environs: From 75ea3984a698cd8a39cfaa9d6e2ac275ca46affe Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Tue, 7 Apr 2020 14:19:22 +0000 Subject: [PATCH 06/13] generalized supports_system to support more formats --- reframe/core/pipeline.py | 27 +++++++++++++++------------ unittests/test_pipeline.py | 10 ++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 7760fee8e5..c30994ac4e 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -158,7 +158,8 @@ class RegressionTest(metaclass=RegressionTestMeta): typ.List[str]) #: List of systems supported by this test. - #: The general syntax for systems is ``[:[:]``. + #: Other supported options are: ``*``, ``*:*``, ``*:``, ``:*`` #: #: :type: :class:`List[str]` #: :default: ``[]`` @@ -949,24 +950,26 @@ def info(self): return ret - def supports_system(self, partition_name): - if '*' in self.valid_systems: - return True + def supports_system(self, name): if self.current_system.name in self.valid_systems: return True + names_to_test = ['*'] + names_to_test.append(self.current_system.name) + names_to_test.append('%s:*' % self.current_system.name) + # Check if this is a relative name - if partition_name.find(':') != -1: - system_partition, partition_name = partition_name.split(':') + if name.find(':') != -1: + system_name, partition_name = name.split(':') + names_to_test.append('%s:*' % system_name) + names_to_test.append('*:%s' % partition_name) + names_to_test.append('%s:%s' % (system_name, partition_name)) else: - system_name = self.current_system.name - partition_name = partition_name - - specific = '%s:%s' % (system_name, partition_name) - generic = '*:%s' % partition_name + names_to_test.append('*:%s' % name) + names_to_test.append('%s:%s' % (self.current_system.name, name) - return specific in self.valid_systems or generic in self.valid_systems + return any(n in self.valid_systems for n in names_to_test) def supports_environ(self, env_name): if '*' in self.valid_prog_environs: diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index f1ea85ff0c..87e630497d 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -293,6 +293,16 @@ def test_supports_system(self): assert not test.supports_system('testsys:cpu') assert not test.supports_system('testsys:login') + test.valid_systems = ['*:*'] + assert test.supports_system('gpu') + assert test.supports_system('login') + assert test.supports_system('testsys:gpu') + assert test.supports_system('testsys:login') + + test.valid_systems = ['testsys:*'] + assert test.supports_system('testsys:login') + assert not test.supports_system('foo:gpu') + def test_supports_environ(self): test = self.loader.load_from_file( 'unittests/resources/checks/hellocheck.py')[0] From 77db99ab9b25d6bd369b6b425883e5664c6e43c9 Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Tue, 7 Apr 2020 14:22:09 +0000 Subject: [PATCH 07/13] fixed missing parenthesis --- 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 c30994ac4e..97d4083fdc 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -967,7 +967,7 @@ def supports_system(self, name): names_to_test.append('%s:%s' % (system_name, partition_name)) else: names_to_test.append('*:%s' % name) - names_to_test.append('%s:%s' % (self.current_system.name, name) + names_to_test.append('%s:%s' % (self.current_system.name, name)) return any(n in self.valid_systems for n in names_to_test) From b6776113df42e59a6d2f242d9d1d1b635424d6cd Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Tue, 7 Apr 2020 14:25:16 +0000 Subject: [PATCH 08/13] adding forgotten variant *:* --- 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 97d4083fdc..c33f8c7c4b 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -955,7 +955,7 @@ def supports_system(self, name): if self.current_system.name in self.valid_systems: return True - names_to_test = ['*'] + names_to_test = ['*', '*:*'] names_to_test.append(self.current_system.name) names_to_test.append('%s:*' % self.current_system.name) From 80d3c72b2096dd56dae9a7bd5a76e55d21bb3034 Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Tue, 7 Apr 2020 14:29:04 +0000 Subject: [PATCH 09/13] fixing buggy over-generalization --- reframe/core/pipeline.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index c33f8c7c4b..75b64e9453 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -962,8 +962,6 @@ def supports_system(self, name): # Check if this is a relative name if name.find(':') != -1: system_name, partition_name = name.split(':') - names_to_test.append('%s:*' % system_name) - names_to_test.append('*:%s' % partition_name) names_to_test.append('%s:%s' % (system_name, partition_name)) else: names_to_test.append('*:%s' % name) From b4fbdc4d936c2ed2754b396b0e2e073c123c51e5 Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Tue, 7 Apr 2020 15:05:52 +0000 Subject: [PATCH 10/13] streamlined the logic and ensured all tests pass --- reframe/core/pipeline.py | 18 +++++++----------- unittests/test_pipeline.py | 1 + 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 75b64e9453..49bdd61cf1 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -951,21 +951,17 @@ def info(self): return ret def supports_system(self, name): - - if self.current_system.name in self.valid_systems: - return True - - names_to_test = ['*', '*:*'] - names_to_test.append(self.current_system.name) - names_to_test.append('%s:*' % self.current_system.name) - # Check if this is a relative name if name.find(':') != -1: system_name, partition_name = name.split(':') - names_to_test.append('%s:%s' % (system_name, partition_name)) else: - names_to_test.append('*:%s' % name) - names_to_test.append('%s:%s' % (self.current_system.name, name)) + system_name, partition_name = self.current_system.name, name + + names_to_test = ['*', '*:*'] + names_to_test.append(system_name) + names_to_test.append('%s:*' % system_name) + names_to_test.append('*:%s' % partition_name) + names_to_test.append('%s:%s' % (system_name, partition_name)) return any(n in self.valid_systems for n in names_to_test) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 87e630497d..af88ab7fab 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -301,6 +301,7 @@ def test_supports_system(self): test.valid_systems = ['testsys:*'] assert test.supports_system('testsys:login') + assert test.supports_system('gpu') assert not test.supports_system('foo:gpu') def test_supports_environ(self): From fe7283ddb3bd014e001f4464f359c5e4b80ada9c Mon Sep 17 00:00:00 2001 From: Maxime Boissonneault Date: Thu, 9 Apr 2020 14:23:47 +0000 Subject: [PATCH 11/13] addressing most of Vasilio's comments --- reframe/core/pipeline.py | 17 +++++++---------- unittests/test_pipeline.py | 12 ++++++------ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 49bdd61cf1..858bae8aac 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -159,7 +159,8 @@ class RegressionTest(metaclass=RegressionTestMeta): #: List of systems supported by this test. #: The general syntax for systems is ``[:]``. - #: Other supported options are: ``*``, ``*:*``, ``*:``, ``:*`` + #: Both and accept the value ``*`` to mean any value. + #: ``*`` is an alias of ``*:*`` #: #: :type: :class:`List[str]` #: :default: ``[]`` @@ -951,19 +952,15 @@ def info(self): return ret def supports_system(self, name): - # Check if this is a relative name if name.find(':') != -1: - system_name, partition_name = name.split(':') + system, partition = name.split(':') else: - system_name, partition_name = self.current_system.name, name + system, partition = self.current_system.name, name - names_to_test = ['*', '*:*'] - names_to_test.append(system_name) - names_to_test.append('%s:*' % system_name) - names_to_test.append('*:%s' % partition_name) - names_to_test.append('%s:%s' % (system_name, partition_name)) + valid_matches = ['*', '*:*', system, f'{system}:*', + f'*:{partition}', f'{system}:{partition}'] - return any(n in self.valid_systems for n in names_to_test) + return any(n in self.valid_systems for n in valid_matches) def supports_environ(self, env_name): if '*' in self.valid_prog_environs: diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index af88ab7fab..e0c010921a 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -263,6 +263,12 @@ def test_supports_system(self): assert test.supports_system('testsys:gpu') assert test.supports_system('testsys:login') + test.valid_systems = ['*:*'] + assert test.supports_system('gpu') + assert test.supports_system('login') + assert test.supports_system('testsys:gpu') + assert test.supports_system('testsys:login') + test.valid_systems = ['testsys'] assert test.supports_system('gpu') assert test.supports_system('login') @@ -293,12 +299,6 @@ def test_supports_system(self): assert not test.supports_system('testsys:cpu') assert not test.supports_system('testsys:login') - test.valid_systems = ['*:*'] - assert test.supports_system('gpu') - assert test.supports_system('login') - assert test.supports_system('testsys:gpu') - assert test.supports_system('testsys:login') - test.valid_systems = ['testsys:*'] assert test.supports_system('testsys:login') assert test.supports_system('gpu') From 7e1a21515eb92a145e08acf1e58650ed01f32993 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Tue, 14 Apr 2020 14:12:38 +0200 Subject: [PATCH 12/13] Enforce stricter rules for the system names in valid_systems --- reframe/core/pipeline.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 858bae8aac..25c6d29d88 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -164,7 +164,8 @@ class RegressionTest(metaclass=RegressionTestMeta): #: #: :type: :class:`List[str]` #: :default: ``[]`` - valid_systems = fields.TypedField('valid_systems', typ.List[str]) + valid_systems = fields.TypedField('valid_systems', + typ.List[typ.Str[r'^[^:]+(:[^:]+)?$']]) #: A detailed description of the test. #: From d59940c65b1965667067ffd20654293bc623e008 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Tue, 14 Apr 2020 14:39:21 +0200 Subject: [PATCH 13/13] Revert "Enforce stricter rules for the system names in valid_systems" This reverts commit 7e1a21515eb92a145e08acf1e58650ed01f32993. --- reframe/core/pipeline.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 25c6d29d88..858bae8aac 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -164,8 +164,7 @@ class RegressionTest(metaclass=RegressionTestMeta): #: #: :type: :class:`List[str]` #: :default: ``[]`` - valid_systems = fields.TypedField('valid_systems', - typ.List[typ.Str[r'^[^:]+(:[^:]+)?$']]) + valid_systems = fields.TypedField('valid_systems', typ.List[str]) #: A detailed description of the test. #: