chore: enable strict mypy checks and fix pre-commit mypy hook

Add type annotations to all previously untyped functions and remove 9
suppressed error codes (call-arg, assignment, return-value, operator,
has-type, dict-item, misc, list-item, annotation-unchecked) by fixing
the underlying type issues.

Add [[tool.mypy.overrides]] with ignore_missing_imports for KiCAD-specific
modules (pcbnew, sexpdata, skip, cairosvg, kipy, PIL) so the pre-commit
mypy hook passes in its isolated venv. Add types-requests and pytest to
additional_dependencies in .pre-commit-config.yaml.

Also fixes several real bugs uncovered by stricter checks: incorrect static
calls to instance methods in swig_backend, wrong return type on get_size,
missing value param in BoardAPI.place_component, variable shadowing in
kicad_process.py, unqualified LibraryManager reference in kicad_interface,
and missing top-level Path import.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Eugene Mikhantyev
2026-04-05 23:48:35 +01:00
parent 5a2b481db3
commit 9b1024a8f3
35 changed files with 4665 additions and 4610 deletions

View File

@@ -12,6 +12,7 @@ Covers:
import sys
from pathlib import Path
from typing import Any
from unittest.mock import MagicMock, patch
import pytest
@@ -33,7 +34,7 @@ pcbnew_mock = sys.modules["pcbnew"]
@pytest.fixture(autouse=True)
def reset_pcbnew_mock():
def reset_pcbnew_mock() -> Any:
"""Reset pcbnew mock before each test."""
pcbnew_mock.reset_mock()
pcbnew_mock.ExportSpecctraDSN.side_effect = None
@@ -44,7 +45,7 @@ def reset_pcbnew_mock():
@pytest.fixture
def mock_board():
def mock_board() -> Any:
board = MagicMock()
board.GetFileName.return_value = "/tmp/test_project/test.kicad_pcb"
board.GetTracks.return_value = []
@@ -52,16 +53,16 @@ def mock_board():
@pytest.fixture
def cmds(mock_board):
def cmds(mock_board: Any) -> Any:
return FreeroutingCommands(board=mock_board)
@pytest.fixture
def cmds_no_board():
def cmds_no_board() -> Any:
return FreeroutingCommands(board=None)
def _patch_direct_java():
def _patch_direct_java() -> Any:
"""Patch to simulate Java 21+ available locally."""
return patch.object(
FreeroutingCommands,
@@ -70,7 +71,7 @@ def _patch_direct_java():
)
def _patch_docker_mode():
def _patch_docker_mode() -> Any:
"""Patch to simulate Docker execution mode."""
return patch.object(
FreeroutingCommands,
@@ -79,7 +80,7 @@ def _patch_docker_mode():
)
def _patch_no_runtime():
def _patch_no_runtime() -> Any:
"""Patch to simulate no Java and no Docker."""
return patch.object(
FreeroutingCommands,
@@ -97,7 +98,7 @@ def _patch_no_runtime():
class TestCheckFreerouting:
def test_no_java_no_docker(self, cmds):
def test_no_java_no_docker(self, cmds: Any) -> None:
with (
patch("commands.freerouting._find_java", return_value=None),
patch(
@@ -112,7 +113,7 @@ class TestCheckFreerouting:
assert result["ready"] is False
assert result["execution_mode"] == "none"
def test_java_too_old_docker_available(self, cmds, tmp_path):
def test_java_too_old_docker_available(self, cmds: Any, tmp_path: Any) -> None:
jar = tmp_path / "freerouting.jar"
jar.touch()
with (
@@ -135,7 +136,7 @@ class TestCheckFreerouting:
assert result["ready"] is True
assert result["execution_mode"] == "docker"
def test_java_21_direct(self, cmds, tmp_path):
def test_java_21_direct(self, cmds: Any, tmp_path: Any) -> None:
jar = tmp_path / "freerouting.jar"
jar.touch()
with (
@@ -165,12 +166,12 @@ class TestCheckFreerouting:
class TestExportDsn:
def test_no_board(self, cmds_no_board):
def test_no_board(self, cmds_no_board: Any) -> None:
result = cmds_no_board.export_dsn({})
assert result["success"] is False
assert "No board" in result["message"]
def test_export_success(self, cmds, tmp_path):
def test_export_success(self, cmds: Any, tmp_path: Any) -> None:
board_path = str(tmp_path / "test.kicad_pcb")
dsn_path = str(tmp_path / "test.dsn")
cmds.board.GetFileName.return_value = board_path
@@ -182,7 +183,7 @@ class TestExportDsn:
assert result["success"] is True
assert result["path"] == dsn_path
def test_export_custom_path(self, cmds, tmp_path):
def test_export_custom_path(self, cmds: Any, tmp_path: Any) -> None:
output = str(tmp_path / "custom.dsn")
pcbnew_mock.ExportSpecctraDSN.return_value = True
Path(output).write_text("(pcb test)")
@@ -191,7 +192,7 @@ class TestExportDsn:
assert result["success"] is True
assert result["path"] == output
def test_export_failure(self, cmds):
def test_export_failure(self, cmds: Any) -> None:
pcbnew_mock.ExportSpecctraDSN.side_effect = Exception("DSN error")
result = cmds.export_dsn({})
assert result["success"] is False
@@ -204,22 +205,22 @@ class TestExportDsn:
class TestImportSes:
def test_no_board(self, cmds_no_board):
def test_no_board(self, cmds_no_board: Any) -> None:
result = cmds_no_board.import_ses({"sesPath": "/tmp/test.ses"})
assert result["success"] is False
assert "No board" in result["message"]
def test_missing_ses_path(self, cmds):
def test_missing_ses_path(self, cmds: Any) -> None:
result = cmds.import_ses({})
assert result["success"] is False
assert "Missing sesPath" in result["message"]
def test_ses_file_not_found(self, cmds):
def test_ses_file_not_found(self, cmds: Any) -> None:
result = cmds.import_ses({"sesPath": "/nonexistent/test.ses"})
assert result["success"] is False
assert "not found" in result["message"]
def test_import_success(self, cmds, tmp_path):
def test_import_success(self, cmds: Any, tmp_path: Any) -> None:
ses_file = tmp_path / "test.ses"
ses_file.write_text("(session test)")
@@ -229,7 +230,7 @@ class TestImportSes:
result = cmds.import_ses({"sesPath": str(ses_file)})
assert result["success"] is True
def test_import_failure(self, cmds, tmp_path):
def test_import_failure(self, cmds: Any, tmp_path: Any) -> None:
ses_file = tmp_path / "test.ses"
ses_file.write_text("(session test)")
@@ -245,12 +246,12 @@ class TestImportSes:
class TestAutoroute:
def test_no_board(self, cmds_no_board):
def test_no_board(self, cmds_no_board: Any) -> None:
result = cmds_no_board.autoroute({})
assert result["success"] is False
assert "No board" in result["message"]
def test_no_runtime(self, cmds, tmp_path):
def test_no_runtime(self, cmds: Any, tmp_path: Any) -> None:
jar = tmp_path / "freerouting.jar"
jar.touch()
with _patch_no_runtime():
@@ -258,13 +259,13 @@ class TestAutoroute:
assert result["success"] is False
assert "No suitable Java runtime" in result["message"]
def test_no_jar(self, cmds):
def test_no_jar(self, cmds: Any) -> None:
result = cmds.autoroute({"freeroutingJar": "/nonexistent/freerouting.jar"})
assert result["success"] is False
assert "JAR not found" in result["message"]
@patch("commands.freerouting.subprocess.run")
def test_dsn_export_fails(self, mock_run, cmds, tmp_path):
def test_dsn_export_fails(self, mock_run: Any, cmds: Any, tmp_path: Any) -> None:
jar = tmp_path / "freerouting.jar"
jar.touch()
@@ -276,7 +277,7 @@ class TestAutoroute:
assert "DSN export failed" in result["message"]
@patch("commands.freerouting.subprocess.run")
def test_freerouting_timeout(self, mock_run, cmds, tmp_path):
def test_freerouting_timeout(self, mock_run: Any, cmds: Any, tmp_path: Any) -> None:
import subprocess
jar = tmp_path / "freerouting.jar"
@@ -300,7 +301,7 @@ class TestAutoroute:
assert "timed out" in result["message"]
@patch("commands.freerouting.subprocess.run")
def test_full_success_direct(self, mock_run, cmds, tmp_path):
def test_full_success_direct(self, mock_run: Any, cmds: Any, tmp_path: Any) -> None:
jar = tmp_path / "freerouting.jar"
jar.touch()
board_dir = tmp_path / "project"
@@ -336,7 +337,7 @@ class TestAutoroute:
assert "elapsed_seconds" in result
@patch("commands.freerouting.subprocess.run")
def test_full_success_docker(self, mock_run, cmds, tmp_path):
def test_full_success_docker(self, mock_run: Any, cmds: Any, tmp_path: Any) -> None:
jar = tmp_path / "freerouting.jar"
jar.touch()
board_dir = tmp_path / "project"
@@ -375,7 +376,7 @@ class TestAutoroute:
assert "--rm" in call_args
@patch("commands.freerouting.subprocess.run")
def test_freerouting_nonzero_exit(self, mock_run, cmds, tmp_path):
def test_freerouting_nonzero_exit(self, mock_run: Any, cmds: Any, tmp_path: Any) -> None:
jar = tmp_path / "freerouting.jar"
jar.touch()
board_dir = tmp_path / "project"
@@ -404,14 +405,14 @@ class TestAutoroute:
class TestFindJava:
def test_finds_via_which(self):
def test_finds_via_which(self) -> None:
with patch(
"commands.freerouting.shutil.which",
return_value="/usr/bin/java",
):
assert _find_java() == "/usr/bin/java"
def test_none_when_not_found(self):
def test_none_when_not_found(self) -> None:
with (
patch(
"commands.freerouting.shutil.which",
@@ -423,21 +424,21 @@ class TestFindJava:
class TestFindDocker:
def test_finds_docker(self):
def test_finds_docker(self) -> None:
with patch(
"commands.freerouting.shutil.which",
side_effect=lambda x: "/usr/bin/docker" if x == "docker" else None,
):
assert _find_docker() == "/usr/bin/docker"
def test_finds_podman(self):
def test_finds_podman(self) -> None:
with patch(
"commands.freerouting.shutil.which",
side_effect=lambda x: "/usr/bin/podman" if x == "podman" else None,
):
assert _find_docker() == "/usr/bin/podman"
def test_none_when_not_found(self):
def test_none_when_not_found(self) -> None:
with patch(
"commands.freerouting.shutil.which",
return_value=None,
@@ -446,7 +447,7 @@ class TestFindDocker:
class TestDockerAvailable:
def test_docker_found(self):
def test_docker_found(self) -> None:
with (
patch(
"commands.freerouting._find_docker",
@@ -457,14 +458,14 @@ class TestDockerAvailable:
mock_run.return_value = MagicMock(returncode=0)
assert _docker_available() is True
def test_docker_not_installed(self):
def test_docker_not_installed(self) -> None:
with patch(
"commands.freerouting._find_docker",
return_value=None,
):
assert _docker_available() is False
def test_docker_not_running(self):
def test_docker_not_running(self) -> None:
with (
patch(
"commands.freerouting._find_docker",
@@ -477,17 +478,17 @@ class TestDockerAvailable:
class TestJavaVersionOk:
def test_java_21(self):
def test_java_21(self) -> None:
with patch("commands.freerouting.subprocess.run") as mock_run:
mock_run.return_value = MagicMock(stderr='openjdk version "21.0.1"', stdout="")
assert _java_version_ok("/usr/bin/java") is True
def test_java_17(self):
def test_java_17(self) -> None:
with patch("commands.freerouting.subprocess.run") as mock_run:
mock_run.return_value = MagicMock(stderr='openjdk version "17.0.18"', stdout="")
assert _java_version_ok("/usr/bin/java") is False
def test_java_error(self):
def test_java_error(self) -> None:
with patch(
"commands.freerouting.subprocess.run",
side_effect=Exception("not found"),