Merge pull request #578 from d3flex/style/ty_checks

chore: add type checks in ci for python

Author: mergify[bot]

Makefile

@@ -58,7 +58,7 @@ test-yaml:
 	@which yamllint >/dev/null 2>&1 || echo "Command 'yamllint' not found, can not execute YAML syntax checks"
 	yamllint --strict $$(git ls-files "*.yml" "*.yaml" ":!external/")
 
-checkstyle-python: check-ruff check-conventions
+checkstyle-python: check-ruff check-conventions check-ty
 check-ruff:
 	@which ruff >/dev/null 2>&1 || echo "Command 'ruff' not found, can not execute python style checks"
 	@if [ -n "$(PY_FILES)" ]; then ruff format --check $(PY_FILES) && ruff check $(PY_FILES); fi
@@ -70,6 +70,10 @@ check-conventions:
 		exit 1; \
 	fi
 
+.PHONY: check-ty
+check-ty: ## Run ty type checker
+	uv run ty check
+
 check-code-health:
 	@echo "Checking code health…"
 	@vulture $$(git ls-files "**.py") --min-confidence 80

check-netbox-unused-machine-power.py

@@ -7,15 +7,21 @@
 indicating it may still be powered on despite being decommissioned or unused.
 """
 
+from __future__ import annotations
+
 import os
 import re
 import sys
+from typing import TYPE_CHECKING
 from urllib.parse import urlparse
 
 import netsnmp
 import pynetbox
 from pynetbox.models import dcim
 
+if TYPE_CHECKING:
+    from pynetbox.models import dcim
+
 BACHMANN_RELAY_ON = 19  # Bachmann PDU relay status value for "on"
 
 verbose = os.environ.get("VERBOSE") == "1"
@@ -74,7 +80,7 @@ def green(s: str) -> str:
     return f"\x1b[32m{s}\x1b[0m"
 
 
-def print_device(device: dcim.Devices, dev_pdu_power: dict, watts: int) -> None:
+def print_device(device: dcim.Devices, dev_pdu_power: dict[str, tuple[str, str]], watts: int) -> None:
     """Report device power consumption per PDU outlet for human review."""
     s = "  " if verbose else ""
     pdu_power = " ".join([f"{h}:{green(p) if s else red(p)}={w}W" for (h, p), (w, s) in dev_pdu_power.items()])

pyproject.toml

@@ -34,6 +34,7 @@ dev = [
     "radon",
     "ruff",
     "vulture",
+    "ty>=0.0.21",
 ]
 
 [build-system]
@@ -145,6 +146,15 @@ allowed-unresolved-imports = ["netsnmp"]
 author = "SUSE LLC"
 notice-rgx = "Copyright"
 
+[tool.ty.rules]
+# sh uses __getattr__ to dynamically wrap system commands (e.g. ping), so its
+# members are not statically resolvable. netsnmp is a C extension installed as
+# a system package (python3-netsnmp) and has no stubs available on PyPI.
+unresolved-import = "ignore"
+
 [tool.ruff.lint.flake8-tidy-imports.banned-api]
 "os.system".msg = "Use `subprocess.run` or `sh` library instead."
 "subprocess.call".msg = "Use `subprocess.run` with `check=True`."
+
+[tool.ty.terminal]
+error-on-warning = true

tests/test_openqa_bats_review.py

@@ -8,12 +8,15 @@
 import importlib.util
 import pathlib
 import sys
-from typing import Any
+from typing import TYPE_CHECKING, Any
 from unittest.mock import Mock
 
 import pytest
 from requests.exceptions import RequestException
 
+if TYPE_CHECKING:
+    from pytest_mock import MockerFixture
+
 # Load the script as module "bats_review" (the file is named `openqa-bats-review`)
 rootpath = pathlib.Path(__file__).parent.parent.resolve()
 loader = importlib.machinery.SourceFileLoader("bats_review", f"{rootpath}/openqa-bats-review")
@@ -29,7 +32,7 @@
 #
 
 
-def test_get_file_success(mocker: pytest.MockerFixture) -> None:
+def test_get_file_success(mocker: MockerFixture) -> None:
     mock_session = mocker.patch("bats_review.session")
     resp = Mock()
     resp.text = "hello"
@@ -46,7 +49,7 @@ def test_get_file_success(mocker: pytest.MockerFixture) -> None:
     resp.raise_for_status.assert_called_once()
 
 
-def test_get_file_request_exception(mocker: pytest.MockerFixture) -> None:
+def test_get_file_request_exception(mocker: MockerFixture) -> None:
     mock_session = mocker.patch("bats_review.session")
     mock_log = mocker.patch("bats_review.log")
     mock_session.get.side_effect = RequestException("network")
@@ -56,7 +59,7 @@ def test_get_file_request_exception(mocker: pytest.MockerFixture) -> None:
     mock_log.exception.assert_called_once()
 
 
-def test_get_job_success(mocker: pytest.MockerFixture) -> None:
+def test_get_job_success(mocker: MockerFixture) -> None:
     with contextlib.suppress(Exception):
         bats_review.get_job.cache_clear()
     mock_session = mocker.patch("bats_review.session")
@@ -74,7 +77,7 @@ def test_get_job_success(mocker: pytest.MockerFixture) -> None:
     )
 
 
-def test_get_job_request_exception(mocker: pytest.MockerFixture) -> None:
+def test_get_job_request_exception(mocker: MockerFixture) -> None:
     with contextlib.suppress(Exception):
         bats_review.get_job.cache_clear()
     mock_session = mocker.patch("bats_review.session")
@@ -86,7 +89,7 @@ def test_get_job_request_exception(mocker: pytest.MockerFixture) -> None:
     mock_log.exception.assert_called_once()
 
 
-def test_grep_failures_success(mocker: pytest.MockerFixture) -> None:
+def test_grep_failures_success(mocker: MockerFixture) -> None:
     mock_get_file = mocker.patch("bats_review.get_file")
     # one passing, one failing testcase (with classname)
     mock_get_file.return_value = """
