diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index edc1a6c..67fdde2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -31,7 +31,7 @@ repos: - id: detect-secrets - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.15.1 + rev: v0.15.2 hooks: - id: ruff - id: ruff-format diff --git a/apps/jira_utils/jira_information.py b/apps/jira_utils/jira_information.py index d044d3d..ed5b766 100644 --- a/apps/jira_utils/jira_information.py +++ b/apps/jira_utils/jira_information.py @@ -122,7 +122,7 @@ def get_jira_information( jira_fix_versions = ",".join([jira_fix_version.name for jira_fix_version in jira_fix_versions]) current_target_versions = re.findall(re_compile, jira_fix_versions) - if any([version in jira_target_versions for version in current_target_versions]): + if any(version in jira_target_versions for version in current_target_versions): return file_name, jira_error_string else: diff --git a/apps/unused_code/unused_code.py b/apps/unused_code/unused_code.py index da989c7..2ab2ca7 100644 --- a/apps/unused_code/unused_code.py +++ b/apps/unused_code/unused_code.py @@ -6,10 +6,11 @@ import re import subprocess import sys +from collections.abc import Callable, Iterable from concurrent.futures import Future, ThreadPoolExecutor, as_completed from functools import lru_cache from pathlib import Path -from typing import Any, Callable, Iterable +from typing import Any import click from ast_comments import parse @@ -46,9 +47,9 @@ def _detect_supported_grep_flag() -> str: result = subprocess.run(probe_cmd, check=False, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) if result.returncode in (0, 1): return flag - except Exception: + except (OSError, subprocess.SubprocessError): # Try next candidate - pass + LOGGER.debug(f"git grep flag {flag!r} probe failed, trying next candidate") raise RuntimeError( "git grep does not support '-P' (PCRE) or '-G' (basic regex) on this platform. " @@ -62,11 +63,15 @@ def is_fixture_autouse(func: ast.FunctionDef) -> bool: if not hasattr(deco, "func"): continue - if getattr(deco.func, "attr", None) and getattr(deco.func, "value", None): - if deco.func.attr == "fixture" and deco.func.value.id == "pytest": - for _key in deco.keywords: - if _key.arg == "autouse": - return True + if ( + getattr(deco.func, "attr", None) + and getattr(deco.func, "value", None) + and deco.func.attr == "fixture" + and deco.func.value.id == "pytest" + ): + for _key in deco.keywords: + if _key.arg == "autouse": + return True return False @@ -80,18 +85,25 @@ def is_pytest_fixture(func: ast.FunctionDef) -> bool: # Case 1: @pytest.fixture(...) if hasattr(decorator, "func"): # e.g. @pytest.fixture(...) - if getattr(decorator.func, "attr", None) and getattr(decorator.func, "value", None): - if decorator.func.attr == "fixture" and getattr(decorator.func.value, "id", None) == "pytest": - return True + if ( + getattr(decorator.func, "attr", None) + and getattr(decorator.func, "value", None) + and decorator.func.attr == "fixture" + and getattr(decorator.func.value, "id", None) == "pytest" + ): + return True # e.g. from pytest import fixture; @fixture(...) if isinstance(decorator.func, ast.Name) and decorator.func.id == "fixture": return True # Case 2: @pytest.fixture (no parentheses) else: # e.g. @pytest.fixture - if getattr(decorator, "attr", None) == "fixture" and getattr(decorator, "value", None): - if getattr(decorator.value, "id", None) == "pytest": - return True + if ( + getattr(decorator, "attr", None) == "fixture" + and getattr(decorator, "value", None) + and getattr(decorator.value, "id", None) == "pytest" + ): + return True # e.g. from pytest import fixture; @fixture if isinstance(decorator, ast.Name) and decorator.id == "fixture": return True @@ -149,17 +161,14 @@ def _build_keyword_unpacking_pattern(function_name: str) -> str: def _is_pytest_mark_usefixtures_call(call_node: ast.Call) -> bool: """Check if an AST Call node represents pytest.mark.usefixtures(...).""" - if isinstance(call_node.func, ast.Attribute): - # Handle pytest.mark.usefixtures - if ( - call_node.func.attr == "usefixtures" - and isinstance(call_node.func.value, ast.Attribute) - and call_node.func.value.attr == "mark" - and isinstance(call_node.func.value.value, ast.Name) - and call_node.func.value.value.id == "pytest" - ): - return True - return False + return ( + isinstance(call_node.func, ast.Attribute) + and call_node.func.attr == "usefixtures" + and isinstance(call_node.func.value, ast.Attribute) + and call_node.func.value.attr == "mark" + and isinstance(call_node.func.value.value, ast.Name) + and call_node.func.value.value.id == "pytest" + ) def _check_fixturenames_insert_pattern(fixture_name: str, file_path: str) -> bool: @@ -173,27 +182,27 @@ def _check_fixturenames_insert_pattern(fixture_name: str, file_path: str) -> boo content = f.read() tree = parse(source=content) - - # Look for method calls like item.fixturenames.insert(...) - for node in ast.walk(tree): - if isinstance(node, ast.Call): - # Check for attribute calls like item.fixturenames.insert - if ( - isinstance(node.func, ast.Attribute) - and node.func.attr == "insert" - and isinstance(node.func.value, ast.Attribute) - and node.func.value.attr == "fixturenames" - ): - # Check the arguments for our fixture name - for arg in node.args: - if isinstance(arg, ast.Constant) and isinstance(arg.value, str): - if arg.value == fixture_name: - return True - - return False except (FileNotFoundError, SyntaxError, ValueError): return False + # Look for method calls like item.fixturenames.insert(...) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + # Check for attribute calls like item.fixturenames.insert + if ( + isinstance(node.func, ast.Attribute) + and node.func.attr == "insert" + and isinstance(node.func.value, ast.Attribute) + and node.func.value.attr == "fixturenames" + ): + # Check the arguments for our fixture name + for arg in node.args: + if isinstance(arg, ast.Constant) and isinstance(arg.value, str) and arg.value == fixture_name: + return True + + return False + def _check_getfixturevalue_pattern(fixture_name: str, file_path: str) -> bool: """Check if a fixture is used in request.getfixturevalue() pattern. @@ -207,31 +216,31 @@ def _check_getfixturevalue_pattern(fixture_name: str, file_path: str) -> bool: content = f.read() tree = parse(source=content) + except (FileNotFoundError, SyntaxError, ValueError): + return False - # Look for method calls like request.getfixturevalue(...) - for node in ast.walk(tree): - if isinstance(node, ast.Call): - # Check for attribute calls like request.getfixturevalue - if isinstance(node.func, ast.Attribute) and node.func.attr == "getfixturevalue": - # Check positional arguments - for arg in node.args: - if isinstance(arg, ast.Constant) and isinstance(arg.value, str): - if arg.value == fixture_name: - return True + # Look for method calls like request.getfixturevalue(...) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + # Check for attribute calls like request.getfixturevalue + if isinstance(node.func, ast.Attribute) and node.func.attr == "getfixturevalue": + # Check positional arguments + for arg in node.args: + if isinstance(arg, ast.Constant) and isinstance(arg.value, str) and arg.value == fixture_name: + return True - # Check keyword arguments (argname="fixture_name") - for keyword in node.keywords: - if keyword.arg == "argname": - if ( - isinstance(keyword.value, ast.Constant) - and isinstance(keyword.value.value, str) - and keyword.value.value == fixture_name - ): - return True + # Check keyword arguments (argname="fixture_name") + for keyword in node.keywords: + if ( + keyword.arg == "argname" + and isinstance(keyword.value, ast.Constant) + and isinstance(keyword.value.value, str) + and keyword.value.value == fixture_name + ): + return True - return False - except (FileNotFoundError, SyntaxError, ValueError): - return False + return False def _is_usefixtures_context(file_path: str, line_number: str, fixture_name: str) -> bool: @@ -246,55 +255,59 @@ def _is_usefixtures_context(file_path: str, line_number: str, fixture_name: str) # Parse the AST of the file tree = parse(source=content) target_line = int(line_number) + except (FileNotFoundError, SyntaxError, ValueError): + return False - # Look for pytest.mark.usefixtures calls that span the target line - for node in ast.walk(tree): - if isinstance(node, ast.Assign): - # Check if the value is a call to pytest.mark.usefixtures - if isinstance(node.value, ast.Call): - if _is_pytest_mark_usefixtures_call(node.value): + # Look for pytest.mark.usefixtures calls that span the target line + for node in ast.walk(tree): + if isinstance(node, ast.Assign): + # Check if the value is a call to pytest.mark.usefixtures + if isinstance(node.value, ast.Call) and _is_pytest_mark_usefixtures_call(node.value): + # Check if the target line is within this call's range + call_start = node.value.lineno + call_end = node.value.end_lineno or call_start + + if call_start <= target_line <= call_end: + # Check if any of the arguments contain our fixture name + for arg in node.value.args: + if isinstance(arg, ast.Constant) and isinstance(arg.value, str) and arg.value == fixture_name: + return True + + # Also check if the value is a list containing pytest.mark.usefixtures calls + elif isinstance(node.value, ast.List): + for element in node.value.elts: + if isinstance(element, ast.Call) and _is_pytest_mark_usefixtures_call(element): # Check if the target line is within this call's range - call_start = node.value.lineno - call_end = node.value.end_lineno or call_start + call_start = element.lineno + call_end = element.end_lineno or call_start if call_start <= target_line <= call_end: # Check if any of the arguments contain our fixture name - for arg in node.value.args: - if isinstance(arg, ast.Constant) and isinstance(arg.value, str): - if arg.value == fixture_name: - return True - - # Also check if the value is a list containing pytest.mark.usefixtures calls - elif isinstance(node.value, ast.List): - for element in node.value.elts: - if isinstance(element, ast.Call) and _is_pytest_mark_usefixtures_call(element): - # Check if the target line is within this call's range - call_start = element.lineno - call_end = element.end_lineno or call_start - - if call_start <= target_line <= call_end: - # Check if any of the arguments contain our fixture name - for arg in element.args: - if isinstance(arg, ast.Constant) and isinstance(arg.value, str): - if arg.value == fixture_name: - return True - - # Also check decorators - elif isinstance(node, (ast.FunctionDef, ast.ClassDef)): - for decorator in node.decorator_list: - if isinstance(decorator, ast.Call) and _is_pytest_mark_usefixtures_call(decorator): - call_start = decorator.lineno - call_end = decorator.end_lineno or call_start - - if call_start <= target_line <= call_end: - for arg in decorator.args: - if isinstance(arg, ast.Constant) and isinstance(arg.value, str): - if arg.value == fixture_name: - return True + for arg in element.args: + if ( + isinstance(arg, ast.Constant) + and isinstance(arg.value, str) + and arg.value == fixture_name + ): + return True + + # Also check decorators + elif isinstance(node, (ast.FunctionDef, ast.ClassDef)): + for decorator in node.decorator_list: + if isinstance(decorator, ast.Call) and _is_pytest_mark_usefixtures_call(decorator): + call_start = decorator.lineno + call_end = decorator.end_lineno or call_start + + if call_start <= target_line <= call_end: + for arg in decorator.args: + if ( + isinstance(arg, ast.Constant) + and isinstance(arg.value, str) + and arg.value == fixture_name + ): + return True - return False - except (FileNotFoundError, SyntaxError, ValueError): - return False + return False def _is_documentation_pattern(line: str, function_name: str) -> bool: @@ -351,11 +364,7 @@ def _is_documentation_pattern(line: str, function_name: str) -> bool: # Pattern 4: Check for common docstring patterns # Lines that start with common documentation patterns doc_starters = ['"""', "'''", "# ", "## ", "### ", "-", "•"] - if any(stripped_line.startswith(starter) for starter in doc_starters): - if f"{function_name}(" in stripped_line: - return True - - return False + return any(stripped_line.startswith(starter) for starter in doc_starters) and f"{function_name}(" in stripped_line def _find_git_root(file_path: str) -> str: @@ -436,10 +445,7 @@ def is_ignore_function_list(ignore_prefix_list: list[str], function: ast.Functio ignore_function_lists = [ function.name for ignore_prefix in ignore_prefix_list if function.name.startswith(ignore_prefix) ] - if ignore_function_lists: - return True - - return False + return bool(ignore_function_lists) def _resolve_absolute_path(git_grep_path: str, reference_file: str) -> str: @@ -493,7 +499,7 @@ def process_file(py_file: str, func_ignore_prefix: list[str], file_ignore_list: # Ignore commented lines (full line or inline) code_part = _line.split("#", 1)[0] - if code_part.startswith("import") or code_part.startswith("from"): + if code_part.startswith(("import", "from")): continue if func.name not in code_part: @@ -663,7 +669,7 @@ def get_unused_functions( try: if unused_func := future.result(): unused_functions.append(unused_func) - except Exception as exc: + except Exception as exc: # noqa: BLE001 processing_errors.append(f"{jobs[future]}: {exc}") if processing_errors: diff --git a/apps/utils.py b/apps/utils.py index f3c1df6..8def6de 100644 --- a/apps/utils.py +++ b/apps/utils.py @@ -2,7 +2,8 @@ import json import os -from typing import Any, Dict, Iterable, Optional +from collections.abc import Iterable +from typing import Any import click import yaml @@ -11,7 +12,7 @@ LOGGER = get_logger(name=__name__) -def get_util_config(util_name: str, config_file_path: Optional[str] = None) -> Dict[str, Any]: +def get_util_config(util_name: str, config_file_path: str | None = None) -> dict[str, Any]: if config_file_path and os.path.exists(config_file_path): with open(config_file_path) as _file: return yaml.safe_load(_file).get(util_name, {}) diff --git a/tests/polarion/test_polarion_set_automated_coverage.py b/tests/polarion/test_polarion_set_automated_coverage.py index f47e276..6e7bd47 100644 --- a/tests/polarion/test_polarion_set_automated_coverage.py +++ b/tests/polarion/test_polarion_set_automated_coverage.py @@ -303,31 +303,33 @@ def test_command_verbose_logging(self, mock_remove, mock_find, mock_get_project) mock_find.return_value = [] mock_remove.return_value = {"updated": [], "failed": []} - with patch("apps.polarion.polarion_set_automated.LOGGER") as mock_logger: - with patch("logging.getLogger") as mock_get_logger: - mock_utils_logger = MagicMock() - mock_get_logger.return_value = mock_utils_logger + with ( + patch("apps.polarion.polarion_set_automated.LOGGER") as mock_logger, + patch("logging.getLogger") as mock_get_logger, + ): + mock_utils_logger = MagicMock() + mock_get_logger.return_value = mock_utils_logger - # Act - result = self.runner.invoke( - polarion_approve_automate, - [ - "--previous-commit", - "abc123", - "--current-commit", - "def456", - "--project-id", - "TEST_PROJECT", - "--verbose", - ], - ) + # Act + result = self.runner.invoke( + polarion_approve_automate, + [ + "--previous-commit", + "abc123", + "--current-commit", + "def456", + "--project-id", + "TEST_PROJECT", + "--verbose", + ], + ) - # Assert - assert result.exit_code == 0 - # Verify logging level was set - mock_logger.setLevel.assert_called_with(logging.DEBUG) - mock_get_logger.assert_called_with("apps.polarion.polarion_utils") - mock_utils_logger.setLevel.assert_called_with(logging.DEBUG) + # Assert + assert result.exit_code == 0 + # Verify logging level was set + mock_logger.setLevel.assert_called_with(logging.DEBUG) + mock_get_logger.assert_called_with("apps.polarion.polarion_utils") + mock_utils_logger.setLevel.assert_called_with(logging.DEBUG) @patch("apps.polarion.polarion_set_automated.get_polarion_project_id") def test_command_uses_config_file_project_id(self, mock_get_project): diff --git a/tests/polarion/test_polarion_verify_tc_requirements_coverage.py b/tests/polarion/test_polarion_verify_tc_requirements_coverage.py index 15ede07..6e22a35 100644 --- a/tests/polarion/test_polarion_verify_tc_requirements_coverage.py +++ b/tests/polarion/test_polarion_verify_tc_requirements_coverage.py @@ -114,23 +114,25 @@ def test_command_verbose_logging(self, mock_validate, mock_find, mock_get_projec mock_find.return_value = [] mock_validate.return_value = [] - with patch("apps.polarion.polarion_verify_tc_requirements.LOGGER") as mock_logger: - with patch("logging.getLogger") as mock_get_logger: - mock_utils_logger = MagicMock() - mock_get_logger.return_value = mock_utils_logger - - # Act - result = self.runner.invoke( - has_verify, - ["--project-id", "TEST_PROJECT", "--verbose"], - ) - - # Assert - assert result.exit_code == 0 - # Verify logging level was set - mock_logger.setLevel.assert_called_with(logging.DEBUG) - mock_get_logger.assert_called_with("apps.polarion.polarion_utils") - mock_utils_logger.setLevel.assert_called_with(logging.DEBUG) + with ( + patch("apps.polarion.polarion_verify_tc_requirements.LOGGER") as mock_logger, + patch("logging.getLogger") as mock_get_logger, + ): + mock_utils_logger = MagicMock() + mock_get_logger.return_value = mock_utils_logger + + # Act + result = self.runner.invoke( + has_verify, + ["--project-id", "TEST_PROJECT", "--verbose"], + ) + + # Assert + assert result.exit_code == 0 + # Verify logging level was set + mock_logger.setLevel.assert_called_with(logging.DEBUG) + mock_get_logger.assert_called_with("apps.polarion.polarion_utils") + mock_utils_logger.setLevel.assert_called_with(logging.DEBUG) @patch("apps.polarion.polarion_verify_tc_requirements.get_polarion_project_id") def test_command_uses_config_file_project_id(self, mock_get_project): @@ -361,19 +363,21 @@ def test_command_mixed_logging_scenarios(self, mock_validate, mock_find, mock_ge mock_find.return_value = test_ids mock_validate.return_value = missing_requirements - with patch("apps.polarion.polarion_verify_tc_requirements.LOGGER") as mock_logger: - with patch("sys.exit") as mock_exit: - # Act - self.runner.invoke( - has_verify, - ["--project-id", "TEST_PROJECT", "--verbose"], - ) - - # Assert - # sys.exit(1) should be called due to missing requirements - assert mock_exit.call_count >= 1 - assert 1 in [call.args[0] for call in mock_exit.call_args_list] - # Verify both debug and error logging were called - mock_logger.debug.assert_called_once_with(f"Checking following ids: {test_ids}") - mock_logger.error.assert_called_once_with(f"TestCases with missing requirement: {missing_requirements}") - mock_logger.setLevel.assert_called_with(logging.DEBUG) + with ( + patch("apps.polarion.polarion_verify_tc_requirements.LOGGER") as mock_logger, + patch("sys.exit") as mock_exit, + ): + # Act + self.runner.invoke( + has_verify, + ["--project-id", "TEST_PROJECT", "--verbose"], + ) + + # Assert + # sys.exit(1) should be called due to missing requirements + assert mock_exit.call_count >= 1 + assert 1 in [call.args[0] for call in mock_exit.call_args_list] + # Verify both debug and error logging were called + mock_logger.debug.assert_called_once_with(f"Checking following ids: {test_ids}") + mock_logger.error.assert_called_once_with(f"TestCases with missing requirement: {missing_requirements}") + mock_logger.setLevel.assert_called_with(logging.DEBUG) diff --git a/tests/unused_code/manifests/unused_code_file_for_test.py b/tests/unused_code/manifests/unused_code_file_for_test.py index 55d30be..90f0592 100644 --- a/tests/unused_code/manifests/unused_code_file_for_test.py +++ b/tests/unused_code/manifests/unused_code_file_for_test.py @@ -9,12 +9,10 @@ # Original functions for existing tests compatibility def unused_code_check_fail() -> None: """Original function for compatibility with existing tests.""" - pass def unused_code_check_file() -> None: """Original function for compatibility with existing tests.""" - pass def unused_code_namespace(): @@ -156,7 +154,6 @@ def unused_code_some_other_function(): Returns: str: The created namespace name. """ - pass def unused_code_edge_case_function(): @@ -172,7 +169,6 @@ def unused_code_edge_case_function(): Returns: None """ - pass def unused_code_function_with_legitimate_calls(): @@ -186,8 +182,7 @@ def unused_code_function_with_legitimate_calls(): unused_code_create_secret() if unused_code_validate_namespace(): - deploy_result = unused_code_deploy_app() - return deploy_result + return unused_code_deploy_app() return result @@ -215,4 +210,4 @@ def unused_code_unused_function_with_docs(): def unused_code_skip_with_comment() -> None: """Function that should be skipped due to comment.""" - pass # skip-unused-code + # skip-unused-code diff --git a/tests/unused_code/test_unused_code_coverage.py b/tests/unused_code/test_unused_code_coverage.py index 6587436..f1355fb 100644 --- a/tests/unused_code/test_unused_code_coverage.py +++ b/tests/unused_code/test_unused_code_coverage.py @@ -151,9 +151,9 @@ def test_is_documentation_pattern(line, is_doc): def test_git_grep_error(mocker): mocker.patch( "apps.unused_code.unused_code.subprocess.run", - side_effect=Exception("git error"), + side_effect=RuntimeError("git error"), ) - with pytest.raises(Exception): + with pytest.raises(RuntimeError): _git_grep("pattern")