""" Regression tests for edit_schematic_component newReference: must also update the (reference "...") leaves inside the symbol's (instances) → (project) → (path) subtree, not just the (property "Reference" "...") field. Background ---------- A KiCad schematic stores a placed symbol's reference designator in two places: (symbol (lib_id "Device:R") (uuid "abcd-1234-…") ... (property "Reference" "R5" ← visible-in-eeschema field (at … …) (effects …) ) ... (instances (project "MyProject" (path "/sheet-uuid/symbol-uuid" (reference "R5") ← used by netlist + PCB sync (unit 1) ) ) ) ) Before the fix, `edit_schematic_component` with `newReference` updated only the (property "Reference" ...) field. The (reference "...") inside (instances) stayed on the old value. eeschema rendered the new ref correctly and ERC passed, but netlist export and "Update PCB from Schematic" both read from (instances) and silently used the OLD reference — producing destructive PCB diffs on what users thought was a clean rename. The fix walks the (instances) subtree after updating the property field and replaces every (reference "OLD") leaf with (reference "NEW"). """ import sys from pathlib import Path from typing import Any from unittest.mock import MagicMock import pytest sys.path.insert(0, str(Path(__file__).parent.parent / "python")) PLACED_RESISTOR_WITH_INSTANCES = """\ (symbol (lib_id "Device:R") (at 50 50 0) (unit 1) (in_bom yes) (on_board yes) (dnp no) (uuid "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee") (property "Reference" "R1" (at 51.27 47.46 0) (effects (font (size 1.27 1.27))) ) (property "Value" "10k" (at 51.27 52.54 0) (effects (font (size 1.27 1.27))) ) (property "Footprint" "Resistor_SMD:R_0603_1608Metric" (at 50 50 0) (effects (font (size 1.27 1.27)) hide) ) (property "Datasheet" "~" (at 50 50 0) (effects (font (size 1.27 1.27)) hide) ) (instances (project "MyProject" (path "/abcdef12-3456-7890-abcd-ef1234567890/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" (reference "R1") (unit 1) ) ) ) ) """ PLACED_RESISTOR_HIERARCHICAL = """\ (symbol (lib_id "Device:R") (at 50 50 0) (unit 1) (in_bom yes) (on_board yes) (dnp no) (uuid "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee") (property "Reference" "R1" (at 51.27 47.46 0) (effects (font (size 1.27 1.27))) ) (property "Value" "10k" (at 51.27 52.54 0) (effects (font (size 1.27 1.27))) ) (property "Footprint" "Resistor_SMD:R_0603_1608Metric" (at 50 50 0) (effects (font (size 1.27 1.27)) hide) ) (property "Datasheet" "~" (at 50 50 0) (effects (font (size 1.27 1.27)) hide) ) (instances (project "MyProject" (path "/sheet-a-uuid/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" (reference "R1") (unit 1) ) (path "/sheet-b-uuid/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" (reference "R1") (unit 1) ) ) ) ) """ PLACED_RESISTOR_NO_INSTANCES = """\ (symbol (lib_id "Device:R") (at 50 50 0) (unit 1) (in_bom yes) (on_board yes) (dnp no) (uuid "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee") (property "Reference" "R1" (at 51.27 47.46 0) (effects (font (size 1.27 1.27))) ) (property "Value" "10k" (at 51.27 52.54 0) (effects (font (size 1.27 1.27))) ) (property "Footprint" "Resistor_SMD:R_0603_1608Metric" (at 50 50 0) (effects (font (size 1.27 1.27)) hide) ) (property "Datasheet" "~" (at 50 50 0) (effects (font (size 1.27 1.27)) hide) ) ) """ def _write_schematic(tmp_path: Path, placed_block: str) -> Path: """Build a minimal kicad_sch wrapper around one placed-symbol block.""" sch = tmp_path / "test.kicad_sch" sch.write_text( "(kicad_sch\n" ' (version 20260306)\n' ' (generator "eeschema")\n' ' (uuid "abcdef12-3456-7890-abcd-ef1234567890")\n' ' (paper "A4")\n' ' (lib_symbols)\n' f"{placed_block}" ")\n", encoding="utf-8", ) return sch def _interface() -> Any: from kicad_interface import KiCADInterface iface = KiCADInterface.__new__(KiCADInterface) # _handle_edit_schematic_component does not require backend setup since it # operates on the .kicad_sch file directly via text-based editing. iface._update_command_handlers = MagicMock() return iface @pytest.mark.unit class TestEditSchematicComponentInstancesReference: """The fix: newReference must update (reference "...") inside (instances) too.""" def test_instances_reference_updated_on_rename(self, tmp_path: Any) -> None: """End-to-end: rename R1→R5, both property and instances must reflect R5.""" sch = _write_schematic(tmp_path, PLACED_RESISTOR_WITH_INSTANCES) result = _interface()._handle_edit_schematic_component( { "schematicPath": str(sch), "reference": "R1", "newReference": "R5", } ) assert result["success"] is True, result content = sch.read_text(encoding="utf-8") # The visible property is updated (existing behaviour). assert '(property "Reference" "R5"' in content assert '(property "Reference" "R1"' not in content # The fix: the (instances) leaf is updated too. The PCB sync / # netlist export path reads this entry. assert '(reference "R5")' in content assert '(reference "R1")' not in content def test_hierarchical_multiple_paths_all_updated(self, tmp_path: Any) -> None: """A symbol reused across hierarchical sub-sheets has one (path) per instance, each with its own (reference "X"). All must be updated atomically — leaving any stale produces the same mismatch bug on only-some-of-the-instances.""" sch = _write_schematic(tmp_path, PLACED_RESISTOR_HIERARCHICAL) result = _interface()._handle_edit_schematic_component( { "schematicPath": str(sch), "reference": "R1", "newReference": "R5", } ) assert result["success"] is True, result content = sch.read_text(encoding="utf-8") # Both (path) entries inside (instances) must now show R5. assert content.count('(reference "R5")') == 2 assert '(reference "R1")' not in content def test_no_instances_block_does_not_crash(self, tmp_path: Any) -> None: """Symbols saved by older KiCad versions (or partial schematics constructed by external tools) may not have an (instances) block. The rename must still succeed — the instances walk is a no-op.""" sch = _write_schematic(tmp_path, PLACED_RESISTOR_NO_INSTANCES) result = _interface()._handle_edit_schematic_component( { "schematicPath": str(sch), "reference": "R1", "newReference": "R5", } ) assert result["success"] is True, result content = sch.read_text(encoding="utf-8") assert '(property "Reference" "R5"' in content # No (instances) entry → no (reference …) leaves to update; the # property update alone is sufficient. assert "(instances" not in content def test_property_reference_not_clobbered_by_instances_pass(self, tmp_path: Any) -> None: """The instances-update regex must match (reference "X") specifically, not (property "Reference" "X"). If the regex were too loose it would double-substitute or, worse, leave the property field untouched after a separate over-eager edit. Pin the shape.""" sch = _write_schematic(tmp_path, PLACED_RESISTOR_WITH_INSTANCES) _interface()._handle_edit_schematic_component( { "schematicPath": str(sch), "reference": "R1", "newReference": "R42", } ) content = sch.read_text(encoding="utf-8") # property line uses (property "Reference" "X") — visible-in-eeschema # form, with a leading "(property" not "(reference". assert '(property "Reference" "R42"' in content # instances leaf uses (reference "X") form. assert '(reference "R42")' in content def test_other_edits_still_work_when_only_newReference_provided( self, tmp_path: Any ) -> None: """Verify the rename leaves other field values intact.""" sch = _write_schematic(tmp_path, PLACED_RESISTOR_WITH_INSTANCES) _interface()._handle_edit_schematic_component( { "schematicPath": str(sch), "reference": "R1", "newReference": "R5", } ) content = sch.read_text(encoding="utf-8") assert '(property "Value" "10k"' in content assert '(property "Footprint" "Resistor_SMD:R_0603_1608Metric"' in content def test_response_payload_reports_reference_change(self, tmp_path: Any) -> None: """The response payload's `updated.reference` should reflect the new reference, preserving the existing handler contract.""" sch = _write_schematic(tmp_path, PLACED_RESISTOR_WITH_INSTANCES) result = _interface()._handle_edit_schematic_component( { "schematicPath": str(sch), "reference": "R1", "newReference": "R5", } ) assert result["updated"]["reference"] == "R5"