@@ -101,7 +104,7 @@ def test_grep_failures_success(mocker: pytest.MockerFixture) -> None:
     assert result == {"suite1:failing_test"}
 
 
-def test_grep_failures_malformed(mocker: pytest.MockerFixture) -> None:
+def test_grep_failures_malformed(mocker: MockerFixture) -> None:
     mock_get_file = mocker.patch("bats_review.get_file")
     mock_log = mocker.patch("bats_review.log")
     mock_get_file.return_value = "<this is not xml"
@@ -112,15 +115,15 @@ def test_grep_failures_malformed(mocker: pytest.MockerFixture) -> None:
     mock_log.exception.assert_called_once()
 
 
-def test_process_logs_single_file(mocker: pytest.MockerFixture) -> None:
+def test_process_logs_single_file(mocker: MockerFixture) -> None:
     mock_grep = mocker.patch("bats_review.grep_failures")
     mock_grep.return_value = {"a", "b"}
     res = bats_review.process_logs(["http://example.com/a.xml"])
     assert res == {"a", "b"}
     mock_grep.assert_called_once_with("http://example.com/a.xml")
 
 
-def test_process_logs_multiple_files(mocker: pytest.MockerFixture) -> None:
+def test_process_logs_multiple_files(mocker: MockerFixture) -> None:
     mock_executor_class = mocker.patch("bats_review.ThreadPoolExecutor")
     # build fake executor that returns map -> iterator of sets
     fake_executor = Mock()
@@ -134,7 +137,7 @@ def test_process_logs_multiple_files(mocker: pytest.MockerFixture) -> None:
     fake_executor.map.assert_called_once()
 
 
-def test_resolve_clone_chain_single(mocker: pytest.MockerFixture) -> None:
+def test_resolve_clone_chain_single(mocker: MockerFixture) -> None:
     with contextlib.suppress(Exception):
         bats_review.get_job.cache_clear()
     mock_get_job = mocker.patch("bats_review.get_job")
@@ -144,7 +147,7 @@ def test_resolve_clone_chain_single(mocker: pytest.MockerFixture) -> None:
     mock_get_job.assert_called_once_with("http://openqa/api/v1/jobs/123/details")
 
 
-def test_resolve_clone_chain_multiple(mocker: pytest.MockerFixture) -> None:
+def test_resolve_clone_chain_multiple(mocker: MockerFixture) -> None:
     with contextlib.suppress(Exception):
         bats_review.get_job.cache_clear()
     mock_get_job = mocker.patch("bats_review.get_job")
@@ -167,7 +170,7 @@ def side(url: str) -> dict[str, Any] | None:
 # main
 
 
-def test_main_no_clones(mocker: pytest.MockerFixture) -> None:
+def test_main_no_clones(mocker: MockerFixture) -> None:
     with contextlib.suppress(Exception):
         bats_review.get_job.cache_clear()
     mock_resolve = mocker.patch("bats_review.resolve_clone_chain")
@@ -179,7 +182,7 @@ def test_main_no_clones(mocker: pytest.MockerFixture) -> None:
     mock_log.info.assert_called_with("No clones. Exiting")
 
 
