diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 00000000..64e5de55 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,8 @@ +[run] +omit = + tests/* + */tests/* + */test/* + *test_*.py + */__pycache__/* + */venv/* diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index fb4e1882..d2f65276 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -40,14 +40,30 @@ jobs: python -m pip install pytest python -m pip install --upgrade flake8 - - name: Run test suite (without coverage) + - name: Run test suite run: | ./test.sh --verbose - - name: Run test suite (with coverage) + - name: Run coverage and create report run: | - python -m pip install pytest-cov - ./test.sh -q --cov=$PWD + python -m pip install coverage + PYTHON_SOURCES=$(find . -name "*.py" -not -path "tests/*" -not -path ".github/*" | tr '\n' ',' | sed 's/,$//') + echo "determine coverage of pytests for:" + echo "${PYTHON_SOURCES}" | tr ',' '\n' | sed 's/^\.\///' | awk '{print gsub(/\//, "/"), $0}' | sort -n | cut -d' ' -f2- | tree --fromfile -U + coverage run --include="${PYTHON_SOURCES}" -m pytest + coverage report -m + coverage html + +# - name: Upload coverage report +# uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v 4.6.2 +# with: +# name: coverage-report-python-${{ matrix.python }} +# path: htmlcov/ +# retention-days: 7 +# compression-level: 6 # default GNU Gzip level +# if-no-files-found: warn +# overwrite: false +# include-hidden-files: false - name: Run flake8 to verify PEP8-compliance of Python code run: | diff --git a/.gitignore b/.gitignore index 24e6f0f8..8f6b2b64 100644 --- a/.gitignore +++ b/.gitignore @@ -18,3 +18,4 @@ appbackup.cfg venv_bot_p37 *.swo events_log/ +.vscode/ diff --git a/tasks/build.py b/tasks/build.py index 517c4077..75109821 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -620,7 +620,7 @@ def prepare_jobs(pr, cfg, event_info, action_filter): log(f"{fn}(): checking filter {action_filter.to_string()}") context = {"architecture": arch, "repository": repo_id, "instance": app_name} log(f"{fn}(): context is '{json.dumps(context, indent=4)}'") - if not action_filter.check_filters(context): + if not action_filter.check_build_filters(context): log(f"{fn}(): context does NOT satisfy filter(s), skipping") continue else: diff --git a/tests/test_coverage.py b/tests/test_coverage.py new file mode 100644 index 00000000..dc5384a1 --- /dev/null +++ b/tests/test_coverage.py @@ -0,0 +1,37 @@ +import os +import importlib + + +def test_import_all_python_files(): + """Test that imports all Python files to ensure they appear in coverage reports.""" + # Get the project root directory (where this test file is located) + project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + # List of directories to scan for Python files + dirs_to_scan = ['connections', 'tasks', 'tools'] + + # Import all Python files + for dir_name in dirs_to_scan: + dir_path = os.path.join(project_root, dir_name) + if os.path.exists(dir_path): + for file_name in os.listdir(dir_path): + if file_name.endswith('.py') and not file_name.startswith('__'): + module_name = f"{dir_name}.{file_name[:-3]}" + try: + importlib.import_module(module_name) + except ImportError as e: + # Log the error but don't fail the test + print(f"Could not import {module_name}: {e}") + + # Import root Python files + for file_name in os.listdir(project_root): + if file_name.endswith('.py') and not file_name.startswith('__'): + module_name = file_name[:-3] + try: + importlib.import_module(module_name) + except ImportError as e: + # Log the error but don't fail the test + print(f"Could not import {module_name}: {e}") + + # The test always passes as its purpose is just to import files + assert True diff --git a/tests/test_tools_filter.py b/tests/test_tools_filter.py index b689aa60..b223ccd6 100644 --- a/tests/test_tools_filter.py +++ b/tests/test_tools_filter.py @@ -13,6 +13,7 @@ import copy # Third party imports (anything installed into the local Python environment) +from unittest.mock import patch import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) @@ -20,8 +21,13 @@ COMPONENT_UNKNOWN, EESSIBotActionFilter, EESSIBotActionFilterError, - FILTER_EMPTY_PATTERN, - FILTER_FORMAT_ERROR) + FILTER_COMPONENT_ACCEL, + FILTER_COMPONENT_ARCH, + FILTER_COMPONENT_INST, + FILTER_COMPONENT_REPO, + FILTER_EMPTY_VALUE, + FILTER_FORMAT_ERROR, + UNKNOWN_COMPONENT_CONST) def test_empty_action_filter(): @@ -34,9 +40,9 @@ def test_empty_action_filter(): 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}" + value = 'nvidia/cc80' + af.add_filter_from_string(f"{component}:{value}") + expected = f"accelerator:{value}" actual = af.to_string() assert expected == actual @@ -52,29 +58,29 @@ def test_add_non_wellformed_filter_from_string(): assert str(err1.value) == expected_msg1 component2 = 'a' - pattern2 = 'zen4' - filter_string2 = f"{component2}:{pattern2}" + value2 = 'zen4' + filter_string2 = f"{component2}:{value2}" 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) + expected_msg2 = COMPONENT_TOO_SHORT.format(component=component2, value=value2) assert str(err2.value) == expected_msg2 component3 = 'arc' - pattern3 = '' - filter_string3 = f"{component3}:{pattern3}" + value3 = '' + filter_string3 = f"{component3}:{value3}" 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) + expected_msg3 = FILTER_EMPTY_VALUE.format(filter_string=filter_string3) assert str(err3.value) == expected_msg3 def test_add_single_action_filter(): af = EESSIBotActionFilter("") component = 'arch' - pattern = '.*intel.*' - af.add_filter(component, pattern) + value = '.*intel.*' + af.add_filter(component, value) expected = "architecture:.*intel.*" actual = af.to_string() assert expected == actual @@ -83,22 +89,22 @@ def test_add_single_action_filter(): def test_add_non_supported_component(): af = EESSIBotActionFilter("") component = 'machine' - pattern = '.*intel.*' + value = '.*intel.*' with pytest.raises(Exception) as err: - af.add_filter(component, pattern) + af.add_filter(component, value) assert err.type == EESSIBotActionFilterError - expected_msg = COMPONENT_UNKNOWN.format(component=component, pattern=pattern) + expected_msg = COMPONENT_UNKNOWN.format(component=component, value=value) assert str(err.value) == expected_msg def test_add_too_short_supported_component(): af = EESSIBotActionFilter("") component = 'a' - pattern = '.*intel.*' + value = '.*intel.*' with pytest.raises(Exception) as err: - af.add_filter(component, pattern) + af.add_filter(component, value) assert err.type == EESSIBotActionFilterError - expected_msg = COMPONENT_TOO_SHORT.format(component=component, pattern=pattern) + expected_msg = COMPONENT_TOO_SHORT.format(component=component, value=value) assert str(err.value) == expected_msg @@ -107,31 +113,31 @@ def test_add_too_short_supported_component(): def complex_filter(): af = EESSIBotActionFilter("") component1 = 'arch' - pattern1 = '.*intel.*' - af.add_filter(component1, pattern1) + value1 = '.*intel.*' + af.add_filter(component1, value1) component2 = 'repo' - pattern2 = 'nessi.no-2022.*' - af.add_filter(component2, pattern2) + value2 = 'nessi.no-2022.*' + af.add_filter(component2, value2) component3 = 'inst' - pattern3 = '[aA]' - af.add_filter(component3, pattern3) + value3 = '[aA]' + af.add_filter(component3, value3) yield af def test_remove_existing_filter(complex_filter): component1 = 'architecture' - pattern1 = '.*intel.*' - filter_string1 = f"{component1}:{pattern1}" + value1 = '.*intel.*' + filter_string1 = f"{component1}:{value1}" component2 = 'repository' - pattern2 = 'nessi.no-2022.*' - filter_string2 = f"{component2}:{pattern2}" + value2 = 'nessi.no-2022.*' + filter_string2 = f"{component2}:{value2}" component3 = 'instance' - pattern3 = '[aA]' - filter_string3 = f"{component3}:{pattern3}" + value3 = '[aA]' + filter_string3 = f"{component3}:{value3}" # remove last filter org_filter = copy.deepcopy(complex_filter) - org_filter.remove_filter(component3, pattern3) + org_filter.remove_filter(component3, value3) expected = filter_string1 expected += f" {filter_string2}" actual = org_filter.to_string() @@ -139,7 +145,7 @@ def test_remove_existing_filter(complex_filter): # remove second last filter org_filter = copy.deepcopy(complex_filter) - org_filter.remove_filter(component2, pattern2) + org_filter.remove_filter(component2, value2) expected = filter_string1 expected += f" {filter_string3}" actual = org_filter.to_string() @@ -147,7 +153,7 @@ def test_remove_existing_filter(complex_filter): # remove first filter org_filter = copy.deepcopy(complex_filter) - org_filter.remove_filter(component1, pattern1) + org_filter.remove_filter(component1, value1) expected = filter_string2 expected += f" {filter_string3}" actual = org_filter.to_string() @@ -156,11 +162,11 @@ def test_remove_existing_filter(complex_filter): def test_remove_non_existing_filter(complex_filter): component = 'accel' - pattern = 'amd/gfx90a' + value = 'amd/gfx90a' # remove non-existing filter org_filter = copy.deepcopy(complex_filter) - org_filter.remove_filter(component, pattern) + org_filter.remove_filter(component, value) org_filter_str = org_filter.to_string() complex_filter_str = complex_filter.to_string() assert org_filter_str == complex_filter_str @@ -168,55 +174,171 @@ def test_remove_non_existing_filter(complex_filter): def test_remove_filter_errors(complex_filter): component1 = 'ac' - pattern1 = 'amd/gfx90a' + value1 = 'amd/gfx90a' component2 = 'operating_system' - pattern2 = 'linux' + value2 = '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) + org_filter.remove_filter(component1, value1) assert err1.type == EESSIBotActionFilterError - expected_msg1 = COMPONENT_TOO_SHORT.format(component=component1, pattern=pattern1) + expected_msg1 = COMPONENT_TOO_SHORT.format(component=component1, value=value1) 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) + org_filter.remove_filter(component2, value2) assert err2.type == EESSIBotActionFilterError - expected_msg2 = COMPONENT_UNKNOWN.format(component=component2, pattern=pattern2) + expected_msg2 = COMPONENT_UNKNOWN.format(component=component2, value=value2) assert str(err2.value) == expected_msg2 -def test_check_matching_empty_filter(): +def test_empty_filter_to_string(): af = EESSIBotActionFilter("") expected = '' actual = af.to_string() assert expected == actual + +def test_empty_filter_empty_context_no_component(): + af = EESSIBotActionFilter("") + context = {} + components = [] + actual = af.check_filters(context, components) + expected = True + assert expected == actual + + +def test_empty_filter_arch_context_no_component(): + af = EESSIBotActionFilter("") context = {"arch": "foo"} - actual = af.check_filters(context) + components = [] + actual = af.check_filters(context, components) expected = True assert expected == actual -def test_check_matching_simple_filter(): +def test_empty_filter_arch_context_arch_component(): + af = EESSIBotActionFilter("") + context = {"arch": "foo"} + components = [FILTER_COMPONENT_ARCH] + actual = af.check_filters(context, components) + expected = False + # if component to check is given, they need to be present in filter and + # context + assert expected == actual + + +def test_empty_filter_empty_context_arch_component(): + af = EESSIBotActionFilter("") + context = {} + components = [FILTER_COMPONENT_ARCH] + actual = af.check_filters(context, components) + expected = False + # if component to check is given, they need to be present in filter and + # context + assert expected == actual + + +def test_arch_filter_to_string(): af = EESSIBotActionFilter("") component = 'arch' - pattern = '.*intel.*' - af.add_filter(component, pattern) - expected = f"architecture:{pattern}" + value = '.*intel.*' + af.add_filter(component, value) + expected = f"architecture:{value}" actual = af.to_string() assert expected == actual + +def test_arch_filter_no_context_other_component(): + af = EESSIBotActionFilter("") + component = 'arch' + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) + context = {"architecture": "x86_64/intel/cascadelake"} + components = ['OTHER'] + with pytest.raises(Exception) as err: + af.check_filters(context, components) + assert err.type == EESSIBotActionFilterError + expected_msg = UNKNOWN_COMPONENT_CONST.format(component=components[0]) + assert str(err.value) == expected_msg + + +def test_arch_filter_arch_context_arch_component(): + af = EESSIBotActionFilter("") + component = 'arch' + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) + context = {"architecture": "x86_64/intel/cascadelake"} + components = [FILTER_COMPONENT_ARCH] + actual = af.check_filters(context, components) + expected = True + assert expected == actual + + +def test_arch_filter_arch_context_no_component(): + af = EESSIBotActionFilter("") + component = 'arch' + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) context = {"architecture": "x86_64/intel/cascadelake"} - actual = af.check_filters(context) + components = [] + actual = af.check_filters(context, components) expected = True assert expected == actual -def test_create_complex_filter(complex_filter): +def test_arch_filter_no_context_no_component(): + af = EESSIBotActionFilter("") + component = 'arch' + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) + context = {} + components = [] + actual = af.check_filters(context, components) + expected = True + assert expected == actual + + +def test_arch_filter_no_context_arch_component(): + af = EESSIBotActionFilter("") + component = 'arch' + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) + context = {} + components = [FILTER_COMPONENT_ARCH] + actual = af.check_filters(context, components) + expected = False + assert expected == actual + + +def test_arch_filter_arch_context_inst_component(): + af = EESSIBotActionFilter("") + component = 'arch' + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) + context = {"architecture": "x86_64/intel/cascadelake"} + components = [FILTER_COMPONENT_INST] + actual = af.check_filters(context, components) + expected = False + assert expected == actual + + +def test_arch_filter_arch_context_two_components(): + af = EESSIBotActionFilter("") + component = 'arch' + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) + context = {"architecture": "x86_64/intel/cascadelake"} + components = [FILTER_COMPONENT_ARCH, FILTER_COMPONENT_INST] + actual = af.check_filters(context, components) + expected = False + assert expected == actual + + +def test_complex_filter_to_string(complex_filter): expected = "architecture:.*intel.*" expected += " repository:nessi.no-2022.*" expected += " instance:[aA]" @@ -224,31 +346,53 @@ def test_create_complex_filter(complex_filter): assert expected == actual -def test_match_empty_context(complex_filter): +def test_complex_filter_no_context_other_component(complex_filter): + context = {"architecture": ".*intel.*"} + components = ['OTHER'] + with pytest.raises(Exception) as err: + complex_filter.check_filters(context, components) + assert err.type == EESSIBotActionFilterError + expected_msg = UNKNOWN_COMPONENT_CONST.format(component=components[0]) + assert str(err.value) == expected_msg + + +def test_complex_filter_no_context_no_component(complex_filter): context = {} - expected = False - actual = complex_filter.check_filters(context) + components = [] + expected = True + actual = complex_filter.check_filters(context, components) assert expected == actual -def test_match_architecture_context(complex_filter): - context = {"architecture": "x86_64/intel/cascadelake"} +# def test_complex_filter_no_context_no_component(complex_filter): +# context = {} +# components = [] +# expected = True +# actual = complex_filter.check_filters(context, components) +# assert expected == actual + + +def test_complex_filter_arch_context_arch_component(complex_filter): + context = {"architecture": ".*intel.*"} + components = [FILTER_COMPONENT_ARCH] expected = True - actual = complex_filter.check_filters(context) + actual = complex_filter.check_filters(context, components) assert expected == actual -def test_match_architecture_job_context(complex_filter): - context = {"architecture": "x86_64/intel/cascadelake", "job": 1234} +def test_complex_filter_job_context_arch_component(complex_filter): + context = {"architecture": ".*intel.*", "job": 1234} + components = [FILTER_COMPONENT_ARCH] expected = True - actual = complex_filter.check_filters(context) + actual = complex_filter.check_filters(context, components) assert expected == actual -def test_non_match_architecture_repository_context(complex_filter): - context = {"architecture": "x86_64/intel/cascadelake", "repository": "EESSI"} +def test_complex_filter_repo_context_arch_and_repo_components(complex_filter): + context = {"architecture": ".*intel.*", "repository": "EESSI"} + components = [FILTER_COMPONENT_ARCH, FILTER_COMPONENT_REPO] expected = False - actual = complex_filter.check_filters(context) + actual = complex_filter.check_filters(context, components) assert expected == actual @@ -256,20 +400,21 @@ def test_non_match_architecture_repository_context(complex_filter): def arch_filter_slash_syntax(): af = EESSIBotActionFilter("") component = 'arch' - pattern = '.*/intel/.*' - af.add_filter(component, pattern) + value = 'x86_64/intel/cascadelake' + af.add_filter(component, value) yield af def test_match_architecture_syntax_slash(arch_filter_slash_syntax): - context = {"architecture": "x86_64/intel/cascadelake", "repository": "EESSI"} + components = [FILTER_COMPONENT_ARCH] + context = {"architecture": "x86_64/intel/cascadelake"} expected = True - actual = arch_filter_slash_syntax.check_filters(context) + actual = arch_filter_slash_syntax.check_filters(context, components) assert expected == actual context = {"architecture": "x86_64-intel-cascadelake"} expected = True - actual = arch_filter_slash_syntax.check_filters(context) + actual = arch_filter_slash_syntax.check_filters(context, components) assert expected == actual @@ -277,20 +422,21 @@ def test_match_architecture_syntax_slash(arch_filter_slash_syntax): def arch_filter_dash_syntax(): af = EESSIBotActionFilter("") component = 'arch' - pattern = '.*-intel-.*' - af.add_filter(component, pattern) + value = 'x86_64-intel-cascadelake' + af.add_filter(component, value) yield af def test_match_architecture_syntax_dash(arch_filter_dash_syntax): - context = {"architecture": "x86_64-intel-cascadelake", "repository": "EESSI"} + components = [FILTER_COMPONENT_ARCH] + context = {"architecture": "x86_64-intel-cascadelake"} expected = True - actual = arch_filter_dash_syntax.check_filters(context) + actual = arch_filter_dash_syntax.check_filters(context, components) assert expected == actual context = {"architecture": "x86_64/intel-cascadelake"} expected = True - actual = arch_filter_dash_syntax.check_filters(context) + actual = arch_filter_dash_syntax.check_filters(context, components) assert expected == actual @@ -298,20 +444,21 @@ def test_match_architecture_syntax_dash(arch_filter_dash_syntax): def accel_filter_slash_syntax(): af = EESSIBotActionFilter("") component = 'accel' - pattern = 'nvidia/.*' - af.add_filter(component, pattern) + value = 'nvidia/cc70' + af.add_filter(component, value) yield af def test_match_accelerator_syntax_slash(accel_filter_slash_syntax): - context = {"accelerator": "nvidia/cc70", "repository": "EESSI"} + components = [FILTER_COMPONENT_ACCEL] + context = {"accelerator": "nvidia/cc70"} expected = True - actual = accel_filter_slash_syntax.check_filters(context) + actual = accel_filter_slash_syntax.check_filters(context, components) assert expected == actual context = {"accelerator": "nvidia=cc70"} expected = True - actual = accel_filter_slash_syntax.check_filters(context) + actual = accel_filter_slash_syntax.check_filters(context, components) assert expected == actual @@ -319,18 +466,145 @@ def test_match_accelerator_syntax_slash(accel_filter_slash_syntax): def accel_filter_equal_syntax(): af = EESSIBotActionFilter("") component = 'accel' - pattern = 'amd=gfx90a' - af.add_filter(component, pattern) + value = 'amd=gfx90a' + af.add_filter(component, value) yield af def test_match_accelerator_syntax_equal(accel_filter_equal_syntax): - context = {"accelerator": "amd=gfx90a", "repository": "EESSI"} + components = [FILTER_COMPONENT_ACCEL] + context = {"accelerator": "amd=gfx90a"} expected = True - actual = accel_filter_equal_syntax.check_filters(context) + actual = accel_filter_equal_syntax.check_filters(context, components) assert expected == actual context = {"accelerator": "amd/gfx90a"} expected = True - actual = accel_filter_equal_syntax.check_filters(context) + actual = accel_filter_equal_syntax.check_filters(context, components) assert expected == actual + +# All after this line is generated by Claude (via Cursor) +# import pytest +# from tools.filter import (EESSIBotActionFilter, +# EESSIBotActionFilterError, +# FILTER_COMPONENT_ARCH, +# FILTER_COMPONENT_INST, +# FILTER_COMPONENT_REPO) + + +def test_filter_creation(): + """Test basic filter creation from string""" + filter_str = "arch:x86_64 inst:AWS repo:eessi-2023.06" + action_filter = EESSIBotActionFilter(filter_str) + + # Check that filters were parsed correctly + assert len(action_filter.action_filters) == 3 + assert action_filter.get_filter_by_component(FILTER_COMPONENT_ARCH) == ['x86_64'] + assert action_filter.get_filter_by_component(FILTER_COMPONENT_INST) == ['AWS'] + assert action_filter.get_filter_by_component(FILTER_COMPONENT_REPO) == ['eessi-2023.06'] + + +def test_unexpected_exception_handling(): + """Test that unexpected exceptions in filter creation are logged and re-raised""" + # Create a mock exception + mock_exception = ValueError("Test unexpected error") + + # Mock the log function to verify it's called + with patch('tools.filter.log') as mock_log: + # Mock the add_filter_from_string method to raise our mock exception + with patch.object(EESSIBotActionFilter, 'add_filter_from_string', side_effect=mock_exception): + # Try to create a filter, which should raise our mock exception + with pytest.raises(ValueError) as exc_info: + EESSIBotActionFilter("arch:x86_64") + + # Verify the exception was re-raised + assert exc_info.value == mock_exception + + # Verify the log function was called with the expected message + mock_log.assert_called_once() + log_call_args = mock_log.call_args[0][0] + assert "Unexpected err=" in log_call_args + assert "Test unexpected error" in log_call_args + assert "type(err)=" in log_call_args + + +def test_filter_creation_invalid_format(): + """Test filter creation with invalid format""" + with pytest.raises(EESSIBotActionFilterError): + EESSIBotActionFilter("invalid_format") + + +def test_filter_creation_empty_value(): + """Test filter creation with empty value""" + with pytest.raises(EESSIBotActionFilterError): + EESSIBotActionFilter("arch:") + + +def test_filter_creation_short_component(): + """Test filter creation with component name too short""" + with pytest.raises(EESSIBotActionFilterError): + EESSIBotActionFilter("ar:x86_64") + + +def test_add_filter(): + """Test adding a filter manually""" + action_filter = EESSIBotActionFilter("") + action_filter.add_filter("arch", "x86_64") + assert action_filter.get_filter_by_component(FILTER_COMPONENT_ARCH) == ['x86_64'] + + +def test_remove_filter(): + """Test removing a filter""" + action_filter = EESSIBotActionFilter("arch:x86_64 inst:AWS") + action_filter.remove_filter("arch", "x86_64") + assert len(action_filter.get_filter_by_component(FILTER_COMPONENT_ARCH)) == 0 + assert len(action_filter.get_filter_by_component(FILTER_COMPONENT_INST)) == 1 + + +def test_check_build_filters(): + """Test checking build filters against a context""" + action_filter = EESSIBotActionFilter("arch:x86_64 inst:AWS repo:eessi-2023.06") + + # Matching context + matching_context = { + FILTER_COMPONENT_ARCH: "x86_64", + FILTER_COMPONENT_INST: "AWS", + FILTER_COMPONENT_REPO: "eessi-2023.06" + } + assert action_filter.check_build_filters(matching_context) is True + + # Non-matching context + non_matching_context = { + FILTER_COMPONENT_ARCH: "aarch64", + FILTER_COMPONENT_INST: "AWS", + FILTER_COMPONENT_REPO: "eessi-2023.06" + } + assert action_filter.check_build_filters(non_matching_context) is False + + +def test_to_string(): + """Test converting filters to string""" + filter_str = "arch:x86_64 inst:AWS repo:eessi-2023.06" + action_filter = EESSIBotActionFilter(filter_str) + # The to_string method returns the expanded component names + expected_str = "architecture:x86_64 instance:AWS repository:eessi-2023.06" + assert action_filter.to_string() == expected_str + + +def test_clear_all(): + """Test clearing all filters""" + action_filter = EESSIBotActionFilter("arch:x86_64 inst:AWS") + action_filter.clear_all() + assert len(action_filter.action_filters) == 0 + + +def test_architecture_filter_normalization(): + """Test that architecture filters are normalized (replacing all - with /)""" + action_filter = EESSIBotActionFilter("arch:x86_64-intel-haswell") + assert action_filter.get_filter_by_component(FILTER_COMPONENT_ARCH) == ['x86_64/intel/haswell'] + + # Test matching with normalized value - only check architecture component + context = { + FILTER_COMPONENT_ARCH: "x86_64/intel/haswell" + } + assert action_filter.check_filters(context, [FILTER_COMPONENT_ARCH]) is True diff --git a/tools/filter.py b/tools/filter.py index 0caa2af8..d34b1d07 100644 --- a/tools/filter.py +++ b/tools/filter.py @@ -11,7 +11,6 @@ # Standard library imports from collections import namedtuple -import re # Third party imports (anything installed into the local Python environment) from pyghee.utils import log @@ -37,13 +36,13 @@ 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'" +COMPONENT_TOO_SHORT = "component in filter spec '{component}:{value}' is too short; must be 3 characters or longer" +COMPONENT_UNKNOWN = "unknown component={component} in {component}:{value}" +FILTER_EMPTY_VALUE = "value in filter string '{filter_string}' is empty" +FILTER_FORMAT_ERROR = "filter string '{filter_string}' does not conform to format 'component:value'" UNKNOWN_COMPONENT_CONST = "unknown component constant {component}" -Filter = namedtuple('Filter', ('component', 'pattern')) +Filter = namedtuple('Filter', ('component', 'value')) class EESSIBotActionFilterError(Exception): @@ -64,7 +63,7 @@ class EESSIBotActionFilter: Class for representing a filter that limits in which contexts bot commands are applied. A filter contains a list of key:value pairs where the key corresponds to a component (see FILTER_COMPONENTS) and the value is a - pattern used to filter commands based on the context a command is applied to. + string used to filter commands based on the context a command is applied to. """ def __init__(self, filter_string): """ @@ -101,15 +100,15 @@ def clear_all(self): """ self.action_filters = [] - def add_filter(self, component, pattern): + def add_filter(self, component, value): """ - Adds a filter given by a component and a pattern + Adds a filter given by a component and a value. Uses the full component + name even if only a prefix was used. Args: 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 + value (string): string that represents value of component Returns: None (implicitly) @@ -122,7 +121,7 @@ def add_filter(self, component, pattern): # - 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) + msg = COMPONENT_TOO_SHORT.format(component=component, value=value) log(msg) raise EESSIBotActionFilterError(msg) full_component = None @@ -135,17 +134,17 @@ def add_filter(self, component, pattern): break if full_component: log(f"processing component {component}") - # replace '-' with '/' in pattern when using 'architecture' filter + # replace '-' with '/' in value when using 'architecture' filter # 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 + value = value.replace('-', '/') + # replace '=' with '/' in value 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)) + value = value.replace('=', '/') + self.action_filters.append(Filter(full_component, value)) else: - msg = COMPONENT_UNKNOWN.format(component=component, pattern=pattern) + msg = COMPONENT_UNKNOWN.format(component=component, value=value) log(msg) raise EESSIBotActionFilterError(msg) @@ -154,14 +153,14 @@ def add_filter_from_string(self, filter_string): Adds a filter provided as a string Args: - filter_string (string): filter provided as component:pattern string + filter_string (string): filter provided as component:value string Returns: None (implicitly) Raises: EESSIBotActionFilterError: raised if filter_string does not conform - to 'component:pattern' format or pattern is empty + to 'component:value' format or value is empty """ _filter_split = filter_string.split(':') if len(_filter_split) != 2: @@ -169,48 +168,48 @@ def add_filter_from_string(self, 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]) + msg = COMPONENT_TOO_SHORT.format(component=_filter_split[0], value=_filter_split[1]) log(msg) raise EESSIBotActionFilterError(msg) if len(_filter_split[1]) == 0: - msg = FILTER_EMPTY_PATTERN.format(filter_string=filter_string) + msg = FILTER_EMPTY_VALUE.format(filter_string=filter_string) log(msg) raise EESSIBotActionFilterError(msg) self.add_filter(_filter_split[0], _filter_split[1]) def get_filter_by_component(self, component): """ - Returns filter pattern for component. + Returns filter value for component. Args: component (string): one of FILTER_COMPONENTS Returns: - (list): list of pattern for filters whose component matches argument + (list): list of value for filters whose component matches argument """ if component not in FILTER_COMPONENTS: msg = UNKNOWN_COMPONENT_CONST.format(component=component) raise EESSIBotActionFilterError(msg) - pattern = [] + values = [] for _filter in self.action_filters: if component == _filter.component: - pattern.append(_filter.pattern) - return pattern + values.append(_filter.value) + return values - def remove_filter(self, component, pattern): + def remove_filter(self, component, value): """ - Removes all elements matching the filter given by (component, pattern) + Removes all elements matching the filter given by (component, value) Args: component (string): one of FILTER_COMPONENTS - pattern (string): regex that is applied to a string representing the component + value (string): value for the component Returns: None (implicitly) """ if len(component) < 3: - msg = COMPONENT_TOO_SHORT.format(component=component, pattern=pattern) + msg = COMPONENT_TOO_SHORT.format(component=component, value=value) log(msg) raise EESSIBotActionFilterError(msg) full_component = None @@ -223,7 +222,7 @@ def remove_filter(self, component, pattern): 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) + msg = COMPONENT_UNKNOWN.format(component=component, value=value) log(msg) raise EESSIBotActionFilterError(msg) @@ -238,8 +237,8 @@ def remove_filter(self, component, pattern): # 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})") + if _filter.component.startswith(component) and value == _filter.value: + log(f"removing filter ({_filter.component}, {value})") self.action_filters.pop(index) def to_string(self): @@ -254,53 +253,91 @@ def to_string(self): """ filter_str_list = [] for _filter in self.action_filters: - cm = _filter.component - re = _filter.pattern - filter_str_list.append(f"{cm}:{re}") + component = _filter.component + value = _filter.value + filter_str_list.append(f"{component}:{value}") return " ".join(filter_str_list) - def check_filters(self, context): + def check_build_filters(self, context): """ - Checks filters for a given context which is defined by one to five - components (accelerator, architecture, instance, job, repository) + Checks if the filter in self matches a given context which is defined + by the three components FILTER_COMPONENT_ARCH, FILTER_COMPONENT_INST and + FILTER_COMPONENT_REPO for building. + A single component of a filter matches the corresponding component in a + context if the values for these components are exactly the same. Args: - context (dict) : dictionary that maps components to their value + context (dict) : dictionary that contents (component,value)-pairs + representing the context in which the method is called Returns: - True if no filters are defined or all defined filters match - their corresponding component in the given context - False if any defined filter does not match its corresponding - component in the given context + True if all required components in a given context match their + corresponding component in a filter + False otherwise """ - # if no filters are defined we return True - if len(self.action_filters) == 0: - return True + build_components = [FILTER_COMPONENT_ARCH, FILTER_COMPONENT_INST, FILTER_COMPONENT_REPO] + return self.check_filters(context, build_components) - # we have at least one filter which has to match or we return False - check = False + def check_filters(self, context, components): + """ + Checks if the filter in self matches a given context which is defined + by components. + A single component of a filter matches the corresponding component in a + context if the values for these components are exactly the same. + + Args: + context (dict) : dictionary that contents (component,value)-pairs + representing the context in which the method is called + The context is typically defined by a combination of the build + target (CPU microarchitecture, CernVM-FS repository, optionally + accelerator architecture) and the build system. + components (list) : contains constants FILTER_COMPONENT_* that must + be checked + Returns: + True if all required components in a given context match their + corresponding component in a filter + False otherwise + """ # examples: # filter: 'arch:intel instance:AWS' --> evaluates to True if - # context['architecture'] matches 'intel' and if - # context['instance'] matches 'AWS' + # context['architecture'] is 'intel' and if context['instance'] is 'AWS' # filter: 'repository:eessi-2023.06' --> evaluates to True if - # context['repository'] matches 'eessi-2023.06' - - # we iterate over all defined filters - for af in self.action_filters: - if af.component in context: - value = context[af.component] - # 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 - else: - check = False - break - return check + # context['repository'] is 'eessi-2023.06' + + # TODO should we require a 'double'-match, that is, all components given + # have to match AND all filters given have to match? + # + # we iterate over the components and verify if their value in the given + # context matches the corresponding value in the filter + for component in components: + filter_values = self.get_filter_by_component(component) + if len(filter_values) == 0: + # no filter for component provided --> at least one filter per + # component MUST be given + log(f"no filter for component '{component}' provided; check_filters returns False") + return False + if component in context: + # check if all values for the filter are identical to the value + # of the component in the context + context_value = context[component] + if component == FILTER_COMPONENT_ARCH: + if context_value.startswith('linux'): + # strip leading OS name from architecture string + context_value = '/'.join(context_value.split('/')[1:]) + # replace '-' with '/' in architecture string + context_value = context_value.replace('-', '/') + if component == FILTER_COMPONENT_ACCEL: + context_value = context_value.replace('=', '/') + for filter_value in filter_values: + if filter_value != context_value: + log_msg = "filter value '%s' does not match context value '%s' for component '%s';" + log_msg += " check_filters returns False" + log(log_msg % (filter_value, context_value, component)) + return False + else: + # missing required component in context, could lead to unexpected behavior + log_msg = "missing required component '%s' in context; check_filters returns False" + log(log_msg % component) + return False + return True