From 25410e76f53299881df03b53e2427d468f6ad24c Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Wed, 21 Aug 2024 14:52:40 +0200 Subject: [PATCH] add filter for accelerators and pytests --- tests/test_tools_filter.py | 216 +++++++++++++++++++++++++++++++++---- tools/filter.py | 105 +++++++++++++----- 2 files changed, 275 insertions(+), 46 deletions(-) diff --git a/tests/test_tools_filter.py b/tests/test_tools_filter.py index 3cf4c699..b689aa60 100644 --- a/tests/test_tools_filter.py +++ b/tests/test_tools_filter.py @@ -10,12 +10,18 @@ # # Standard library imports +import copy # Third party imports (anything installed into the local Python environment) import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tools.filter import EESSIBotActionFilter, EESSIBotActionFilterError +from tools.filter import (COMPONENT_TOO_SHORT, + COMPONENT_UNKNOWN, + EESSIBotActionFilter, + EESSIBotActionFilterError, + FILTER_EMPTY_PATTERN, + FILTER_FORMAT_ERROR) def test_empty_action_filter(): @@ -25,6 +31,45 @@ def test_empty_action_filter(): assert expected == actual +def test_add_wellformed_filter_from_string(): + af = EESSIBotActionFilter("") + component = 'acc' + pattern = 'nvidia/cc80' + af.add_filter_from_string(f"{component}:{pattern}") + expected = f"accelerator:{pattern}" + actual = af.to_string() + assert expected == actual + + +def test_add_non_wellformed_filter_from_string(): + af = EESSIBotActionFilter("") + component1 = 'acc' + filter_string1 = f"{component1}" + with pytest.raises(Exception) as err1: + af.add_filter_from_string(filter_string1) + assert err1.type == EESSIBotActionFilterError + expected_msg1 = FILTER_FORMAT_ERROR.format(filter_string=filter_string1) + assert str(err1.value) == expected_msg1 + + component2 = 'a' + pattern2 = 'zen4' + filter_string2 = f"{component2}:{pattern2}" + with pytest.raises(Exception) as err2: + af.add_filter_from_string(filter_string2) + assert err2.type == EESSIBotActionFilterError + expected_msg2 = COMPONENT_TOO_SHORT.format(component=component2, pattern=pattern2) + assert str(err2.value) == expected_msg2 + + component3 = 'arc' + pattern3 = '' + filter_string3 = f"{component3}:{pattern3}" + with pytest.raises(Exception) as err3: + af.add_filter_from_string(filter_string3) + assert err3.type == EESSIBotActionFilterError + expected_msg3 = FILTER_EMPTY_PATTERN.format(filter_string=filter_string3) + assert str(err3.value) == expected_msg3 + + def test_add_single_action_filter(): af = EESSIBotActionFilter("") component = 'arch' @@ -42,6 +87,106 @@ def test_add_non_supported_component(): with pytest.raises(Exception) as err: af.add_filter(component, pattern) assert err.type == EESSIBotActionFilterError + expected_msg = COMPONENT_UNKNOWN.format(component=component, pattern=pattern) + assert str(err.value) == expected_msg + + +def test_add_too_short_supported_component(): + af = EESSIBotActionFilter("") + component = 'a' + pattern = '.*intel.*' + with pytest.raises(Exception) as err: + af.add_filter(component, pattern) + assert err.type == EESSIBotActionFilterError + expected_msg = COMPONENT_TOO_SHORT.format(component=component, pattern=pattern) + assert str(err.value) == expected_msg + + +# TODO tests for removing filters +@pytest.fixture +def complex_filter(): + af = EESSIBotActionFilter("") + component1 = 'arch' + pattern1 = '.*intel.*' + af.add_filter(component1, pattern1) + component2 = 'repo' + pattern2 = 'nessi.no-2022.*' + af.add_filter(component2, pattern2) + component3 = 'inst' + pattern3 = '[aA]' + af.add_filter(component3, pattern3) + yield af + + +def test_remove_existing_filter(complex_filter): + component1 = 'architecture' + pattern1 = '.*intel.*' + filter_string1 = f"{component1}:{pattern1}" + component2 = 'repository' + pattern2 = 'nessi.no-2022.*' + filter_string2 = f"{component2}:{pattern2}" + component3 = 'instance' + pattern3 = '[aA]' + filter_string3 = f"{component3}:{pattern3}" + + # remove last filter + org_filter = copy.deepcopy(complex_filter) + org_filter.remove_filter(component3, pattern3) + expected = filter_string1 + expected += f" {filter_string2}" + actual = org_filter.to_string() + assert expected == actual + + # remove second last filter + org_filter = copy.deepcopy(complex_filter) + org_filter.remove_filter(component2, pattern2) + expected = filter_string1 + expected += f" {filter_string3}" + actual = org_filter.to_string() + assert expected == actual + + # remove first filter + org_filter = copy.deepcopy(complex_filter) + org_filter.remove_filter(component1, pattern1) + expected = filter_string2 + expected += f" {filter_string3}" + actual = org_filter.to_string() + assert expected == actual + + +def test_remove_non_existing_filter(complex_filter): + component = 'accel' + pattern = 'amd/gfx90a' + + # remove non-existing filter + org_filter = copy.deepcopy(complex_filter) + org_filter.remove_filter(component, pattern) + org_filter_str = org_filter.to_string() + complex_filter_str = complex_filter.to_string() + assert org_filter_str == complex_filter_str + + +def test_remove_filter_errors(complex_filter): + component1 = 'ac' + pattern1 = 'amd/gfx90a' + component2 = 'operating_system' + pattern2 = 'linux' + + # remove filter using too short component name + org_filter = copy.deepcopy(complex_filter) + with pytest.raises(Exception) as err1: + org_filter.remove_filter(component1, pattern1) + assert err1.type == EESSIBotActionFilterError + expected_msg1 = COMPONENT_TOO_SHORT.format(component=component1, pattern=pattern1) + assert str(err1.value) == expected_msg1 + + # remove filter using unknown component name + org_filter = copy.deepcopy(complex_filter) + with pytest.raises(Exception) as err2: + org_filter.remove_filter(component2, pattern2) + assert err2.type == EESSIBotActionFilterError + expected_msg2 = COMPONENT_UNKNOWN.format(component=component2, pattern=pattern2) + assert str(err2.value) == expected_msg2 def test_check_matching_empty_filter(): @@ -71,21 +216,6 @@ def test_check_matching_simple_filter(): assert expected == actual -@pytest.fixture -def complex_filter(): - af = EESSIBotActionFilter("") - component1 = 'arch' - pattern1 = '.*intel.*' - af.add_filter(component1, pattern1) - component2 = 'repo' - pattern2 = 'nessi.no-2022.*' - af.add_filter(component2, pattern2) - component3 = 'i' - pattern3 = '[aA]' - af.add_filter(component3, pattern3) - yield af - - def test_create_complex_filter(complex_filter): expected = "architecture:.*intel.*" expected += " repository:nessi.no-2022.*" @@ -125,9 +255,9 @@ def test_non_match_architecture_repository_context(complex_filter): @pytest.fixture def arch_filter_slash_syntax(): af = EESSIBotActionFilter("") - component1 = 'arch' - pattern1 = '.*/intel/.*' - af.add_filter(component1, pattern1) + component = 'arch' + pattern = '.*/intel/.*' + af.add_filter(component, pattern) yield af @@ -146,9 +276,9 @@ def test_match_architecture_syntax_slash(arch_filter_slash_syntax): @pytest.fixture def arch_filter_dash_syntax(): af = EESSIBotActionFilter("") - component1 = 'arch' - pattern1 = '.*-intel-.*' - af.add_filter(component1, pattern1) + component = 'arch' + pattern = '.*-intel-.*' + af.add_filter(component, pattern) yield af @@ -162,3 +292,45 @@ def test_match_architecture_syntax_dash(arch_filter_dash_syntax): expected = True actual = arch_filter_dash_syntax.check_filters(context) assert expected == actual + + +@pytest.fixture +def accel_filter_slash_syntax(): + af = EESSIBotActionFilter("") + component = 'accel' + pattern = 'nvidia/.*' + af.add_filter(component, pattern) + yield af + + +def test_match_accelerator_syntax_slash(accel_filter_slash_syntax): + context = {"accelerator": "nvidia/cc70", "repository": "EESSI"} + expected = True + actual = accel_filter_slash_syntax.check_filters(context) + assert expected == actual + + context = {"accelerator": "nvidia=cc70"} + expected = True + actual = accel_filter_slash_syntax.check_filters(context) + assert expected == actual + + +@pytest.fixture +def accel_filter_equal_syntax(): + af = EESSIBotActionFilter("") + component = 'accel' + pattern = 'amd=gfx90a' + af.add_filter(component, pattern) + yield af + + +def test_match_accelerator_syntax_equal(accel_filter_equal_syntax): + context = {"accelerator": "amd=gfx90a", "repository": "EESSI"} + expected = True + actual = accel_filter_equal_syntax.check_filters(context) + assert expected == actual + + context = {"accelerator": "amd/gfx90a"} + expected = True + actual = accel_filter_equal_syntax.check_filters(context) + assert expected == actual diff --git a/tools/filter.py b/tools/filter.py index 8d791936..721cf3d7 100644 --- a/tools/filter.py +++ b/tools/filter.py @@ -23,11 +23,22 @@ # NOTE because one can use any prefix of one of the four components below to # define a filter, we need to make sure that no two filters share the same # prefix OR we have to change the handling of filters. +FILTER_COMPONENT_ACCEL = 'accelerator' FILTER_COMPONENT_ARCH = 'architecture' FILTER_COMPONENT_INST = 'instance' FILTER_COMPONENT_JOB = 'job' FILTER_COMPONENT_REPO = 'repository' -FILTER_COMPONENTS = [FILTER_COMPONENT_ARCH, FILTER_COMPONENT_INST, FILTER_COMPONENT_JOB, FILTER_COMPONENT_REPO] +FILTER_COMPONENTS = [FILTER_COMPONENT_ACCEL, + FILTER_COMPONENT_ARCH, + FILTER_COMPONENT_INST, + FILTER_COMPONENT_JOB, + FILTER_COMPONENT_REPO + ] + +COMPONENT_TOO_SHORT = "component in filter spec '{component}:{pattern}' is too short; must be 3 characters or longer" +COMPONENT_UNKNOWN = "unknown component={component} in {component}:{pattern}" +FILTER_EMPTY_PATTERN = "pattern in filter string '{filter_string}' is empty" +FILTER_FORMAT_ERROR = "filter string '{filter_string}' does not conform to format 'component:pattern'" Filter = namedtuple('Filter', ('component', 'pattern')) @@ -37,7 +48,12 @@ class EESSIBotActionFilterError(Exception): Exception to be raised when encountering an error in creating or adding a filter """ - pass + + def __init__(self, value): + self.value = value + + def __str__(self): + return self.value class EESSIBotActionFilter: @@ -87,8 +103,8 @@ def add_filter(self, component, pattern): Adds a filter given by a component and a pattern Args: - component (string): any prefix of known filter components (see - FILTER_COMPONENTS) + component (string): any prefix (min 3 characters long) of known filter + components (see FILTER_COMPONENTS) pattern (string): regular expression pattern that is applied to a string representing the component @@ -99,26 +115,36 @@ def add_filter(self, component, pattern): EESSIBotActionFilterError: raised if unknown component is provided as argument """ - # check if component is supported (i.e., it is a prefix of one of the - # elements in FILTER_COMPONENTS) + # check if component is supported + # - it is a 3+ character-long string, _and_ + # - it is a prefix of one of the elements in FILTER_COMPONENTS + if len(component) < 3: + msg = COMPONENT_TOO_SHORT.format(component=component, pattern=pattern) + log(msg) + raise EESSIBotActionFilterError(msg) full_component = None for cis in FILTER_COMPONENTS: # NOTE the below code assumes that no two filter share the same - # prefix (e.g., 'repository' and 'request' would have the same - # prefixes 'r' and 're') + # 3-character-long prefix (e.g., 'repository' and 'repeat' would + # have the same prefix 'rep') if cis.startswith(component): full_component = cis break if full_component: log(f"processing component {component}") # replace '-' with '/' in pattern when using 'architecture' filter - # component (done to make sure that values are comparable) + # component (done to make sure that values are comparable) if full_component == FILTER_COMPONENT_ARCH: pattern = pattern.replace('-', '/') + # replace '=' with '/' in pattern when using 'accelerator' filter + # component (done to make sure that values are comparable) + if full_component == FILTER_COMPONENT_ACCEL: + pattern = pattern.replace('=', '/') self.action_filters.append(Filter(full_component, pattern)) else: - log(f"component {component} is unknown") - raise EESSIBotActionFilterError(f"unknown component={component} in {component}:{pattern}") + msg = COMPONENT_UNKNOWN.format(component=component, pattern=pattern) + log(msg) + raise EESSIBotActionFilterError(msg) def add_filter_from_string(self, filter_string): """ @@ -136,11 +162,17 @@ def add_filter_from_string(self, filter_string): """ _filter_split = filter_string.split(':') if len(_filter_split) != 2: - log(f"filter string '{filter_string}' does not conform to format 'component:pattern'") - raise EESSIBotActionFilterError(f"filter '{filter_string}' does not conform to format 'component:pattern'") + msg = FILTER_FORMAT_ERROR.format(filter_string=filter_string) + log(msg) + raise EESSIBotActionFilterError(msg) + if len(_filter_split[0]) < 3: + msg = COMPONENT_TOO_SHORT.format(component=_filter_split[0], pattern=_filter_split[1]) + log(msg) + raise EESSIBotActionFilterError(msg) if len(_filter_split[1]) == 0: - log(f"pattern in filter string '{filter_string}' is empty") - raise EESSIBotActionFilterError(f"pattern in filter string '{filter_string}' is empty") + msg = FILTER_EMPTY_PATTERN.format(filter_string=filter_string) + log(msg) + raise EESSIBotActionFilterError(msg) self.add_filter(_filter_split[0], _filter_split[1]) def remove_filter(self, component, pattern): @@ -148,22 +180,44 @@ def remove_filter(self, component, pattern): Removes all elements matching the filter given by (component, pattern) Args: - component (string): any prefix of 'architecture', 'instance', 'job' or 'repository' + component (string): one of FILTER_COMPONENTS pattern (string): regex that is applied to a string representing the component Returns: None (implicitly) """ - index = 0 - for _filter in self.action_filters: + if len(component) < 3: + msg = COMPONENT_TOO_SHORT.format(component=component, pattern=pattern) + log(msg) + raise EESSIBotActionFilterError(msg) + full_component = None + for cis in FILTER_COMPONENTS: + # NOTE the below code assumes that no two filter share the same + # 3-character-long prefix (e.g., 'repository' and 'repeat' would + # have the same prefix 'rep') + if cis.startswith(component): + full_component = cis + break + if not full_component: + # the component provided as argument is not in the list of FILTER_COMPONENTS + msg = COMPONENT_UNKNOWN.format(component=component, pattern=pattern) + log(msg) + raise EESSIBotActionFilterError(msg) + + # need to traverse list from end or next elem after a removed item is + # skipped + num_filters = len(self.action_filters) + for idx in range(num_filters, 0, -1): + # idx runs from num_filters to 1; this needs to be corrected to + # num_filters-1 to 0 + index = idx - 1 # NOTE the below code assumes that no two filter share the same - # prefix (e.g., 'repository' and 'request' would have the same - # prefixes 'r' and 're') + # 3-character-long prefix (e.g., 'repository' and 'repeat' would + # have the same prefix 'rep') + _filter = self.action_filters[index] if _filter.component.startswith(component) and pattern == _filter.pattern: log(f"removing filter ({_filter.component}, {pattern})") self.action_filters.pop(index) - else: - index += 1 def to_string(self): """ @@ -184,8 +238,8 @@ def to_string(self): def check_filters(self, context): """ - Checks filters for a given context which is defined by one to four - components (architecture, instance, job, repository) + Checks filters for a given context which is defined by one to five + components (accelerator, architecture, instance, job, repository) Args: context (dict) : dictionary that maps components to their value @@ -217,6 +271,9 @@ def check_filters(self, context): # replace - with / in architecture component if af.component == FILTER_COMPONENT_ARCH: value = value.replace('-', '/') + # replace = with / in accelerator component + if af.component == FILTER_COMPONENT_ACCEL: + value = value.replace('=', '/') if re.search(af.pattern, value): # if the pattern of the filter matches check = True