-def test_main_no_common_failures(mocker: pytest.MockerFixture) -> None:
+def test_main_no_common_failures(mocker: MockerFixture) -> None:
     """Two jobs in chain; each produces different failures -> no common failures.
 
     main should log Tagging as PASSED.
@@ -208,7 +211,7 @@ def job_resp(url: str) -> dict[str, Any]:
     mock_log.info.assert_called_with("No common failures across clone chain. Tagging as PASSED.")
 
 
-def test_main_insufficient_logs(mocker: pytest.MockerFixture) -> None:
+def test_main_insufficient_logs(mocker: MockerFixture) -> None:
     """If jobs do not have the expected number of logs (e.g. podman expects 4 but provides 2).
 
     main should log the 'only X logs' messages for each job and eventually exit(0).
@@ -235,13 +238,13 @@ def job_resp(url: str) -> dict[str, Any]:
     mock_log.info.assert_any_call("No logs found in chain. Exiting")
 
 
-def test_parse_args_success(mocker: pytest.MockerFixture) -> None:
+def test_parse_args_success(mocker: MockerFixture) -> None:
     mocker.patch("sys.argv", ["script.py", "http://example.com/tests/123"])
     args = bats_review.parse_args()
     assert args.url == "http://example.com/tests/123"
 
 
-def test_parse_args_missing_url(mocker: pytest.MockerFixture) -> None:
+def test_parse_args_missing_url(mocker: MockerFixture) -> None:
     mocker.patch("sys.argv", ["script.py"])
     with pytest.raises(SystemExit):
         bats_review.parse_args()
@@ -250,7 +253,7 @@ def test_parse_args_missing_url(mocker: pytest.MockerFixture) -> None:
 # Integration test
 
 
