diff --git a/doc/changelog.d/2217.fixed.md b/doc/changelog.d/2217.fixed.md new file mode 100644 index 0000000000..fa98127277 --- /dev/null +++ b/doc/changelog.d/2217.fixed.md @@ -0,0 +1 @@ +Issue #2213 fixed diff --git a/src/pyedb/configuration/cfg_components.py b/src/pyedb/configuration/cfg_components.py index 3179b4903c..757cb87870 100644 --- a/src/pyedb/configuration/cfg_components.py +++ b/src/pyedb/configuration/cfg_components.py @@ -302,8 +302,7 @@ def _set_solder_ball_properties_to_edb(self): if self._pedb.grpc: if not shape: raise ValueError("Solderball shape must be either cylinder or spheroid") - cp = self.pyedb_obj.component_property - sbp = cp.solder_ball_property + sbp = self.pyedb_obj.component_property.solder_ball_property shape_lower = shape.lower() if shape_lower == "cylinder": sbp.set_diameter(self._pedb.value(diameter)) @@ -336,12 +335,8 @@ def _retrieve_ic_die_properties_from_edb(self): die_props = self.pyedb_obj.ic_die_properties die_type = die_props.die_type temp = {"type": "no_die" if die_type in _NO_DIE_TYPES else die_type} - if die_type in _NO_DIE_TYPES: - if self._pedb and self._pedb.grpc: - orientation = die_props.die_orientation - temp["orientation"] = orientation if orientation is not None else "chip_up" - else: - temp["orientation"] = die_props.die_orientation + temp["orientation"] = die_props.die_orientation + if die_type not in _NO_DIE_TYPES: if die_type == "wire_bond": temp["height"] = str(die_props.height) self.ic_die_properties = temp diff --git a/src/pyedb/configuration/cfg_s_parameter_models.py b/src/pyedb/configuration/cfg_s_parameter_models.py index a5a78cd112..777ee048f0 100644 --- a/src/pyedb/configuration/cfg_s_parameter_models.py +++ b/src/pyedb/configuration/cfg_s_parameter_models.py @@ -54,7 +54,11 @@ def apply(self): for s_param in self.models: fpath = s_param.file_path if not Path(fpath).anchor: - fpath = str(Path(self.path_libraries) / fpath) + if self.path_libraries: + base = Path(self.path_libraries) + else: + base = Path(self._pedb.edbpath).parent + fpath = str(base / fpath) comp_def = self._pedb.definitions.component[s_param.component_definition] if s_param.pin_order: comp_def.set_properties(pin_order=s_param.pin_order) diff --git a/src/pyedb/configuration/cfg_spice_models.py b/src/pyedb/configuration/cfg_spice_models.py index 81e64111e7..99ecb3013f 100644 --- a/src/pyedb/configuration/cfg_spice_models.py +++ b/src/pyedb/configuration/cfg_spice_models.py @@ -78,7 +78,11 @@ def apply(self): if self._pedb is None: return self.model_dump(exclude_none=True) if not Path(self.file_path).anchor: - fpath = str(Path(self._path_libraries or "") / self.file_path) + if self._path_libraries: + base = Path(self._path_libraries) + else: + base = Path(self._pedb.edbpath).parent + fpath = str(base / self.file_path) else: fpath = self.file_path diff --git a/src/pyedb/grpc/database/hierarchy/component.py b/src/pyedb/grpc/database/hierarchy/component.py index ad45804403..c73720049c 100644 --- a/src/pyedb/grpc/database/hierarchy/component.py +++ b/src/pyedb/grpc/database/hierarchy/component.py @@ -1389,7 +1389,7 @@ def assign_spice_model( pname, pnumber = pair if pname not in pin_names_sp: # pragma: no cover raise ValueError(f"Pin name {pname} doesn't exist in {file_path}.") - model.core.add_terminal(str(pnumber), pname) + model.core.add_terminal(pname, str(pnumber)) else: for idx, pname in enumerate(pin_names_sp): model.core.add_terminal(pname, str(idx + 1)) diff --git a/tests/system/test_edb_configuration.py b/tests/system/test_edb_configuration.py index bb2467c591..9399570f9b 100644 --- a/tests/system/test_edb_configuration.py +++ b/tests/system/test_edb_configuration.py @@ -1911,11 +1911,7 @@ def test_17_ic_die_properties(self): comps_edb = db.configuration.get_data_from_db(components=True)["components"] component = [i for i in comps_edb if i["reference_designator"] == "U8"][0] assert component["ic_die_properties"]["type"] in ["none", "no_die", "flip_chip", "flipchip"] - if db.grpc: - # grpc does have this property not DotNet. - assert "orientation" in component["ic_die_properties"] - else: - assert "orientation" not in component["ic_die_properties"] + assert "orientation" in component["ic_die_properties"] assert "height" not in component["ic_die_properties"] db.configuration.load(U8_IC_DIE_PROPERTIES, apply_file=True) comps_edb = db.configuration.get_data_from_db(components=True)["components"] diff --git a/tests/system/test_edb_stackup.py b/tests/system/test_edb_stackup.py index fa2b5ce35a..d93365b2aa 100644 --- a/tests/system/test_edb_stackup.py +++ b/tests/system/test_edb_stackup.py @@ -25,6 +25,7 @@ import math import os import platform +import time import pytest @@ -36,6 +37,43 @@ @pytest.mark.usefixtures("close_rpc_session") class TestClass(BaseTestClass): + @staticmethod + def _wait_until_ready(path, attempts=20, delay=0.25): + """Wait until a file path exists and is non-empty.""" + for _ in range(attempts): + if os.path.exists(path) and os.path.getsize(path) > 0: + return True + time.sleep(delay) + return os.path.exists(path) and os.path.getsize(path) > 0 + + def _load_stackup_with_retries(self, source_path, stackup_path, **kwargs): + """Open an EDB and import stackup with bounded retries for CI filesystem/server races.""" + last_error = None + for _ in range(3): + edbapp = None + try: + edbapp = self.edb_examples.load_edb(source_path) + assert edbapp.stackup.load(stackup_path, **kwargs) + return + except Exception as e: + last_error = e + time.sleep(0.5) + finally: + if edbapp: + edbapp.close(terminate_rpc_session=False) + raise AssertionError(f"Failed to load stackup after retries: {last_error}") + + def _get_si_verse_with_retries(self): + """Open the SI-Verse example with retries to reduce transient CI failures.""" + last_error = None + for _ in range(3): + try: + return self.edb_examples.get_si_verse() + except Exception as e: + last_error = e + time.sleep(0.5) + raise AssertionError(f"Failed to open SI-Verse example after retries: {last_error}") + def test_stackup_get_signal_layers(self): """Report residual copper area per layer.""" edbapp = self.edb_examples.get_si_verse() @@ -287,7 +325,6 @@ def test_stackup_properties_0(self): assert edbapp.stackup.add_layer("new_bottom", "1_Top", "add_at_elevation", "dielectric", elevation=0.0003) edbapp.close(terminate_rpc_session=False) - @pytest.mark.skipif(config["use_grpc"] and platform.system() == "Linux", reason="random fails on Linux.") def test_stackup_properties_1(self): """Evaluate various stackup properties.""" edbapp = self.edb_examples.create_empty_edb() @@ -295,10 +332,14 @@ def test_stackup_properties_1(self): export_method = edbapp.stackup.export csv_path = self.edb_examples.copy_test_files_into_local_folder("TEDB/ansys_pcb_stackup.csv")[0] - # Verify the file is fully written before importing (guards against - # Linux filesystem flush races that caused intermittent failures). - assert os.path.exists(csv_path) and os.path.getsize(csv_path) > 0, f"CSV file not ready: {csv_path}" - assert import_method(csv_path) + assert self._wait_until_ready(csv_path), f"CSV file not ready: {csv_path}" + import_ok = False + for _ in range(3): + import_ok = import_method(csv_path) + if import_ok: + break + time.sleep(0.5) + assert import_ok assert "18_Bottom" in edbapp.stackup.layers.keys() assert edbapp.stackup.add_layer("19_Bottom", None, "add_on_top", material="iron") export_stackup_path = os.path.join(self.edb_examples.test_folder, "export_stackup.csv") @@ -307,7 +348,6 @@ def test_stackup_properties_1(self): edbapp.close(terminate_rpc_session=False) - @pytest.mark.skipif(config["use_grpc"] and platform.system() == "Linux", reason="random fails on Linux.") def test_stackup_properties_2(self): """Evaluate various stackup properties (JSON export round-trip).""" edbapp = self.edb_examples.create_empty_edb() @@ -315,8 +355,14 @@ def test_stackup_properties_2(self): export_method = edbapp.stackup.export csv_path = self.edb_examples.copy_test_files_into_local_folder("TEDB/ansys_pcb_stackup.csv")[0] - assert os.path.exists(csv_path) and os.path.getsize(csv_path) > 0, f"CSV file not ready: {csv_path}" - assert import_method(csv_path) + assert self._wait_until_ready(csv_path), f"CSV file not ready: {csv_path}" + import_ok = False + for _ in range(3): + import_ok = import_method(csv_path) + if import_ok: + break + time.sleep(0.5) + assert import_ok assert "18_Bottom" in edbapp.stackup.layers.keys() assert edbapp.stackup.add_layer("19_Bottom", None, "add_on_top", material="iron") export_stackup_path = os.path.join(self.edb_examples.test_folder, "export_stackup_2.csv") @@ -329,13 +375,10 @@ def test_stackup_load_json(self): source_path, fpath = self.edb_examples.copy_test_files_into_local_folder( ["TEDB/ANSYS-HSD_V1.aedb", "TEDB/stackup.json"] ) - # On Linux, shutil.copytree may return before the OS has fully flushed - # all inodes; verify both paths are accessible before opening. - assert os.path.exists(source_path), f"EDB source not ready: {source_path}" - assert os.path.exists(fpath) and os.path.getsize(fpath) > 0, f"JSON not ready: {fpath}" - edbapp = self.edb_examples.load_edb(source_path) - assert edbapp.stackup.load(fpath) - edbapp.close(terminate_rpc_session=False) + edb_def = os.path.join(source_path, "edb.def") + assert self._wait_until_ready(edb_def), f"EDB source not ready: {edb_def}" + assert self._wait_until_ready(fpath), f"JSON not ready: {fpath}" + self._load_stackup_with_retries(source_path, fpath) def test_stackup_export_json(self): """Export stackup into a JSON file.""" @@ -406,6 +449,7 @@ def test_stackup_load_xml(self): edbapp = self.edb_examples.get_si_verse() except Exception as e: pytest.skip(f"Skipping test due to file access failure (possible CI instability): {e}") + assert self._wait_until_ready(file_path), f"XML file not ready: {file_path}" assert edbapp.stackup.load(file_path) assert "Inner1" in list(edbapp.stackup.layers.keys()) # Renamed layer assert "DE1" not in edbapp.stackup.layers.keys() # Removed layer @@ -419,13 +463,27 @@ def test_stackup_load_layer_renamed(self): source_path, fpath = self.edb_examples.copy_test_files_into_local_folder( ["TEDB/ANSYS-HSD_V1.aedb", "TEDB/stackup_renamed.json"] ) - - edbapp = self.edb_examples.load_edb(source_path) - edbapp.stackup.load(fpath, rename=True) - assert "1_Top_renamed" in edbapp.stackup.layers - assert "DE1_renamed" in edbapp.stackup.layers - assert "16_Bottom_renamed" in edbapp.stackup.layers - edbapp.close(terminate_rpc_session=False) + edb_def = os.path.join(source_path, "edb.def") + assert self._wait_until_ready(edb_def), f"EDB source not ready: {edb_def}" + assert self._wait_until_ready(fpath), f"JSON not ready: {fpath}" + + last_error = None + for _ in range(3): + edbapp = None + try: + edbapp = self.edb_examples.load_edb(source_path) + assert edbapp.stackup.load(fpath, rename=True) + assert "1_Top_renamed" in edbapp.stackup.layers + assert "DE1_renamed" in edbapp.stackup.layers + assert "16_Bottom_renamed" in edbapp.stackup.layers + return + except Exception as e: + last_error = e + time.sleep(0.5) + finally: + if edbapp: + edbapp.close(terminate_rpc_session=False) + raise AssertionError(f"Failed to load renamed stackup after retries: {last_error}") def test_stackup_place_in_3d_with_flipped_stackup(self): """Place into another cell using 3d placement method with and @@ -1369,14 +1427,21 @@ def test_stackup_limits_only_metals(self): edbapp.close(terminate_rpc_session=False) @pytest.mark.skipif(not config["use_grpc"], reason="gRPC only.") - @pytest.mark.skipif(platform.system() == "Linux", reason="random fails on Linux.") def test_stackup_export_csv(self): """Export stackup to CSV format.""" - edbapp = self.edb_examples.get_si_verse() + edbapp = self._get_si_verse_with_retries() csv_path = os.path.join(self.edb_examples.test_folder, "test_export_stackup.csv") - assert edbapp.stackup.export(csv_path) - assert os.path.exists(csv_path) - edbapp.close(terminate_rpc_session=False) + try: + export_ok = False + for _ in range(3): + export_ok = edbapp.stackup.export(csv_path) + if export_ok and self._wait_until_ready(csv_path): + break + time.sleep(0.5) + assert export_ok + assert self._wait_until_ready(csv_path), f"CSV export not ready: {csv_path}" + finally: + edbapp.close(terminate_rpc_session=False) @pytest.mark.skipif(not config["use_grpc"], reason="gRPC only.") def test_stackup_export_unsupported_format(self): @@ -1473,20 +1538,28 @@ def test_stackup_adjust_solder_dielectrics(self): edbapp.close(terminate_rpc_session=False) @pytest.mark.skipif(not config["use_grpc"], reason="gRPC only.") - @pytest.mark.skipif(platform.system() == "Linux", reason="random fails on Linux.") def test_stackup_export_json_with_material_in_layer(self): """Export stackup JSON with include_material_with_layer=True.""" import json - edbapp = self.edb_examples.get_si_verse() + edbapp = self._get_si_verse_with_retries() json_path = os.path.join(self.edb_examples.test_folder, "stackup_with_material.json") - assert edbapp.stackup.export(json_path, include_material_with_layer=True) - with open(json_path, "r") as f: - data = json.load(f) - # When include_material_with_layer=True, there should be no top-level 'materials' key. - assert "materials" not in data - assert "layers" in data - edbapp.close(terminate_rpc_session=False) + try: + export_ok = False + for _ in range(3): + export_ok = edbapp.stackup.export(json_path, include_material_with_layer=True) + if export_ok and self._wait_until_ready(json_path): + break + time.sleep(0.5) + assert export_ok + assert self._wait_until_ready(json_path), f"JSON export not ready: {json_path}" + with open(json_path, "r") as f: + data = json.load(f) + # When include_material_with_layer=True, there should be no top-level 'materials' key. + assert "materials" not in data + assert "layers" in data + finally: + edbapp.close(terminate_rpc_session=False) @pytest.mark.skipif(not config["use_grpc"], reason="gRPC only.") def test_stackup_place_in_layout(self): @@ -1510,10 +1583,16 @@ def test_stackup_place_in_layout(self): edb1.close(terminate_rpc_session=False) @pytest.mark.skipif(not config["use_grpc"], reason="gRPC only.") - @pytest.mark.skipif(platform.system() == "Linux", reason="random fails on Linux.") def test_stackup_export_json_no_output_file(self): """_export_layer_stackup_to_json returns False when output_file is None.""" - edbapp = self.edb_examples.get_si_verse() - result = edbapp.stackup._export_layer_stackup_to_json(output_file=None) - assert result is False - edbapp.close(terminate_rpc_session=False) + edbapp = self._get_si_verse_with_retries() + try: + last_result = None + for _ in range(3): + last_result = edbapp.stackup._export_layer_stackup_to_json(output_file=None) + if last_result is False: + break + time.sleep(0.5) + assert last_result is False + finally: + edbapp.close(terminate_rpc_session=False) diff --git a/tests/unit/test_components.py b/tests/unit/test_components.py index 3bd9fc00b8..1b60b9ae7e 100644 --- a/tests/unit/test_components.py +++ b/tests/unit/test_components.py @@ -690,13 +690,15 @@ def test_wire_bond_die_retrieves_height(self): assert c.ic_die_properties["type"] == "wire_bond" assert "height" in c.ic_die_properties - def test_no_die_type_skips_orientation_and_height(self): + def test_no_die_type_skips_height_but_includes_orientation(self): pedb = _make_pedb(grpc=True) obj = self._make_ic_obj() obj.ic_die_properties.die_type = "no_die" c = CfgComponent(pedb, obj, reference_designator="U1", part_type="ic") c.retrieve_parameters_from_edb() assert c.ic_die_properties["type"] == "no_die" + # gRPC always exposes orientation; height is only for wire_bond + assert "orientation" in c.ic_die_properties assert "height" not in c.ic_die_properties @@ -2908,6 +2910,15 @@ def test_unknown_comp_type_string_upcased(self): finally: os.unlink(fname) + def test_apply_calls_set_parameters_for_each(self): + cc = CfgComponents(None, None) + mock1 = MagicMock(spec=CfgComponent) + mock2 = MagicMock(spec=CfgComponent) + cc.components = [mock1, mock2] + cc.apply() + mock1.set_parameters_to_edb.assert_called_once() + mock2.set_parameters_to_edb.assert_called_once() + @_grpc_only class TestShortComponentPins: @@ -3009,3 +3020,136 @@ def test_short_component_pins_fallback_layer(self): result = comps.short_component_pins("R1", width=0.1e-3) assert result is True + + +# --------------------------------------------------------------------------- +# Component.assign_spice_model – terminal_pairs argument order (regression) +# --------------------------------------------------------------------------- + + +def _spice_file_content(node_names): + """Return minimal SPICE file text with the given subckt node names.""" + return f".subckt MyModel {' '.join(node_names)}\n.ends\n" + + +def _make_comp_for_spice(pin_layout_names, pedb=None): + """Build a minimal mock Component with the given pin names.""" + comp = MagicMock() + comp.name = "COMP1" + comp.pins = {p: MagicMock() for p in pin_layout_names} + comp._pedb = pedb or MagicMock() + comp._pedb.logger = MagicMock() + return comp + + +class TestAssignSpiceModelTerminalPairs: + """Regression tests for terminal_pairs argument order in Component.assign_spice_model. + + Bug: add_terminal was called with swapped arguments when terminal_pairs were explicitly + provided, causing pin mapping to be ignored in EDB. + Expected call: add_terminal(spice_pin_name, component_pin) + Buggy call was: add_terminal(component_pin, spice_pin_name) + """ + + def _run(self, tmp_path, pin_layout_names, spice_nodes, terminal_pairs, sub_circuit_name=None): + """Create a fake SPICE file, mock internals, call assign_spice_model, + and return (add_terminal call list, result).""" + from unittest.mock import patch + + from pyedb.grpc.database.hierarchy.component import Component + + sp_file = tmp_path / "model.sp" + sp_file.write_text(_spice_file_content(spice_nodes)) + + comp = _make_comp_for_spice(pin_layout_names) + + mock_spice_model = MagicMock() + mock_spice_model.core.is_null = False + + with ( + patch("pyedb.grpc.database.hierarchy.component.SpiceModel", return_value=mock_spice_model), + patch.object(Component, "_set_model", return_value=None), + ): + result = Component.assign_spice_model( + comp, + str(sp_file), + name="MyModel", + sub_circuit_name=sub_circuit_name or "", + terminal_pairs=terminal_pairs, + ) + + return mock_spice_model.core.add_terminal.call_args_list, result + + def test_terminal_pairs_spice_node_is_first_arg(self, tmp_path): + """add_terminal must be called with (spice_node, layout_pin) — not reversed.""" + from unittest.mock import call + + terminal_pairs = [["n1", "A1"], ["n2", "B2"]] + calls, result = self._run( + tmp_path, + pin_layout_names=["A1", "B2"], + spice_nodes=["n1", "n2"], + terminal_pairs=terminal_pairs, + ) + + assert len(calls) == 2 + assert calls[0] == call("n1", "A1"), f"Expected call('n1', 'A1') but got {calls[0]}" + assert calls[1] == call("n2", "B2"), f"Expected call('n2', 'B2') but got {calls[1]}" + + def test_terminal_pairs_with_integer_pin_number(self, tmp_path): + """Layout pin numbers given as integers must be converted to str.""" + from unittest.mock import call + + terminal_pairs = [["n1", 2], ["n2", 1]] + calls, _ = self._run( + tmp_path, + pin_layout_names=["1", "2"], + spice_nodes=["n1", "n2"], + terminal_pairs=terminal_pairs, + ) + + assert calls[0] == call("n1", "2") + assert calls[1] == call("n2", "1") + + def test_terminal_pairs_four_nodes(self, tmp_path): + """Verify the full 4-pair scenario from edb_configuration_SPICE.json.""" + from unittest.mock import call + + terminal_pairs = [["n1", "A1"], ["n2", "B2"], ["n3", "A2"], ["n4", "B1"]] + calls, _ = self._run( + tmp_path, + pin_layout_names=["A1", "B2", "A2", "B1"], + spice_nodes=["n1", "n2", "n3", "n4"], + terminal_pairs=terminal_pairs, + ) + + expected = [call("n1", "A1"), call("n2", "B2"), call("n3", "A2"), call("n4", "B1")] + assert calls == expected + + def test_no_terminal_pairs_auto_index(self, tmp_path): + """When no terminal_pairs given, add_terminal uses (spice_node, str(idx+1)).""" + from unittest.mock import call + + calls, _ = self._run( + tmp_path, + pin_layout_names=["1", "2"], + spice_nodes=["n1", "n2"], + terminal_pairs=None, + ) + + assert calls[0] == call("n1", "1") + assert calls[1] == call("n2", "2") + + def test_single_pair_not_nested_list(self, tmp_path): + """A single pair passed as a flat list [pname, pnumber] must still work.""" + from unittest.mock import call + + terminal_pairs = ["n1", "A1"] + calls, _ = self._run( + tmp_path, + pin_layout_names=["A1"], + spice_nodes=["n1"], + terminal_pairs=terminal_pairs, + ) + + assert calls[0] == call("n1", "A1")