From 38391175b45887206d92f48e27a6648d424d8841 Mon Sep 17 00:00:00 2001 From: rob Date: Sat, 30 May 2026 15:18:01 -0300 Subject: [PATCH] Fix drag-drop crash + cover it with offscreen GUI tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _drop_piece looked up plan.item(item.pid), but item.pid is a Placement id (pl2) while CutPlan.item() expects a CutItem id (ci2) — every drop raised StopIteration before validate/revert could run. Use the already-found placement's item_id (plan.item(p.item_id)) for the stock-compat check and message. Added tests/test_bom_window.py (offscreen QGraphicsScene): drop-overlap reverts without crashing, drop-onto-incompatible-stock reverts, and a valid move commits. 128 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/woodshop/gui/bom_window.py | 5 +-- tests/test_bom_window.py | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 tests/test_bom_window.py diff --git a/src/woodshop/gui/bom_window.py b/src/woodshop/gui/bom_window.py index 7982e16..3c9575a 100644 --- a/src/woodshop/gui/bom_window.py +++ b/src/woodshop/gui/bom_window.py @@ -343,9 +343,10 @@ class BomWindow(QDialog): if target is None: target = sp_cur # Stock-type compatibility: a 2x4 can't go on a plywood sheet, etc. - if plan.item(item.pid).stock != target.stock: + item_stock = plan.item(p.item_id).stock + if item_stock != target.stock: self._revert(plan, item.pid, home) - self._status.setText(f"✗ {plan.item(item.pid).stock} can't go on {target.stock} — reverted") + self._status.setText(f"✗ {item_stock} can't go on {target.stock} — reverted") recompute(plan); self._refresh_all() return row_y0 = self._row_of(target.id) diff --git a/tests/test_bom_window.py b/tests/test_bom_window.py new file mode 100644 index 0000000..b0812e6 --- /dev/null +++ b/tests/test_bom_window.py @@ -0,0 +1,59 @@ +"""Offscreen tests for the BOM window's drag/drop path (no display needed — +QGraphicsScene is pure Qt). Guards the placement-id vs cut-item-id crash.""" +import os + +import pytest + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") +pytest.importorskip("PySide6") + +from PySide6.QtWidgets import QApplication # noqa: E402 + +from woodshop.cutplan import find_placement # noqa: E402 +from woodshop.gui.bom_window import BomWindow, _Piece # noqa: E402 +from woodshop.gui.controller import Controller # noqa: E402 + +_app = QApplication.instance() or QApplication([]) + + +def _pieces(w): + return sorted((it for it in w.scene.items() if isinstance(it, _Piece)), + key=lambda it: it.pos().x()) + + +def test_drop_overlap_reverts_without_crashing(tmp_path): + c = Controller(str(tmp_path / "s.json")) + c.place("2x4", 30) + c.place("2x4", 30) # one stick, two pieces + w = BomWindow(c) + first, second = _pieces(w)[:2] + home = (second.sp_id, second.pos().x(), second.pos().y()) + second.setPos(0, second.pos().y()) # drop on top of the first -> overlap + w._drop_piece(second, home) # must not raise (was StopIteration) + assert "revert" in w._status.text().lower() + + +def test_drop_onto_incompatible_stock_reverts(tmp_path): + c = Controller(str(tmp_path / "s.json")) + c.place("2x4", 24) + c.place("ply-3/4", 24, width_in=24) + w = BomWindow(c) + lumber = next(it for it in _pieces(w) + if not find_placement(w._plan, it.pid)[0].is_sheet) + sheet_y = next(y0 for y0, _y1, sp in w._rows if sp.is_sheet) + home = (lumber.sp_id, lumber.pos().x(), lumber.pos().y()) + lumber.setPos(10, sheet_y + 5) # drag the 2x4 onto the plywood sheet + w._drop_piece(lumber, home) # must not raise + assert "can't go" in w._status.text() + + +def test_valid_move_commits(tmp_path): + c = Controller(str(tmp_path / "s.json")) + c.place("2x4", 20) + c.place("2x4", 20) + w = BomWindow(c) + second = _pieces(w)[1] + home = (second.sp_id, second.pos().x(), second.pos().y()) + second.setPos(50 * w._px, second.pos().y()) # slide it right, still clear + w._drop_piece(second, home) + assert "placed" in w._status.text().lower()