-def test_full_workflow_no_common_failures(mocker: pytest.MockerFixture) -> None:
+def test_full_workflow_no_common_failures(mocker: MockerFixture) -> None:
     """Patch session.get to simulate two jobs each with a different failing testcase.
 
     Asserts that the script decides to tag as PASSED (dry_run) when there are no common failures.

tests/test_openqa_llm_investigate.py

@@ -8,12 +8,15 @@
 import logging
 import pathlib
 import sys
-from typing import Any
+from typing import TYPE_CHECKING, Any
 from unittest.mock import MagicMock, Mock
 
 import httpx
 import pytest
 
+if TYPE_CHECKING:
+    from pytest_mock import MockerFixture
+
 # Load the script as module "llm_investigate" (the file is named `openqa-llm-investigate`)
 rootpath = pathlib.Path(__file__).parent.parent.resolve()
 loader = importlib.machinery.SourceFileLoader("llm_investigate", f"{rootpath}/openqa-llm-investigate")
@@ -72,7 +75,7 @@ def test_fetch_text_failure() -> None:
     assert not res
 
 
-def test_post_comment(mocker: pytest.MockerFixture) -> None:
+def test_post_comment(mocker: MockerFixture) -> None:
     mock_run = mocker.patch("llm_investigate.subprocess.run")
     llm_investigate.post_comment("http://base", "123", "test comment")
     mock_run.assert_called_once()
@@ -82,7 +85,7 @@ def test_post_comment(mocker: pytest.MockerFixture) -> None:
     assert "text=test comment" in args
 
 
-def setup_mock_client(mocker: pytest.MockerFixture, overrides: dict[str, Any] | None = None) -> MagicMock:
+def setup_mock_client(mocker: MockerFixture, overrides: dict[str, Any] | None = None) -> MagicMock:
     mock_client_class = mocker.patch("llm_investigate.httpx.Client")
     mock_client = MagicMock()
     mock_client_class.return_value.__enter__.return_value = mock_client
@@ -143,7 +146,7 @@ def mock_post_call(url: str, json: dict[str, Any] | None = None, *args: Any, **k
     return mock_client
 
 
-def test_investigate_cmd(mocker: pytest.MockerFixture) -> None:
+def test_investigate_cmd(mocker: MockerFixture) -> None:
     mock_post = mocker.patch("llm_investigate.post_comment")
     mock_print = mocker.patch("builtins.print")
     setup_mock_client(mocker)
@@ -153,7 +156,7 @@ def test_investigate_cmd(mocker: pytest.MockerFixture) -> None:
     assert "BISECT: YES" in mock_post.call_args[0][2]
 
 
-def test_investigate_cmd_passed_job(mocker: pytest.MockerFixture) -> None:
+def test_investigate_cmd_passed_job(mocker: MockerFixture) -> None:
     mock_print = mocker.patch("builtins.print")
     setup_mock_client(mocker, overrides={"api/v1/jobs": {"job": {"id": 123, "result": "passed"}}})
 
@@ -164,7 +167,7 @@ def test_investigate_cmd_passed_job(mocker: pytest.MockerFixture) -> None:
     mock_print.assert_not_called()
 
 
-def test_investigate_cmd_softfailed_job(mocker: pytest.MockerFixture) -> None:
+def test_investigate_cmd_softfailed_job(mocker: MockerFixture) -> None:
     mock_print = mocker.patch("builtins.print")
     setup_mock_client(mocker, overrides={"api/v1/jobs": {"job": {"id": 123, "result": "softfailed"}}})
 
@@ -175,7 +178,7 @@ def test_investigate_cmd_softfailed_job(mocker: pytest.MockerFixture) -> None:
     mock_print.assert_not_called()
 
 
-def test_investigate_cmd_already_commented(mocker: pytest.MockerFixture) -> None:
+def test_investigate_cmd_already_commented(mocker: MockerFixture) -> None:
     mock_log = mocker.patch("llm_investigate.log")
     setup_mock_client(mocker, overrides={"comments": [{"text": "**LLM Investigation summary:** already done"}]})
 
@@ -187,7 +190,7 @@ def test_investigate_cmd_already_commented(mocker: pytest.MockerFixture) -> None
     assert "already has an LLM investigation summary" in mock_log.info.call_args[0][0]
 
 
-def test_investigate_cmd_already_commented_with_force(mocker: pytest.MockerFixture) -> None:
+def test_investigate_cmd_already_commented_with_force(mocker: MockerFixture) -> None:
     mock_post = mocker.patch("llm_investigate.post_comment")
     mock_print = mocker.patch("builtins.print")
     setup_mock_client(mocker, overrides={"comments": [{"text": "**LLM Investigation summary:** already done"}]})
@@ -199,7 +202,7 @@ def test_investigate_cmd_already_commented_with_force(mocker: pytest.MockerFixtu
     assert "BISECT: YES" in mock_post.call_args[0][2]
 
 
-def test_investigate_cmd_dry_run(mocker: pytest.MockerFixture) -> None:
+def test_investigate_cmd_dry_run(mocker: MockerFixture) -> None:
     mock_post = mocker.patch("llm_investigate.post_comment")
     mock_print = mocker.patch("builtins.print")
     client = setup_mock_client(mocker)
@@ -214,7 +217,7 @@ def test_investigate_cmd_dry_run(mocker: pytest.MockerFixture) -> None:
     mock_print.assert_any_call("**LLM Investigation summary:**\n\nBISECT: NO. Already known.")
 
 
-def test_investigate_cmd_connection_error(mocker: pytest.MockerFixture) -> None:
+def test_investigate_cmd_connection_error(mocker: MockerFixture) -> None:
     mock_log = mocker.patch("llm_investigate.log")
     client = setup_mock_client(mocker)
     client.post.side_effect = httpx.ConnectError("Connection refused")
@@ -227,7 +230,7 @@ def test_investigate_cmd_connection_error(mocker: pytest.MockerFixture) -> None:
     assert "Could not connect to LLM server" in mock_log.error.call_args[0][0]
 
 
-def test_investigate_logging_levels(mocker: pytest.MockerFixture) -> None:
+def test_investigate_logging_levels(mocker: MockerFixture) -> None:
     mock_basic_config = mocker.patch("llm_investigate.logging.basicConfig")
     mocker.patch("llm_investigate.httpx.Client")
     mock_fetch = mocker.patch("llm_investigate.fetch_json")

tests/test_trigger_bisect_jobs.py

@@ -9,7 +9,7 @@
 import re
 import subprocess  # noqa: S404
 from argparse import Namespace
-from typing import Any
+from typing import Any, cast
 from unittest.mock import MagicMock, call, patch
 from urllib.parse import urlparse
 
@@ -20,7 +20,8 @@
 
 loader = importlib.machinery.SourceFileLoader("openqa", f"{rootpath}/openqa-trigger-bisect-jobs")
 spec = importlib.util.spec_from_loader(loader.name, loader)
-openqa = importlib.util.module_from_spec(spec)
+assert spec is not None
+openqa = cast("Any", importlib.util.module_from_spec(spec))
 loader.exec_module(openqa)
 
 Incident = openqa.Incident
@@ -36,11 +37,11 @@ def args_factory() -> Namespace:
 
 def mocked_fetch_url(url: str, request_type: str = "text") -> Any:
     content = ""
-    url = urlparse(url)
+    parsed = urlparse(url)
 
-    if url.scheme in {"http", "https"}:
-        path = url.geturl()
-        path = path[len(url.scheme) + 3 :]
+    if parsed.scheme in {"http", "https"}:
+        path = parsed.geturl()
+        path = path[len(parsed.scheme) + 3 :]
         path = "tests/data/python-requests/" + path
         content = pathlib.Path(path).read_text(encoding="utf-8")
     if request_type == "json":