fix: ERC handler fails on KiCad 9 schematics
Two issues with run_erc on KiCad 9: 1. kicad-cli returns non-zero exit code when ERC violations exist. The handler treated this as a command failure and returned early with success=false, even though valid JSON output was produced. Fix: check for output file existence instead of exit code. 2. KiCad 9 nests violations under sheets[].violations instead of (or in addition to) the top-level violations[] array used by KiCad 8. The handler only read the top-level array, reporting 0 violations on schematics with sub-sheets. Fix: iterate sheets[] and collect all nested violations. Both fixes are backward-compatible with KiCad 8. 6 unit tests added covering non-zero exit codes, KiCad 8 top-level violations, KiCad 9 sheets[] nesting, mixed structures, zero violations, and missing output files.
This commit is contained in:
@@ -2664,11 +2664,14 @@ class KiCADInterface:
|
||||
|
||||
result = subprocess.run(cmd, capture_output=True, text=True, timeout=120)
|
||||
|
||||
if result.returncode != 0:
|
||||
logger.error(f"ERC command failed: {result.stderr}")
|
||||
# kicad-cli returns non-zero when ERC violations are found —
|
||||
# this is normal, not an error. Only fail when no JSON was
|
||||
# produced (genuine CLI failure).
|
||||
if not os.path.exists(json_output) or os.path.getsize(json_output) == 0:
|
||||
logger.error(f"ERC command produced no output: {result.stderr}")
|
||||
return {
|
||||
"success": False,
|
||||
"message": "ERC command failed",
|
||||
"message": "ERC command failed - no output produced",
|
||||
"errorDetails": result.stderr,
|
||||
}
|
||||
|
||||
@@ -2678,7 +2681,14 @@ class KiCADInterface:
|
||||
violations = []
|
||||
severity_counts = {"error": 0, "warning": 0, "info": 0}
|
||||
|
||||
for v in erc_data.get("violations", []):
|
||||
# KiCad 9 nests violations under sheets[].violations
|
||||
# instead of (or in addition to) the top-level violations
|
||||
# array used by KiCad 8.
|
||||
all_violations = erc_data.get("violations", [])
|
||||
for sheet in erc_data.get("sheets", []):
|
||||
all_violations.extend(sheet.get("violations", []))
|
||||
|
||||
for v in all_violations:
|
||||
vseverity = v.get("severity", "error")
|
||||
items = v.get("items", [])
|
||||
loc = {}
|
||||
|
||||
248
tests/test_erc_handler.py
Normal file
248
tests/test_erc_handler.py
Normal file
@@ -0,0 +1,248 @@
|
||||
"""
|
||||
Tests for run_erc handler.
|
||||
|
||||
Covers:
|
||||
- Non-zero exit code acceptance (kicad-cli returns non-zero when violations exist)
|
||||
- KiCad 9 sheets[].violations JSON structure parsing
|
||||
- KiCad 8 top-level violations[] JSON structure (backward compat)
|
||||
- Missing/empty output file handling
|
||||
"""
|
||||
|
||||
import json
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent / "python"))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Shared fixture: KiCADInterface instance (no __init__, avoids pcbnew/IPC)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_iface() -> Any:
|
||||
with patch("kicad_interface.USE_IPC_BACKEND", False):
|
||||
from kicad_interface import KiCADInterface
|
||||
|
||||
iface = KiCADInterface.__new__(KiCADInterface)
|
||||
return iface
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def iface():
|
||||
return _make_iface()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Sample ERC JSON outputs
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# KiCad 8 style: violations at top level
|
||||
_ERC_KICAD8_JSON = {
|
||||
"violations": [
|
||||
{
|
||||
"type": "pin_not_connected",
|
||||
"severity": "error",
|
||||
"description": "Pin not connected",
|
||||
"items": [{"pos": {"x": 100.0, "y": 50.0}}],
|
||||
},
|
||||
{
|
||||
"type": "wire_dangling",
|
||||
"severity": "warning",
|
||||
"description": "Wire end not connected",
|
||||
"items": [{"pos": {"x": 200.0, "y": 75.0}}],
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
# KiCad 9 style: violations nested under sheets[]
|
||||
_ERC_KICAD9_JSON = {
|
||||
"violations": [],
|
||||
"sheets": [
|
||||
{
|
||||
"path": "/",
|
||||
"violations": [
|
||||
{
|
||||
"type": "pin_not_connected",
|
||||
"severity": "error",
|
||||
"description": "Pin not connected",
|
||||
"items": [{"pos": {"x": 10.0, "y": 20.0}}],
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
"path": "/sub-sheet-1",
|
||||
"violations": [
|
||||
{
|
||||
"type": "label_dangling",
|
||||
"severity": "error",
|
||||
"description": "Label not connected to anything",
|
||||
"items": [{"pos": {"x": 30.0, "y": 40.0}}],
|
||||
},
|
||||
{
|
||||
"type": "wire_dangling",
|
||||
"severity": "warning",
|
||||
"description": "Wire end not connected",
|
||||
"items": [{"pos": {"x": 50.0, "y": 60.0}}],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
# KiCad 9 with violations in both top-level and sheets (edge case)
|
||||
_ERC_MIXED_JSON = {
|
||||
"violations": [
|
||||
{
|
||||
"type": "power_pin_not_driven",
|
||||
"severity": "error",
|
||||
"description": "Power pin not driven",
|
||||
"items": [{"pos": {"x": 1.0, "y": 2.0}}],
|
||||
},
|
||||
],
|
||||
"sheets": [
|
||||
{
|
||||
"path": "/sub",
|
||||
"violations": [
|
||||
{
|
||||
"type": "pin_not_connected",
|
||||
"severity": "error",
|
||||
"description": "Pin not connected",
|
||||
"items": [{"pos": {"x": 3.0, "y": 4.0}}],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
def _mock_erc_run(erc_json: dict, returncode: int = 1):
|
||||
"""Create a mock subprocess.run that writes ERC JSON to the output file."""
|
||||
|
||||
def _side_effect(cmd, **kwargs):
|
||||
# Find the output path from the command args (--output <path>)
|
||||
output_idx = cmd.index("--output") + 1
|
||||
output_path = cmd[output_idx]
|
||||
with open(output_path, "w") as f:
|
||||
json.dump(erc_json, f)
|
||||
result = MagicMock()
|
||||
result.returncode = returncode
|
||||
result.stderr = ""
|
||||
return result
|
||||
|
||||
return _side_effect
|
||||
|
||||
|
||||
def _mock_erc_no_output(returncode: int = 2):
|
||||
"""Create a mock subprocess.run that produces no output file."""
|
||||
|
||||
def _side_effect(cmd, **kwargs):
|
||||
result = MagicMock()
|
||||
result.returncode = returncode
|
||||
result.stderr = "kicad-cli: error: schematic not found"
|
||||
return result
|
||||
|
||||
return _side_effect
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# Tests
|
||||
# ===========================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestERCNonZeroExitCode:
|
||||
"""kicad-cli returns non-zero when violations exist — this is not an error."""
|
||||
|
||||
def test_nonzero_returncode_with_valid_json_succeeds(self, iface, tmp_path):
|
||||
sch = tmp_path / "test.kicad_sch"
|
||||
sch.write_text("(kicad_sch)")
|
||||
|
||||
iface.design_rule_commands = MagicMock()
|
||||
iface.design_rule_commands._find_kicad_cli.return_value = "/usr/bin/kicad-cli"
|
||||
|
||||
with patch("subprocess.run", side_effect=_mock_erc_run(_ERC_KICAD8_JSON, returncode=1)):
|
||||
result = iface._handle_run_erc({"schematicPath": str(sch)})
|
||||
|
||||
assert result["success"] is True
|
||||
assert "2 violation" in result["message"]
|
||||
|
||||
def test_zero_returncode_no_violations(self, iface, tmp_path):
|
||||
sch = tmp_path / "test.kicad_sch"
|
||||
sch.write_text("(kicad_sch)")
|
||||
|
||||
iface.design_rule_commands = MagicMock()
|
||||
iface.design_rule_commands._find_kicad_cli.return_value = "/usr/bin/kicad-cli"
|
||||
|
||||
with patch("subprocess.run", side_effect=_mock_erc_run({"violations": []}, returncode=0)):
|
||||
result = iface._handle_run_erc({"schematicPath": str(sch)})
|
||||
|
||||
assert result["success"] is True
|
||||
assert "0 violation" in result["message"]
|
||||
|
||||
def test_no_output_file_fails(self, iface, tmp_path):
|
||||
sch = tmp_path / "test.kicad_sch"
|
||||
sch.write_text("(kicad_sch)")
|
||||
|
||||
iface.design_rule_commands = MagicMock()
|
||||
iface.design_rule_commands._find_kicad_cli.return_value = "/usr/bin/kicad-cli"
|
||||
|
||||
with patch("subprocess.run", side_effect=_mock_erc_no_output()):
|
||||
result = iface._handle_run_erc({"schematicPath": str(sch)})
|
||||
|
||||
assert result["success"] is False
|
||||
assert "no output" in result["message"].lower()
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestERCKicad9SheetsViolations:
|
||||
"""KiCad 9 nests violations under sheets[].violations."""
|
||||
|
||||
def test_kicad9_sheets_violations_collected(self, iface, tmp_path):
|
||||
sch = tmp_path / "test.kicad_sch"
|
||||
sch.write_text("(kicad_sch)")
|
||||
|
||||
iface.design_rule_commands = MagicMock()
|
||||
iface.design_rule_commands._find_kicad_cli.return_value = "/usr/bin/kicad-cli"
|
||||
|
||||
with patch("subprocess.run", side_effect=_mock_erc_run(_ERC_KICAD9_JSON)):
|
||||
result = iface._handle_run_erc({"schematicPath": str(sch)})
|
||||
|
||||
assert result["success"] is True
|
||||
assert "3 violation" in result["message"]
|
||||
assert result["summary"]["by_severity"]["error"] == 2
|
||||
assert result["summary"]["by_severity"]["warning"] == 1
|
||||
|
||||
def test_kicad8_top_level_violations_still_work(self, iface, tmp_path):
|
||||
sch = tmp_path / "test.kicad_sch"
|
||||
sch.write_text("(kicad_sch)")
|
||||
|
||||
iface.design_rule_commands = MagicMock()
|
||||
iface.design_rule_commands._find_kicad_cli.return_value = "/usr/bin/kicad-cli"
|
||||
|
||||
with patch("subprocess.run", side_effect=_mock_erc_run(_ERC_KICAD8_JSON)):
|
||||
result = iface._handle_run_erc({"schematicPath": str(sch)})
|
||||
|
||||
assert result["success"] is True
|
||||
assert "2 violation" in result["message"]
|
||||
assert result["summary"]["by_severity"]["error"] == 1
|
||||
assert result["summary"]["by_severity"]["warning"] == 1
|
||||
|
||||
def test_mixed_top_level_and_sheets_violations(self, iface, tmp_path):
|
||||
sch = tmp_path / "test.kicad_sch"
|
||||
sch.write_text("(kicad_sch)")
|
||||
|
||||
iface.design_rule_commands = MagicMock()
|
||||
iface.design_rule_commands._find_kicad_cli.return_value = "/usr/bin/kicad-cli"
|
||||
|
||||
with patch("subprocess.run", side_effect=_mock_erc_run(_ERC_MIXED_JSON)):
|
||||
result = iface._handle_run_erc({"schematicPath": str(sch)})
|
||||
|
||||
assert result["success"] is True
|
||||
# 1 top-level + 1 from sheets = 2 total
|
||||
assert "2 violation" in result["message"]
|
||||
assert result["summary"]["by_severity"]["error"] == 2
|
||||
Reference in New Issue
Block a user