From 7f349c6d6315601b3257ded6c6ddf1b0dcfe7fb1 Mon Sep 17 00:00:00 2001 From: Himanshu Gohel <1551217+hgohel@users.noreply.github.com> Date: Fri, 17 Apr 2026 13:28:55 -0400 Subject: [PATCH 01/21] Wrap label vaue to ensure it is a string The argument `label` to Gtk.Label must be of type string. In recent versions of Python and/or PyGObject, this check has become stricter and as a result is throwing an exception instead of silently converting to a string. So now we explicitly convert to string. Fixes #14181 --- TimelinePedigreeView/TimelinePedigreeView.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TimelinePedigreeView/TimelinePedigreeView.py b/TimelinePedigreeView/TimelinePedigreeView.py index d490f0242..a35b27371 100644 --- a/TimelinePedigreeView/TimelinePedigreeView.py +++ b/TimelinePedigreeView/TimelinePedigreeView.py @@ -705,7 +705,7 @@ def Tree_Rebuild(self): if Tick[1] > 0 and Tick[1] < RequiredWidth: self.gtklayout_lines.append([Tick[1], int(5*TimeLineHeight/8), Tick[1], int(7*TimeLineHeight/8), 1]) if Tick[0]: - label = Gtk.Label(label=Tick[0]) + label = Gtk.Label(label=str(Tick[0])) label.set_justify(Gtk.Justification.CENTER) label.show() layout_widget.put(label, int(Tick[1]-label.get_preferred_size()[0].width/2), 1*TimeLineHeight/4) From 9700390ecbaecc09837c26d3af2c04f4e0d4b957 Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 01:28:34 +0200 Subject: [PATCH 02/21] TimelinePedigreeView: fix crash on second right-click in context menu (bug 0012387) The settings submenu builder appended the same SeparatorMenuItem twice, which GTK rejected with "Can't set a parent on widget which has a parent". The corrupt parent linkage caused a segfault when the previous menu was garbage-collected on the next right-click. Removing the stray duplicate append fixes all four context-menu paths (background canvas, person, relation, missing parent) and silences the related GTK_IS_WIDGET assertion warnings. Also resolves duplicate report 0013463. --- TimelinePedigreeView/TimelinePedigreeView.py | 1 - 1 file changed, 1 deletion(-) diff --git a/TimelinePedigreeView/TimelinePedigreeView.py b/TimelinePedigreeView/TimelinePedigreeView.py index a35b27371..1926987cf 100644 --- a/TimelinePedigreeView/TimelinePedigreeView.py +++ b/TimelinePedigreeView/TimelinePedigreeView.py @@ -1501,7 +1501,6 @@ def add_settings_to_menu(self, menu): menu.append(item) # Help menu entry - menu.append(item) item = Gtk.MenuItem(label=_("About Timeline Pedigree View")) item.connect("activate", self.on_help_clicked) item.show() From cbbeff18d08bfe7ba92e9dd9921cc5e84788fc2e Mon Sep 17 00:00:00 2001 From: GaryGriffin Date: Tue, 21 Apr 2026 09:20:40 -0700 Subject: [PATCH 03/21] Merge TimelinePedigree PR 819, 823 --- TimelinePedigreeView/TimelinePedigreeView.gpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TimelinePedigreeView/TimelinePedigreeView.gpr.py b/TimelinePedigreeView/TimelinePedigreeView.gpr.py index 2d6237670..11bdc514b 100644 --- a/TimelinePedigreeView/TimelinePedigreeView.gpr.py +++ b/TimelinePedigreeView/TimelinePedigreeView.gpr.py @@ -36,7 +36,7 @@ "The view shows a timeline pedigree with ancestors and " "descendants of the selected person" ), - version = '0.1.66', + version = '0.1.67', gramps_target_version="6.0", status=STABLE, fname="TimelinePedigreeView.py", From afb90542e582a62515b30fd889da0468e7ad4961 Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 01:40:53 +0200 Subject: [PATCH 04/21] DataEntryGramplet: fix crash when adding person with no Family Tree open (bug 0012691) Clicking Add (or Save after a dirty edit) when no tree was loaded raised AttributeError: 'DummyDb' object has no attribute 'get_undodb' from DbTxn. Guard both mutating callbacks on dbstate.is_open() and surface a clear ErrorDialog instead. Add unit tests for the closed-db guards, pre-existing input guards, and .gpr.py registration metadata so future refactors can't silently break the bug-12691 fix. Co-Authored-By: Claude Opus 4.7 --- DataEntryGramplet/DataEntryGramplet.py | 9 + .../tests/test_data_entry_gramplet.py | 223 ++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 DataEntryGramplet/tests/test_data_entry_gramplet.py diff --git a/DataEntryGramplet/DataEntryGramplet.py b/DataEntryGramplet/DataEntryGramplet.py index e00a4e771..bb3964055 100644 --- a/DataEntryGramplet/DataEntryGramplet.py +++ b/DataEntryGramplet/DataEntryGramplet.py @@ -423,6 +423,11 @@ def make_person(self, firstname, surname, gender): def save_data_edit(self, obj): if self._dirty: + if not self.dbstate.is_open(): + from gramps.gui.dialog import ErrorDialog + ErrorDialog(_("No Family Tree is open."), + _("Please open a Family Tree to edit data.")) + return # Save the edits ---------------------------------- person = self._dirty_person # First, get the data: @@ -498,6 +503,10 @@ def add_source(self, obj, source): def add_data_entry(self, obj): from gramps.gui.dialog import ErrorDialog + if not self.dbstate.is_open(): + ErrorDialog(_("No Family Tree is open."), + _("Please open a Family Tree before adding a person.")) + return # First, get the data: if "," in self.de_widgets["NPName"].get_text(): surname, firstname = self.de_widgets["NPName"].get_text().split(",", 1) diff --git a/DataEntryGramplet/tests/test_data_entry_gramplet.py b/DataEntryGramplet/tests/test_data_entry_gramplet.py new file mode 100644 index 000000000..e012df87e --- /dev/null +++ b/DataEntryGramplet/tests/test_data_entry_gramplet.py @@ -0,0 +1,223 @@ +# +# Gramps - a GTK+/GNOME based genealogy program +# +# Copyright (C) 2026 Eduard Ralph +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# + +""" +Tests for the Data Entry Gramplet — covers +``gramps-project/gramps#12691`` (``AttributeError: 'DummyDb' object +has no attribute 'get_undodb'`` when the user presses *Add* or *Save* +without a Family Tree loaded). + +The Gramplet subclass requires a live Gramps GUI to instantiate, so +these tests build a minimal stub via ``__new__`` and invoke the +mutating callbacks directly. That keeps the tests fast and avoids +spinning up GTK, while still exercising the real guard code paths. +""" + +# ------------------------ +# Python modules +# ------------------------ +import os +import sys + +import pytest + +# Gramps is imported by the module under test. +pytest.importorskip("gramps") + + +# --------------------------------------------------------------------------- +# Make the addon importable and ensure GRAMPS_RESOURCES is set +# --------------------------------------------------------------------------- +ADDON_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if ADDON_DIR not in sys.path: + sys.path.insert(0, ADDON_DIR) + +if "GRAMPS_RESOURCES" not in os.environ: + import gramps + + os.environ["GRAMPS_RESOURCES"] = os.path.dirname( + os.path.dirname(gramps.__file__) + ) + + +# ------------------------ +# Module under test +# ------------------------ +import gramps.gui.dialog as gramps_dialog # noqa: E402 + +from DataEntryGramplet import DataEntryGramplet # noqa: E402 + + +# --------------------------------------------------------------------------- +# Lightweight stubs — enough surface for the guard paths under test +# --------------------------------------------------------------------------- +class _FakeDbState: + def __init__(self, is_open=True): + self._is_open = is_open + + def is_open(self): + return self._is_open + + +class _FakeEntry: + def __init__(self, text=""): + self._text = text + + def get_text(self): + return self._text + + +class _FakeCombo: + def __init__(self, active=0): + self._active = active + + def get_active(self): + return self._active + + +def _make_gramplet( + *, + db_open=True, + np_name="", + np_gender=2, # UNKNOWN + np_relation=DataEntryGramplet.NO_REL, + dirty=False, +): + """Return an instance skipping ``__init__`` so no GTK is required.""" + stub = DataEntryGramplet.__new__(DataEntryGramplet) + stub.dbstate = _FakeDbState(db_open) + stub._dirty = dirty + stub._dirty_person = None + stub.de_widgets = { + "NPName": _FakeEntry(np_name), + "NPGender": _FakeCombo(np_gender), + "NPRelation": _FakeCombo(np_relation), + } + # save_data_edit falls through to self.update() even on the guarded path. + stub.update = lambda: None + stub.get_active_object = lambda _type: None + return stub + + +@pytest.fixture +def captured_errors(monkeypatch): + """Replace gramps.gui.dialog.ErrorDialog so tests can inspect calls.""" + shown: list[tuple[str, str]] = [] + + def _fake(title, body="", *args, **kwargs): + shown.append((str(title), str(body))) + + monkeypatch.setattr(gramps_dialog, "ErrorDialog", _fake) + return shown + + +# --------------------------------------------------------------------------- +# Bug 12691 — closed Family Tree must not crash +# --------------------------------------------------------------------------- +def test_add_data_entry_with_closed_db_shows_error(captured_errors): + """Pressing *Add* with no tree open surfaces an ErrorDialog, not a crash.""" + stub = _make_gramplet(db_open=False, np_name="Doe, Jane") + + stub.add_data_entry(None) + + assert captured_errors, "ErrorDialog was not displayed" + title, body = captured_errors[0] + assert "Family Tree" in title + assert "open" in body.lower() + + +def test_save_data_edit_with_closed_db_shows_error(captured_errors): + """Pressing *Save* while dirty with no tree open must not invoke DbTxn.""" + stub = _make_gramplet(db_open=False, dirty=True) + + stub.save_data_edit(None) + + assert captured_errors, "ErrorDialog was not displayed" + title, _body = captured_errors[0] + assert "Family Tree" in title + + +def test_save_data_edit_noop_when_not_dirty(captured_errors): + """A *Save* click with nothing pending should be a silent no-op.""" + stub = _make_gramplet(db_open=True, dirty=False) + + stub.save_data_edit(None) + + assert captured_errors == [] + assert stub._dirty is False + + +# --------------------------------------------------------------------------- +# Pre-existing guards — lock them in against regressions +# --------------------------------------------------------------------------- +def test_add_data_entry_requires_name(captured_errors): + """Empty name with a valid tree should surface the name-required error.""" + stub = _make_gramplet(db_open=True, np_name="") + + stub.add_data_entry(None) + + assert captured_errors + title, _body = captured_errors[0] + assert "name" in title.lower() + + +def test_add_data_entry_parent_without_active_person(captured_errors): + """Adding as a parent without an active person surfaces a clear error.""" + stub = _make_gramplet( + db_open=True, + np_name="Doe, Jane", + np_relation=DataEntryGramplet.AS_PARENT, + ) + + stub.add_data_entry(None) + + assert captured_errors + _title, body = captured_errors[0] + assert "parent" in body.lower() + + +# --------------------------------------------------------------------------- +# Plugin registration — catch metadata breakage early +# --------------------------------------------------------------------------- +def test_gpr_registration_metadata(): + """The .gpr.py file must register a single gramplet with expected keys.""" + gpr_path = os.path.join(ADDON_DIR, "DataEntryGramplet.gpr.py") + calls: list[tuple[tuple, dict]] = [] + + namespace = { + "register": lambda *args, **kwargs: calls.append((args, kwargs)), + "GRAMPLET": "GRAMPLET", + "STABLE": "STABLE", + "EXPERT": "EXPERT", + "_": lambda s: s, + } + with open(gpr_path, encoding="utf-8") as handle: + exec(compile(handle.read(), gpr_path, "exec"), namespace) + + assert len(calls) == 1, "expected exactly one register() call" + args, kwargs = calls[0] + assert args == ("GRAMPLET",) + assert kwargs["id"] == "Data Entry Gramplet" + assert kwargs["gramplet"] == "DataEntryGramplet" + assert kwargs["fname"] == "DataEntryGramplet.py" + assert kwargs["gramps_target_version"] == "6.0" + assert kwargs["status"] == "STABLE" + # Navigation type must stay Person — the active object is fetched that way. + assert kwargs["navtypes"] == ["Person"] From e4d7f816fb3c6bc706ffe8d54e74f35b10592ffd Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sun, 19 Apr 2026 13:57:52 +0200 Subject: [PATCH 05/21] DataEntryGramplet: convert tests to stdlib unittest (bug 0012691) Gramps' own test suite uses unittest, not pytest, so addon tests that ship alongside the codebase should follow the same convention to stay contributable upstream without rewriting. Replace pytest fixtures, monkeypatch, and module-level test functions with unittest.TestCase classes and mock.patch.object. Guard the module-level addon import with a try/except that raises SkipTest so collection is quiet on environments without the GUI stack, and pin Gtk to 3.0 before gramps imports to avoid the GTK4 fallback crash on Gtk.IconSize.MENU. --- .../tests/test_data_entry_gramplet.py | 330 ++++++++++++------ 1 file changed, 216 insertions(+), 114 deletions(-) diff --git a/DataEntryGramplet/tests/test_data_entry_gramplet.py b/DataEntryGramplet/tests/test_data_entry_gramplet.py index e012df87e..9f48f39cb 100644 --- a/DataEntryGramplet/tests/test_data_entry_gramplet.py +++ b/DataEntryGramplet/tests/test_data_entry_gramplet.py @@ -35,72 +35,146 @@ # ------------------------ import os import sys +import unittest +from typing import Any +from unittest import mock + +# The addon imports Gtk at module load — skip cleanly if gi/Gtk are not +# available. On systems where both GTK3 and GTK4 are present, pin Gtk to +# 3.0 before any gramps import (mirrors what gramps.grampsapp does at +# startup); otherwise PyGObject loads GTK4 and the gramps.gui import +# chain crashes on Gtk.IconSize.MENU (a GTK3-only enum). +try: + import gi + + gi.require_version("Gtk", "3.0") + gi.require_version("Gdk", "3.0") +except (ImportError, ValueError, AttributeError) as err: + raise unittest.SkipTest("GTK 3.0 / PyGObject not available: %s" % err) -import pytest - -# Gramps is imported by the module under test. -pytest.importorskip("gramps") - - -# --------------------------------------------------------------------------- -# Make the addon importable and ensure GRAMPS_RESOURCES is set -# --------------------------------------------------------------------------- +# ------------------------ +# Gramps modules +# ------------------------ +# Addon root goes on sys.path so ``from DataEntryGramplet import +# DataEntryGramplet`` resolves the module-level class. The unittest +# discovery entrypoint (tests/) lacks an __init__.py, so this +# ``__file__``-based hack is still the right way to make the addon +# importable during local and CI runs. ADDON_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) if ADDON_DIR not in sys.path: sys.path.insert(0, ADDON_DIR) -if "GRAMPS_RESOURCES" not in os.environ: +try: import gramps +except ImportError as err: + raise unittest.SkipTest("gramps package not available: %s" % err) +if "GRAMPS_RESOURCES" not in os.environ: os.environ["GRAMPS_RESOURCES"] = os.path.dirname( os.path.dirname(gramps.__file__) ) - # ------------------------ -# Module under test +# Gramps specific # ------------------------ -import gramps.gui.dialog as gramps_dialog # noqa: E402 - -from DataEntryGramplet import DataEntryGramplet # noqa: E402 - - -# --------------------------------------------------------------------------- -# Lightweight stubs — enough surface for the guard paths under test -# --------------------------------------------------------------------------- +# The addon module pulls in the full Gramps GUI stack at import time. On +# environments where GTK is missing or version-mismatched the import +# fails; skip the whole module cleanly in that case so collection does +# not surface spurious errors. +try: + import gramps.gui.dialog as gramps_dialog # noqa: E402 + from DataEntryGramplet import DataEntryGramplet # noqa: E402 +except Exception as err: # noqa: BLE001 — environment guard + raise unittest.SkipTest("DataEntryGramplet module unavailable: %s" % err) + + +# ------------------------------------------------------------ +# +# _FakeDbState +# +# ------------------------------------------------------------ class _FakeDbState: - def __init__(self, is_open=True): + """Stub for ``Gramplet.dbstate`` — only ``is_open()`` is consulted + by the guards under test.""" + + def __init__(self, is_open: bool = True) -> None: + """ + :param is_open: Value returned from ``is_open()``. + :type is_open: bool + """ self._is_open = is_open - def is_open(self): + def is_open(self) -> bool: + """Return the configured open state.""" return self._is_open +# ------------------------------------------------------------ +# +# _FakeEntry +# +# ------------------------------------------------------------ class _FakeEntry: - def __init__(self, text=""): + """Stub for ``Gtk.Entry`` — exposes only ``get_text()``.""" + + def __init__(self, text: str = "") -> None: + """ + :param text: Value returned from ``get_text()``. + :type text: str + """ self._text = text - def get_text(self): + def get_text(self) -> str: + """Return the configured text.""" return self._text +# ------------------------------------------------------------ +# +# _FakeCombo +# +# ------------------------------------------------------------ class _FakeCombo: - def __init__(self, active=0): + """Stub for ``Gtk.ComboBox`` — exposes only ``get_active()``.""" + + def __init__(self, active: int = 0) -> None: + """ + :param active: Value returned from ``get_active()``. + :type active: int + """ self._active = active - def get_active(self): + def get_active(self) -> int: + """Return the configured active index.""" return self._active def _make_gramplet( *, - db_open=True, - np_name="", - np_gender=2, # UNKNOWN - np_relation=DataEntryGramplet.NO_REL, - dirty=False, -): - """Return an instance skipping ``__init__`` so no GTK is required.""" + db_open: bool = True, + np_name: str = "", + np_gender: int = 2, # UNKNOWN + np_relation: int = DataEntryGramplet.NO_REL, + dirty: bool = False, +) -> Any: + """ + Build a ``DataEntryGramplet`` via ``__new__`` so its ``__init__`` + (which pulls in GTK) does not run, then attach just enough state + for the isolated guards to execute. + + :param db_open: Whether ``dbstate.is_open()`` reports an open tree. + :type db_open: bool + :param np_name: Value stored in the Name entry. + :type np_name: str + :param np_gender: Value stored in the Gender combo (UNKNOWN by default). + :type np_gender: int + :param np_relation: Value stored in the Relation combo. + :type np_relation: int + :param dirty: Value of the private ``_dirty`` flag. + :type dirty: bool + :returns: A stub instance with the minimum attributes set. + :rtype: DataEntryGramplet + """ stub = DataEntryGramplet.__new__(DataEntryGramplet) stub.dbstate = _FakeDbState(db_open) stub._dirty = dirty @@ -116,108 +190,136 @@ def _make_gramplet( return stub -@pytest.fixture -def captured_errors(monkeypatch): - """Replace gramps.gui.dialog.ErrorDialog so tests can inspect calls.""" - shown: list[tuple[str, str]] = [] - - def _fake(title, body="", *args, **kwargs): - shown.append((str(title), str(body))) +# ------------------------------------------------------------ +# +# _ErrorDialogTestCase +# +# ------------------------------------------------------------ +class _ErrorDialogTestCase(unittest.TestCase): + """Base class that patches ``gramps.gui.dialog.ErrorDialog`` so + tests can inspect calls without opening any GTK dialogs.""" - monkeypatch.setattr(gramps_dialog, "ErrorDialog", _fake) - return shown + def setUp(self) -> None: + """Install the ErrorDialog capture and reset the buffer.""" + self.captured_errors: list[tuple[str, str]] = [] + def _fake(title: Any, body: Any = "", *_args: Any, **_kwargs: Any) -> None: + self.captured_errors.append((str(title), str(body))) -# --------------------------------------------------------------------------- -# Bug 12691 — closed Family Tree must not crash -# --------------------------------------------------------------------------- -def test_add_data_entry_with_closed_db_shows_error(captured_errors): - """Pressing *Add* with no tree open surfaces an ErrorDialog, not a crash.""" - stub = _make_gramplet(db_open=False, np_name="Doe, Jane") + patcher = mock.patch.object(gramps_dialog, "ErrorDialog", _fake) + patcher.start() + self.addCleanup(patcher.stop) - stub.add_data_entry(None) - assert captured_errors, "ErrorDialog was not displayed" - title, body = captured_errors[0] - assert "Family Tree" in title - assert "open" in body.lower() +# ------------------------------------------------------------ +# +# TestBug12691ClosedDb +# +# ------------------------------------------------------------ +class TestBug12691ClosedDb(_ErrorDialogTestCase): + """Regression coverage for bug 12691 — pressing *Add* or *Save* + with no tree open must surface an ErrorDialog instead of crashing + inside ``DbTxn`` with ``AttributeError: 'DummyDb' ... get_undodb``.""" + def test_add_data_entry_with_closed_db_shows_error(self) -> None: + """*Add* with no tree open surfaces an ErrorDialog, not a crash.""" + stub = _make_gramplet(db_open=False, np_name="Doe, Jane") -def test_save_data_edit_with_closed_db_shows_error(captured_errors): - """Pressing *Save* while dirty with no tree open must not invoke DbTxn.""" - stub = _make_gramplet(db_open=False, dirty=True) + stub.add_data_entry(None) - stub.save_data_edit(None) + self.assertTrue(self.captured_errors, "ErrorDialog was not displayed") + title, body = self.captured_errors[0] + self.assertIn("Family Tree", title) + self.assertIn("open", body.lower()) - assert captured_errors, "ErrorDialog was not displayed" - title, _body = captured_errors[0] - assert "Family Tree" in title + def test_save_data_edit_with_closed_db_shows_error(self) -> None: + """*Save* while dirty with no tree open must not invoke DbTxn.""" + stub = _make_gramplet(db_open=False, dirty=True) + stub.save_data_edit(None) -def test_save_data_edit_noop_when_not_dirty(captured_errors): - """A *Save* click with nothing pending should be a silent no-op.""" - stub = _make_gramplet(db_open=True, dirty=False) + self.assertTrue(self.captured_errors, "ErrorDialog was not displayed") + title, _body = self.captured_errors[0] + self.assertIn("Family Tree", title) - stub.save_data_edit(None) + def test_save_data_edit_noop_when_not_dirty(self) -> None: + """A *Save* click with nothing pending should be a silent no-op.""" + stub = _make_gramplet(db_open=True, dirty=False) - assert captured_errors == [] - assert stub._dirty is False + stub.save_data_edit(None) + self.assertEqual(self.captured_errors, []) + self.assertFalse(stub._dirty) -# --------------------------------------------------------------------------- -# Pre-existing guards — lock them in against regressions -# --------------------------------------------------------------------------- -def test_add_data_entry_requires_name(captured_errors): - """Empty name with a valid tree should surface the name-required error.""" - stub = _make_gramplet(db_open=True, np_name="") - stub.add_data_entry(None) +# ------------------------------------------------------------ +# +# TestInputGuards +# +# ------------------------------------------------------------ +class TestInputGuards(_ErrorDialogTestCase): + """Lock in the pre-existing input validators so future refactors + cannot silently weaken the guardrails around ``add_data_entry``.""" - assert captured_errors - title, _body = captured_errors[0] - assert "name" in title.lower() + def test_add_data_entry_requires_name(self) -> None: + """Empty name with a valid tree should surface the name-required error.""" + stub = _make_gramplet(db_open=True, np_name="") + stub.add_data_entry(None) -def test_add_data_entry_parent_without_active_person(captured_errors): - """Adding as a parent without an active person surfaces a clear error.""" - stub = _make_gramplet( - db_open=True, - np_name="Doe, Jane", - np_relation=DataEntryGramplet.AS_PARENT, - ) + self.assertTrue(self.captured_errors) + title, _body = self.captured_errors[0] + self.assertIn("name", title.lower()) - stub.add_data_entry(None) + def test_add_data_entry_parent_without_active_person(self) -> None: + """Adding as a parent without an active person surfaces a clear error.""" + stub = _make_gramplet( + db_open=True, + np_name="Doe, Jane", + np_relation=DataEntryGramplet.AS_PARENT, + ) - assert captured_errors - _title, body = captured_errors[0] - assert "parent" in body.lower() + stub.add_data_entry(None) + self.assertTrue(self.captured_errors) + _title, body = self.captured_errors[0] + self.assertIn("parent", body.lower()) -# --------------------------------------------------------------------------- -# Plugin registration — catch metadata breakage early -# --------------------------------------------------------------------------- -def test_gpr_registration_metadata(): - """The .gpr.py file must register a single gramplet with expected keys.""" - gpr_path = os.path.join(ADDON_DIR, "DataEntryGramplet.gpr.py") - calls: list[tuple[tuple, dict]] = [] - namespace = { - "register": lambda *args, **kwargs: calls.append((args, kwargs)), - "GRAMPLET": "GRAMPLET", - "STABLE": "STABLE", - "EXPERT": "EXPERT", - "_": lambda s: s, - } - with open(gpr_path, encoding="utf-8") as handle: - exec(compile(handle.read(), gpr_path, "exec"), namespace) - - assert len(calls) == 1, "expected exactly one register() call" - args, kwargs = calls[0] - assert args == ("GRAMPLET",) - assert kwargs["id"] == "Data Entry Gramplet" - assert kwargs["gramplet"] == "DataEntryGramplet" - assert kwargs["fname"] == "DataEntryGramplet.py" - assert kwargs["gramps_target_version"] == "6.0" - assert kwargs["status"] == "STABLE" - # Navigation type must stay Person — the active object is fetched that way. - assert kwargs["navtypes"] == ["Person"] +# ------------------------------------------------------------ +# +# TestGprRegistration +# +# ------------------------------------------------------------ +class TestGprRegistration(unittest.TestCase): + """Catch metadata breakage in the plugin registration file early.""" + + def test_gpr_registration_metadata(self) -> None: + """The .gpr.py file must register a single gramplet with expected keys.""" + gpr_path = os.path.join(ADDON_DIR, "DataEntryGramplet.gpr.py") + calls: list[tuple[tuple, dict]] = [] + + namespace: dict[str, Any] = { + "register": lambda *args, **kwargs: calls.append((args, kwargs)), + "GRAMPLET": "GRAMPLET", + "STABLE": "STABLE", + "EXPERT": "EXPERT", + "_": lambda s: s, + } + with open(gpr_path, encoding="utf-8") as handle: + exec(compile(handle.read(), gpr_path, "exec"), namespace) + + self.assertEqual(len(calls), 1, "expected exactly one register() call") + args, kwargs = calls[0] + self.assertEqual(args, ("GRAMPLET",)) + self.assertEqual(kwargs["id"], "Data Entry Gramplet") + self.assertEqual(kwargs["gramplet"], "DataEntryGramplet") + self.assertEqual(kwargs["fname"], "DataEntryGramplet.py") + self.assertEqual(kwargs["gramps_target_version"], "6.0") + self.assertEqual(kwargs["status"], "STABLE") + # Navigation type must stay Person — the active object is fetched that way. + self.assertEqual(kwargs["navtypes"], ["Person"]) + + +if __name__ == "__main__": + unittest.main() From 84121156b4f915c49eadbb9992bc1b87e14bbb6f Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Mon, 20 Apr 2026 18:21:02 +0200 Subject: [PATCH 06/21] DataEntryGramplet: qualify test import to pick up the class not the module When unittest loads this file as DataEntryGramplet.tests.test_..., the outer DataEntryGramplet is already a namespace package in sys.modules, so `from DataEntryGramplet import DataEntryGramplet` binds the submodule and DataEntryGramplet.NO_REL (class attr) raises AttributeError. Fix by importing the class via its fully-qualified path. --- .../tests/test_data_entry_gramplet.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/DataEntryGramplet/tests/test_data_entry_gramplet.py b/DataEntryGramplet/tests/test_data_entry_gramplet.py index 9f48f39cb..72005d553 100644 --- a/DataEntryGramplet/tests/test_data_entry_gramplet.py +++ b/DataEntryGramplet/tests/test_data_entry_gramplet.py @@ -55,11 +55,15 @@ # ------------------------ # Gramps modules # ------------------------ -# Addon root goes on sys.path so ``from DataEntryGramplet import -# DataEntryGramplet`` resolves the module-level class. The unittest -# discovery entrypoint (tests/) lacks an __init__.py, so this -# ``__file__``-based hack is still the right way to make the addon -# importable during local and CI runs. +# Addon root goes on sys.path so ``from DataEntryGramplet.DataEntryGramplet +# import DataEntryGramplet`` resolves the class inside the addon module. +# The fully-qualified form matters: when unittest loads this file as +# ``DataEntryGramplet.tests.test_...``, the outer ``DataEntryGramplet`` +# is already a namespace package in ``sys.modules``, so a bare +# ``from DataEntryGramplet import DataEntryGramplet`` would bind the +# submodule instead of the class. The ``tests/`` directory lacks an +# __init__.py, so this ``__file__``-based hack is still the right way +# to make the addon importable during local and CI runs. ADDON_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) if ADDON_DIR not in sys.path: sys.path.insert(0, ADDON_DIR) @@ -83,7 +87,7 @@ # not surface spurious errors. try: import gramps.gui.dialog as gramps_dialog # noqa: E402 - from DataEntryGramplet import DataEntryGramplet # noqa: E402 + from DataEntryGramplet.DataEntryGramplet import DataEntryGramplet # noqa: E402 except Exception as err: # noqa: BLE001 — environment guard raise unittest.SkipTest("DataEntryGramplet module unavailable: %s" % err) From 5d386efe6278d712359ef1ab7b226b6d69f326bf Mon Sep 17 00:00:00 2001 From: GaryGriffin Date: Tue, 21 Apr 2026 09:31:18 -0700 Subject: [PATCH 07/21] Merge DataEntryGramplet: fix crash when adding person with no Family Tree open (bug 0012691) #824 --- DataEntryGramplet/DataEntryGramplet.gpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DataEntryGramplet/DataEntryGramplet.gpr.py b/DataEntryGramplet/DataEntryGramplet.gpr.py index 50308593f..dc61a1bab 100644 --- a/DataEntryGramplet/DataEntryGramplet.gpr.py +++ b/DataEntryGramplet/DataEntryGramplet.gpr.py @@ -14,7 +14,7 @@ gramplet_title=_("Data Entry"), detached_width=510, detached_height=480, - version = '1.0.52', + version = '1.0.53', gramps_target_version="6.0", status=STABLE, audience=EXPERT, From 4f087ee75081ac3f257e1173799296fcf052d08d Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 02:03:26 +0200 Subject: [PATCH 08/21] CalculateEstimatedDates: handle ancestry-loop DatabaseError per-person (bug 0007898) probably_alive_range raises DatabaseError when it detects loops in ancestor or descendant chains. Previously this propagated out of the removal, selection, and apply loops and tore down the entire tool, leaving signals disabled and the progress dialog stuck open. Wrap each per-person iteration with try/except so a single bad record is logged and skipped, and add outer try/finally blocks so signals are re-enabled and the progress dialog is closed even on unexpected failures. Surface a "Skipped N people due to errors" message to the user when any rows were skipped. Add unit tests covering get_modifier branches, calc_estimates happy path, DatabaseError propagation from calc_estimates, and .gpr.py registration metadata. The addon module is loaded lazily inside a fixture so pytest collection succeeds even when the GUI stack cannot import. Co-Authored-By: Claude Opus 4.7 --- .../CalculateEstimatedDates.py | 438 ++++++++++-------- .../tests/test_calculate_estimated_dates.py | 206 ++++++++ 2 files changed, 456 insertions(+), 188 deletions(-) create mode 100644 CalculateEstimatedDates/tests/test_calculate_estimated_dates.py diff --git a/CalculateEstimatedDates/CalculateEstimatedDates.py b/CalculateEstimatedDates/CalculateEstimatedDates.py index 7e2c166a2..297cdce6c 100644 --- a/CalculateEstimatedDates/CalculateEstimatedDates.py +++ b/CalculateEstimatedDates/CalculateEstimatedDates.py @@ -29,6 +29,7 @@ # python modules # #------------------------------------------------------------------------ +import logging import time #------------------------------------------------------------------------ @@ -43,6 +44,7 @@ from gramps.gen.lib import (Date, Event, EventType, EventRef, Source, Citation, Note, NoteType) from gramps.gen.db import DbTxn +from gramps.gen.errors import DatabaseError from gramps.gen.config import config from gramps.gen.display.name import displayer as name_displayer from gramps.gen.plug.report.utils import get_person_filters @@ -53,6 +55,8 @@ from gramps.gen.utils.alive import probably_alive_range from gramps.gen.datehandler import displayer as date_displayer from gramps.gen.const import GRAMPS_LOCALE as glocale + +LOG = logging.getLogger(__name__) try: _trans = glocale.get_addon_translator(__file__) except ValueError: @@ -250,140 +254,183 @@ def run(self): self.MAX_AGE_PROB_ALIVE = self.options.handler.options_dict['MAX_AGE_PROB_ALIVE'] self.AVG_GENERATION_GAP = self.options.handler.options_dict['AVG_GENERATION_GAP'] if remove_old: - with DbTxn("", self.db, batch=True) as self.trans: - self.db.disable_signals() - self.results_write(_("Removing old estimations... ")) - self.progress.set_pass((_("Removing '%s'...") % source_text), - num_people) - supdate = None - for person_handle in people: - self.progress.step() - pupdate = 0 - person = self.db.get_person_from_handle(person_handle) - birth_ref = person.get_birth_ref() - if birth_ref: - birth = self.db.get_event_from_handle(birth_ref.ref) - for citation_handle in birth.get_citation_list(): - citation = self.db.get_citation_from_handle(citation_handle) - source_handle = citation.get_reference_handle() - #print "birth handle:", source_handle - source = self.db.get_source_from_handle(source_handle) - if source: - if source.get_title() == source_text: - #print("birth event removed from:", - # person.gramps_id) - person.set_birth_ref(None) - person.remove_handle_references('Event',[birth_ref.ref]) - # remove note - note_list = birth.get_referenced_note_handles() - birth.remove_handle_references('Note', - [note_handle for (obj_type, note_handle) in note_list]) - for (obj_type, note_handle) in note_list: - self.db.remove_note(note_handle, self.trans) - self.db.remove_event(birth_ref.ref, self.trans) - self.db.remove_citation(citation_handle, - self.trans) - pupdate = 1 - supdate = source # found the source. - break - death_ref = person.get_death_ref() - if death_ref: - death = self.db.get_event_from_handle(death_ref.ref) - for citation_handle in death.get_citation_list(): - citation = self.db.get_citation_from_handle(citation_handle) - source_handle = citation.get_reference_handle() - #print "death handle:", source_handle - source = self.db.get_source_from_handle(source_handle) - if source: - if source.get_title() == source_text: - #print("death event removed from:", - # person.gramps_id) - person.set_death_ref(None) - person.remove_handle_references('Event',[death_ref.ref]) - # remove note - note_list = death.get_referenced_note_handles() - death.remove_handle_references('Note', - [note_handle for (obj_type, note_handle) in note_list]) - for (obj_type, note_handle) in note_list: - self.db.remove_note(note_handle, self.trans) - self.db.remove_event(death_ref.ref, self.trans) - self.db.remove_citation(citation_handle, - self.trans) - pupdate = 1 - supdate = source # found the source. - break - if pupdate == 1: - self.db.commit_person(person, self.trans) - if supdate: - self.db.remove_source(supdate.handle, self.trans) - self.results_write(_("done!\n")) - self.db.enable_signals() + skipped = 0 + try: + with DbTxn("", self.db, batch=True) as self.trans: + self.db.disable_signals() + self.results_write(_("Removing old estimations... ")) + self.progress.set_pass((_("Removing '%s'...") % source_text), + num_people) + supdate = None + for person_handle in people: + self.progress.step() + try: + pupdate = 0 + person = self.db.get_person_from_handle(person_handle) + birth_ref = person.get_birth_ref() + if birth_ref: + birth = self.db.get_event_from_handle(birth_ref.ref) + for citation_handle in birth.get_citation_list(): + citation = self.db.get_citation_from_handle(citation_handle) + source_handle = citation.get_reference_handle() + #print "birth handle:", source_handle + source = self.db.get_source_from_handle(source_handle) + if source: + if source.get_title() == source_text: + #print("birth event removed from:", + # person.gramps_id) + person.set_birth_ref(None) + person.remove_handle_references('Event',[birth_ref.ref]) + # remove note + note_list = birth.get_referenced_note_handles() + birth.remove_handle_references('Note', + [note_handle for (obj_type, note_handle) in note_list]) + for (obj_type, note_handle) in note_list: + self.db.remove_note(note_handle, self.trans) + self.db.remove_event(birth_ref.ref, self.trans) + self.db.remove_citation(citation_handle, + self.trans) + pupdate = 1 + supdate = source # found the source. + break + death_ref = person.get_death_ref() + if death_ref: + death = self.db.get_event_from_handle(death_ref.ref) + for citation_handle in death.get_citation_list(): + citation = self.db.get_citation_from_handle(citation_handle) + source_handle = citation.get_reference_handle() + #print "death handle:", source_handle + source = self.db.get_source_from_handle(source_handle) + if source: + if source.get_title() == source_text: + #print("death event removed from:", + # person.gramps_id) + person.set_death_ref(None) + person.remove_handle_references('Event',[death_ref.ref]) + # remove note + note_list = death.get_referenced_note_handles() + death.remove_handle_references('Note', + [note_handle for (obj_type, note_handle) in note_list]) + for (obj_type, note_handle) in note_list: + self.db.remove_note(note_handle, self.trans) + self.db.remove_event(death_ref.ref, self.trans) + self.db.remove_citation(citation_handle, + self.trans) + pupdate = 1 + supdate = source # found the source. + break + if pupdate == 1: + self.db.commit_person(person, self.trans) + except Exception: + skipped += 1 + LOG.warning( + "Failed to remove estimated dates for person" + " handle %s; skipping.", + person_handle, exc_info=True) + continue + if supdate: + try: + self.db.remove_source(supdate.handle, self.trans) + except Exception: + LOG.warning( + "Failed to remove estimated-dates source;" + " continuing.", exc_info=True) + self.results_write(_("done!\n")) + if skipped: + self.results_write( + _("Skipped %d people due to errors (see log).\n") + % skipped) + finally: + self.db.enable_signals() self.db.request_rebuild() if add_birth or add_death: self.results_write(_("Selecting... \n\n")) self.progress.set_pass(_('Selecting...'), num_people) row = 0 + skipped = 0 for person_handle in people: self.progress.step() - person = self.db.get_person_from_handle(person_handle) - birth_ref = person.get_birth_ref() - death_ref = person.get_death_ref() - add_birth_event, add_death_event = False, False - if not birth_ref or not death_ref: - date1, date2, explain, other = self.calc_estimates(person) - if birth_ref: - ev = self.db.get_event_from_handle(birth_ref.ref) - date1 = ev.get_date_object() - elif not birth_ref and add_birth and date1: - if date1.match( current_date, "<"): - add_birth_event = True - date1.make_vague() + try: + person = self.db.get_person_from_handle(person_handle) + birth_ref = person.get_birth_ref() + death_ref = person.get_death_ref() + add_birth_event, add_death_event = False, False + if not birth_ref or not death_ref: + try: + date1, date2, explain, other = \ + self.calc_estimates(person) + except DatabaseError as err: + skipped += 1 + LOG.warning( + "Could not estimate dates for %s (%s): %s" + " — skipping.", + name_displayer.display(person), + person.gramps_id, err) + continue + if birth_ref: + ev = self.db.get_event_from_handle(birth_ref.ref) + date1 = ev.get_date_object() + elif not birth_ref and add_birth and date1: + if date1.match( current_date, "<"): + add_birth_event = True + date1.make_vague() + else: + date1 = Date() else: date1 = Date() - else: - date1 = Date() - if death_ref: - ev = self.db.get_event_from_handle(death_ref.ref) - date2 = ev.get_date_object() - elif not death_ref and add_death and date2: - if date2.match( current_date, "<"): - add_death_event = True - date2.make_vague() + if death_ref: + ev = self.db.get_event_from_handle(death_ref.ref) + date2 = ev.get_date_object() + elif not death_ref and add_death and date2: + if date2.match( current_date, "<"): + add_death_event = True + date2.make_vague() + else: + date2 = Date() else: date2 = Date() - else: - date2 = Date() - # Describe - if add_birth_event and add_death_event: - action = _("Add birth and death events") - elif add_birth_event: - action = _("Add birth event") - elif add_death_event: - action = _("Add death event") - else: - continue - #stab.columns(_("Select"), _("Person"), _("Action"), - # _("Birth Date"), _("Death Date"), _("Evidence"), _("Relative")) - if add_birth == 1 and not birth_ref: # no date - date1 = Date() - if add_death == 1 and not death_ref: # no date - date2 = Date() - if person == other: - other = None - stab.row("checkbox", - person, - action, - date1, - date2, - explain or "", - other or "") - if add_birth_event: - stab.set_cell_markup(3, row, "%s" % date_displayer.display(date1)) - if add_death_event: - stab.set_cell_markup(4, row, "%s" % date_displayer.display(date2)) - self.action[person.handle] = (add_birth_event, add_death_event) - row += 1 + # Describe + if add_birth_event and add_death_event: + action = _("Add birth and death events") + elif add_birth_event: + action = _("Add birth event") + elif add_death_event: + action = _("Add death event") + else: + continue + #stab.columns(_("Select"), _("Person"), _("Action"), + # _("Birth Date"), _("Death Date"), _("Evidence"), _("Relative")) + if add_birth == 1 and not birth_ref: # no date + date1 = Date() + if add_death == 1 and not death_ref: # no date + date2 = Date() + if person == other: + other = None + stab.row("checkbox", + person, + action, + date1, + date2, + explain or "", + other or "") + if add_birth_event: + stab.set_cell_markup(3, row, "%s" % date_displayer.display(date1)) + if add_death_event: + stab.set_cell_markup(4, row, "%s" % date_displayer.display(date2)) + self.action[person.handle] = (add_birth_event, add_death_event) + row += 1 + except Exception: + skipped += 1 + LOG.warning( + "Unexpected error processing person handle %s;" + " skipping.", + person_handle, exc_info=True) + continue + if skipped: + self.results_write( + _("Skipped %d people due to errors (see log).\n") + % skipped) if row > 0: self.results_write(" ") for text, function in BUTTONS: @@ -430,75 +477,90 @@ def apply_selection(self, *args, **kwargs): # Do not add birth or death event if one exists, no matter what if self.table.treeview.get_model() is None: return - with DbTxn("", self.db, batch=True) as self.trans: - self.pre_run() - source_text = self.options.handler.options_dict['source_text'] - select_col = self.table.model_index_of_column[_("Select")] - source = self.get_or_create_source(source_text) - self.db.disable_signals() - self.results_write(_("Selecting... ")) - self.progress.set_pass((_("Adding events '%s'...") % source_text), - len(self.table.treeview.get_model())) - count = 0 - for row in self.table.treeview.get_model(): - self.progress.step() - select = row[select_col] # live select value - if not select: - continue - pupdate = False - index = row[0] # order put in - row_data = self.table.get_raw_data(index) - person = row_data[1] # check, person, action, date1, date2 - date1 = row_data[3] # date - date2 = row_data[4] # date - evidence = row_data[5] # evidence - other = row_data[6] # other person - if other: - other_name = self.sdb.name(other) - else: - other_name = None - add_birth_event, add_death_event = self.action[person.handle] - birth_ref = person.get_birth_ref() - death_ref = person.get_death_ref() - if not birth_ref and add_birth_event: - if other_name: - explanation = _("Added birth event based on %(evidence)s, from %(name)s") % { - 'evidence' : evidence, 'name' : other_name } - else: - explanation = _("Added birth event based on %s") % evidence - modifier = self.get_modifier("birth") - birth = self.create_event(_("Estimated birth date"), - EventType.BIRTH, - date1, source, explanation, modifier) - event_ref = EventRef() - event_ref.set_reference_handle(birth.get_handle()) - person.set_birth_ref(event_ref) - pupdate = True - count += 1 - if not death_ref and add_death_event: - if other_name: - explanation = _("Added death event based on %(evidence)s, from %(person)s") % { - 'evidence' : evidence, 'person' : other_name } - else: - explanation = _("Added death event based on %s") % evidence - modifier = self.get_modifier("death") - death = self.create_event(_("Estimated death date"), - EventType.DEATH, - date2, source, explanation, modifier) - event_ref = EventRef() - event_ref.set_reference_handle(death.get_handle()) - person.set_death_ref(event_ref) - pupdate = True - count += 1 - if pupdate: - self.db.commit_person(person, self.trans) - self.results_write(_(" Done! Committing...")) - self.results_write("\n") - self.db.enable_signals() + count = 0 + skipped = 0 + try: + with DbTxn("", self.db, batch=True) as self.trans: + self.pre_run() + source_text = self.options.handler.options_dict['source_text'] + select_col = self.table.model_index_of_column[_("Select")] + source = self.get_or_create_source(source_text) + self.db.disable_signals() + self.results_write(_("Selecting... ")) + self.progress.set_pass( + (_("Adding events '%s'...") % source_text), + len(self.table.treeview.get_model())) + for row in self.table.treeview.get_model(): + self.progress.step() + select = row[select_col] # live select value + if not select: + continue + try: + pupdate = False + index = row[0] # order put in + row_data = self.table.get_raw_data(index) + person = row_data[1] # check, person, action, date1, date2 + date1 = row_data[3] # date + date2 = row_data[4] # date + evidence = row_data[5] # evidence + other = row_data[6] # other person + if other: + other_name = self.sdb.name(other) + else: + other_name = None + add_birth_event, add_death_event = self.action[person.handle] + birth_ref = person.get_birth_ref() + death_ref = person.get_death_ref() + if not birth_ref and add_birth_event: + if other_name: + explanation = _("Added birth event based on %(evidence)s, from %(name)s") % { + 'evidence' : evidence, 'name' : other_name } + else: + explanation = _("Added birth event based on %s") % evidence + modifier = self.get_modifier("birth") + birth = self.create_event(_("Estimated birth date"), + EventType.BIRTH, + date1, source, explanation, modifier) + event_ref = EventRef() + event_ref.set_reference_handle(birth.get_handle()) + person.set_birth_ref(event_ref) + pupdate = True + count += 1 + if not death_ref and add_death_event: + if other_name: + explanation = _("Added death event based on %(evidence)s, from %(person)s") % { + 'evidence' : evidence, 'person' : other_name } + else: + explanation = _("Added death event based on %s") % evidence + modifier = self.get_modifier("death") + death = self.create_event(_("Estimated death date"), + EventType.DEATH, + date2, source, explanation, modifier) + event_ref = EventRef() + event_ref.set_reference_handle(death.get_handle()) + person.set_death_ref(event_ref) + pupdate = True + count += 1 + if pupdate: + self.db.commit_person(person, self.trans) + except Exception: + skipped += 1 + LOG.warning( + "Failed to apply estimated dates for row %s;" + " skipping.", + row[0] if row else "?", exc_info=True) + continue + self.results_write(_(" Done! Committing...")) + self.results_write("\n") + finally: + self.db.enable_signals() + self.progress.close() self.db.request_rebuild() self.results_write(_("Added %d events.") % count) + if skipped: + self.results_write( + _(" (Skipped %d rows due to errors; see log.)") % skipped) self.results_write("\n\n") - self.progress.close() def get_modifier(self, event_type): setting = self.options.handler.options_dict['dates'] diff --git a/CalculateEstimatedDates/tests/test_calculate_estimated_dates.py b/CalculateEstimatedDates/tests/test_calculate_estimated_dates.py new file mode 100644 index 000000000..8aa9a2f79 --- /dev/null +++ b/CalculateEstimatedDates/tests/test_calculate_estimated_dates.py @@ -0,0 +1,206 @@ +# +# Gramps - a GTK+/GNOME based genealogy program +# +# Copyright (C) 2026 Eduard Ralph +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# + +""" +Tests for the Calculate Estimated Dates tool — locks in the error +handling added in response to ``gramps-project/bugs#7898`` (ancestry +loops surfaced as ``DatabaseError`` from ``probably_alive_range`` +were tearing down the whole tool). + +The tool's ``__init__`` pulls in the full Gramps GUI stack, so these +tests build stub instances via ``__new__`` and exercise the isolated +helpers plus the ``.gpr.py`` registration. +""" + +# ------------------------ +# Python modules +# ------------------------ +import os +import sys + +import pytest + +# Gramps is imported transitively by the module under test. +pytest.importorskip("gramps") + + +# --------------------------------------------------------------------------- +# Make the addon importable and ensure GRAMPS_RESOURCES is set +# --------------------------------------------------------------------------- +ADDON_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if ADDON_DIR not in sys.path: + sys.path.insert(0, ADDON_DIR) + +if "GRAMPS_RESOURCES" not in os.environ: + import gramps + + os.environ["GRAMPS_RESOURCES"] = os.path.dirname( + os.path.dirname(gramps.__file__) + ) + + +# ------------------------ +# Gramps modules +# ------------------------ +from gramps.gen.errors import DatabaseError # noqa: E402 +from gramps.gen.lib import Date # noqa: E402 + + +# --------------------------------------------------------------------------- +# Lazy module loader — the addon pulls in the full Gramps GUI stack at +# import time, so on environments where GTK is missing or version-mismatched +# the import fails. Importing inside a fixture lets pytest collection succeed +# (and skip cleanly per-test) instead of hanging the entire collection phase. +# --------------------------------------------------------------------------- +@pytest.fixture +def ced_module(): + try: + from CalculateEstimatedDates import CalculateEstimatedDates as mod + except Exception as err: # pragma: no cover - environment guard + pytest.skip(f"CalculateEstimatedDates module unavailable: {err}") + return mod + + +# --------------------------------------------------------------------------- +# Lightweight stubs — enough surface for the helper paths under test +# --------------------------------------------------------------------------- +class _FakeOptionsHandler: + def __init__(self, dates=0): + self.options_dict = {"dates": dates} + + +class _FakeOptions: + def __init__(self, dates=0): + self.handler = _FakeOptionsHandler(dates=dates) + + +def _make_tool(ced_module, *, dates=0): + """Return a CalcToolManagedWindow skipping ``__init__`` (no GTK required).""" + cls = ced_module.CalcToolManagedWindow + stub = cls.__new__(cls) + stub.options = _FakeOptions(dates=dates) + stub.db = object() + stub.MAX_SIB_AGE_DIFF = 20 + stub.MAX_AGE_PROB_ALIVE = 110 + stub.AVG_GENERATION_GAP = 20 + return stub + + +# --------------------------------------------------------------------------- +# get_modifier — pure logic, four branches +# --------------------------------------------------------------------------- +def test_get_modifier_birth_about_when_dates_zero(ced_module): + """dates=0 + birth → MOD_ABOUT (the 'approximate' case).""" + tool = _make_tool(ced_module, dates=0) + assert tool.get_modifier("birth") == Date.MOD_ABOUT + + +def test_get_modifier_birth_after_when_dates_nonzero(ced_module): + """dates=1 + birth → MOD_AFTER (the 'extremes' case).""" + tool = _make_tool(ced_module, dates=1) + assert tool.get_modifier("birth") == Date.MOD_AFTER + + +def test_get_modifier_death_about_when_dates_zero(ced_module): + """dates=0 + death → MOD_ABOUT.""" + tool = _make_tool(ced_module, dates=0) + assert tool.get_modifier("death") == Date.MOD_ABOUT + + +def test_get_modifier_death_before_when_dates_nonzero(ced_module): + """dates=1 + death → MOD_BEFORE (upper-bound estimate).""" + tool = _make_tool(ced_module, dates=1) + assert tool.get_modifier("death") == Date.MOD_BEFORE + + +# --------------------------------------------------------------------------- +# calc_estimates — bug 7898: DatabaseError must propagate so run() can catch +# it per-person instead of letting one loop kill the whole tool. +# --------------------------------------------------------------------------- +def test_calc_estimates_returns_probably_alive_range_result( + ced_module, monkeypatch +): + """Happy path — the helper is a pass-through to probably_alive_range.""" + tool = _make_tool(ced_module) + person = object() + expected = (Date(), Date(), "explain", None) + + calls = [] + + def _fake(person_arg, db_arg, max_sib, max_age, avg_gap): + calls.append((person_arg, db_arg, max_sib, max_age, avg_gap)) + return expected + + monkeypatch.setattr(ced_module, "probably_alive_range", _fake) + + result = tool.calc_estimates(person) + + assert result == expected + assert calls == [(person, tool.db, 20, 110, 20)] + + +def test_calc_estimates_propagates_database_error(ced_module, monkeypatch): + """ + When ``probably_alive_range`` raises ``DatabaseError`` (e.g. an + ancestry loop), ``calc_estimates`` must let it escape so the + per-person handler in ``run()`` can log and skip. + """ + tool = _make_tool(ced_module) + + def _boom(*_args, **_kwargs): + raise DatabaseError("loop in Test, Abel's descendants") + + monkeypatch.setattr(ced_module, "probably_alive_range", _boom) + + with pytest.raises(DatabaseError, match="loop"): + tool.calc_estimates(object()) + + +# --------------------------------------------------------------------------- +# Plugin registration — catch metadata breakage early +# --------------------------------------------------------------------------- +def test_gpr_registration_metadata(): + """The .gpr.py file must register a single TOOL with expected keys.""" + gpr_path = os.path.join(ADDON_DIR, "CalculateEstimatedDates.gpr.py") + calls: list[tuple[tuple, dict]] = [] + + namespace = { + "register": lambda *args, **kwargs: calls.append((args, kwargs)), + "TOOL": "TOOL", + "STABLE": "STABLE", + "UNSTABLE": "UNSTABLE", + "TOOL_DBPROC": "TOOL_DBPROC", + "TOOL_MODE_GUI": "TOOL_MODE_GUI", + "_": lambda s: s, + } + with open(gpr_path, encoding="utf-8") as handle: + exec(compile(handle.read(), gpr_path, "exec"), namespace) + + assert len(calls) == 1, "expected exactly one register() call" + args, kwargs = calls[0] + assert args == ("TOOL",) + assert kwargs["id"] == "calculateestimateddates" + assert kwargs["fname"] == "CalculateEstimatedDates.py" + assert kwargs["gramps_target_version"] == "6.0" + assert kwargs["status"] == "STABLE" + assert kwargs["toolclass"] == "CalcToolManagedWindow" + assert kwargs["optionclass"] == "CalcEstDateOptions" + assert kwargs["category"] == "TOOL_DBPROC" + assert kwargs["tool_modes"] == ["TOOL_MODE_GUI"] From 5260da592f7980a8aff14b5379197beee0991f8a Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sun, 19 Apr 2026 13:49:31 +0200 Subject: [PATCH 09/21] CalculateEstimatedDates: convert tests to stdlib unittest (bug 0007898) Gramps' own test suite uses unittest, not pytest, so addon tests that ship alongside the codebase should follow the same convention to stay contributable upstream without rewriting. Replace pytest fixtures, monkeypatch, and pytest.raises with unittest.TestCase, mock.patch.object, and assertRaisesRegex. Guard the module-level addon import with a try/except that raises SkipTest so collection is quiet on environments without the GUI stack, and pin Gtk to 3.0 before gramps imports to avoid the GTK4 fallback crash on Gtk.IconSize.MENU. Co-Authored-By: Claude Opus 4.7 --- .../tests/test_calculate_estimated_dates.py | 328 ++++++++++-------- 1 file changed, 192 insertions(+), 136 deletions(-) diff --git a/CalculateEstimatedDates/tests/test_calculate_estimated_dates.py b/CalculateEstimatedDates/tests/test_calculate_estimated_dates.py index 8aa9a2f79..e318faeb7 100644 --- a/CalculateEstimatedDates/tests/test_calculate_estimated_dates.py +++ b/CalculateEstimatedDates/tests/test_calculate_estimated_dates.py @@ -19,10 +19,10 @@ # """ -Tests for the Calculate Estimated Dates tool — locks in the error +Tests for the Calculate Estimated Dates tool — lock in the error handling added in response to ``gramps-project/bugs#7898`` (ancestry -loops surfaced as ``DatabaseError`` from ``probably_alive_range`` -were tearing down the whole tool). +loops surfaced as ``DatabaseError`` from ``probably_alive_range`` were +tearing down the whole tool). The tool's ``__init__`` pulls in the full Gramps GUI stack, so these tests build stub instances via ``__new__`` and exercise the isolated @@ -34,65 +34,109 @@ # ------------------------ import os import sys +import unittest +from typing import Any +from unittest import mock + +# The addon imports Gtk at module load — skip cleanly if gi/Gtk are not +# available. On systems where both GTK3 and GTK4 are present, pin Gtk to +# 3.0 before any gramps import (mirrors what gramps.grampsapp does at +# startup); otherwise PyGObject loads GTK4 and the gramps.gui import +# chain crashes on Gtk.IconSize.MENU (a GTK3-only enum). +try: + import gi + + gi.require_version("Gtk", "3.0") + gi.require_version("Gdk", "3.0") +except (ImportError, ValueError, AttributeError) as err: + raise unittest.SkipTest("GTK 3.0 / PyGObject not available: %s" % err) -import pytest - -# Gramps is imported transitively by the module under test. -pytest.importorskip("gramps") - - -# --------------------------------------------------------------------------- -# Make the addon importable and ensure GRAMPS_RESOURCES is set -# --------------------------------------------------------------------------- +# ------------------------ +# Gramps modules +# ------------------------ +# addons-source/ goes on sys.path so ``from CalculateEstimatedDates import +# CalculateEstimatedDates`` resolves package→submodule. With the addon +# directory itself on sys.path instead, ``CalculateEstimatedDates`` binds +# to ``CalculateEstimatedDates.py`` directly, and the submodule lookup +# fails. ADDON_DIR is retained for the ``.gpr.py`` path in the registration +# test. ADDON_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -if ADDON_DIR not in sys.path: - sys.path.insert(0, ADDON_DIR) +ADDONS_SOURCE_DIR = os.path.dirname(ADDON_DIR) +if ADDONS_SOURCE_DIR not in sys.path: + sys.path.insert(0, ADDONS_SOURCE_DIR) -if "GRAMPS_RESOURCES" not in os.environ: +try: import gramps +except ImportError as err: + raise unittest.SkipTest("gramps package not available: %s" % err) +if "GRAMPS_RESOURCES" not in os.environ: os.environ["GRAMPS_RESOURCES"] = os.path.dirname( os.path.dirname(gramps.__file__) ) +from gramps.gen.errors import DatabaseError # noqa: E402 +from gramps.gen.lib import Date # noqa: E402 # ------------------------ -# Gramps modules +# Gramps specific # ------------------------ -from gramps.gen.errors import DatabaseError # noqa: E402 -from gramps.gen.lib import Date # noqa: E402 +# The addon module pulls in the full Gramps GUI stack at import time. On +# environments where GTK is missing or version-mismatched the import +# fails; skip the whole module cleanly in that case so collection does +# not surface spurious errors. +try: + from CalculateEstimatedDates import CalculateEstimatedDates as ced_module +except Exception as err: # noqa: BLE001 — environment guard + raise unittest.SkipTest( + "CalculateEstimatedDates module unavailable: %s" % err + ) -# --------------------------------------------------------------------------- -# Lazy module loader — the addon pulls in the full Gramps GUI stack at -# import time, so on environments where GTK is missing or version-mismatched -# the import fails. Importing inside a fixture lets pytest collection succeed -# (and skip cleanly per-test) instead of hanging the entire collection phase. -# --------------------------------------------------------------------------- -@pytest.fixture -def ced_module(): - try: - from CalculateEstimatedDates import CalculateEstimatedDates as mod - except Exception as err: # pragma: no cover - environment guard - pytest.skip(f"CalculateEstimatedDates module unavailable: {err}") - return mod - - -# --------------------------------------------------------------------------- -# Lightweight stubs — enough surface for the helper paths under test -# --------------------------------------------------------------------------- +# ------------------------------------------------------------ +# +# _FakeOptionsHandler +# +# ------------------------------------------------------------ class _FakeOptionsHandler: - def __init__(self, dates=0): - self.options_dict = {"dates": dates} + """Stub for ``CalcToolManagedWindow.options.handler`` — exposes only + ``options_dict``, which is all the paths under test read.""" + + def __init__(self, dates: int = 0) -> None: + """ + :param dates: Value stored under ``options_dict["dates"]``. + :type dates: int + """ + self.options_dict: dict[str, int] = {"dates": dates} +# ------------------------------------------------------------ +# +# _FakeOptions +# +# ------------------------------------------------------------ class _FakeOptions: - def __init__(self, dates=0): + """Stub for ``CalcToolManagedWindow.options`` with a ``handler`` attribute.""" + + def __init__(self, dates: int = 0) -> None: + """ + :param dates: Value propagated into the handler's ``options_dict``. + :type dates: int + """ self.handler = _FakeOptionsHandler(dates=dates) -def _make_tool(ced_module, *, dates=0): - """Return a CalcToolManagedWindow skipping ``__init__`` (no GTK required).""" +def _make_tool(dates: int = 0) -> Any: + """ + Build a ``CalcToolManagedWindow`` via ``__new__`` so its ``__init__`` + (which pulls in GTK) does not run, then attach just enough state for + the isolated helpers to execute. + + :param dates: Value stored on the fake options handler. + :type dates: int + :returns: A stub instance with the minimum attributes set. + :rtype: CalcToolManagedWindow + """ cls = ced_module.CalcToolManagedWindow stub = cls.__new__(cls) stub.options = _FakeOptions(dates=dates) @@ -103,104 +147,116 @@ def _make_tool(ced_module, *, dates=0): return stub -# --------------------------------------------------------------------------- -# get_modifier — pure logic, four branches -# --------------------------------------------------------------------------- -def test_get_modifier_birth_about_when_dates_zero(ced_module): - """dates=0 + birth → MOD_ABOUT (the 'approximate' case).""" - tool = _make_tool(ced_module, dates=0) - assert tool.get_modifier("birth") == Date.MOD_ABOUT - - -def test_get_modifier_birth_after_when_dates_nonzero(ced_module): - """dates=1 + birth → MOD_AFTER (the 'extremes' case).""" - tool = _make_tool(ced_module, dates=1) - assert tool.get_modifier("birth") == Date.MOD_AFTER - - -def test_get_modifier_death_about_when_dates_zero(ced_module): - """dates=0 + death → MOD_ABOUT.""" - tool = _make_tool(ced_module, dates=0) - assert tool.get_modifier("death") == Date.MOD_ABOUT - - -def test_get_modifier_death_before_when_dates_nonzero(ced_module): - """dates=1 + death → MOD_BEFORE (upper-bound estimate).""" - tool = _make_tool(ced_module, dates=1) - assert tool.get_modifier("death") == Date.MOD_BEFORE - - -# --------------------------------------------------------------------------- -# calc_estimates — bug 7898: DatabaseError must propagate so run() can catch -# it per-person instead of letting one loop kill the whole tool. -# --------------------------------------------------------------------------- -def test_calc_estimates_returns_probably_alive_range_result( - ced_module, monkeypatch -): - """Happy path — the helper is a pass-through to probably_alive_range.""" - tool = _make_tool(ced_module) - person = object() - expected = (Date(), Date(), "explain", None) - - calls = [] +# ------------------------------------------------------------ +# +# TestGetModifier +# +# ------------------------------------------------------------ +class TestGetModifier(unittest.TestCase): + """Pure-logic coverage for ``get_modifier`` across its four branches.""" - def _fake(person_arg, db_arg, max_sib, max_age, avg_gap): - calls.append((person_arg, db_arg, max_sib, max_age, avg_gap)) - return expected + def test_birth_about_when_dates_zero(self) -> None: + """dates=0 + birth → MOD_ABOUT (the 'approximate' case).""" + tool = _make_tool(dates=0) + self.assertEqual(tool.get_modifier("birth"), Date.MOD_ABOUT) - monkeypatch.setattr(ced_module, "probably_alive_range", _fake) + def test_birth_after_when_dates_nonzero(self) -> None: + """dates=1 + birth → MOD_AFTER (the 'extremes' case).""" + tool = _make_tool(dates=1) + self.assertEqual(tool.get_modifier("birth"), Date.MOD_AFTER) - result = tool.calc_estimates(person) + def test_death_about_when_dates_zero(self) -> None: + """dates=0 + death → MOD_ABOUT.""" + tool = _make_tool(dates=0) + self.assertEqual(tool.get_modifier("death"), Date.MOD_ABOUT) - assert result == expected - assert calls == [(person, tool.db, 20, 110, 20)] + def test_death_before_when_dates_nonzero(self) -> None: + """dates=1 + death → MOD_BEFORE (upper-bound estimate).""" + tool = _make_tool(dates=1) + self.assertEqual(tool.get_modifier("death"), Date.MOD_BEFORE) -def test_calc_estimates_propagates_database_error(ced_module, monkeypatch): - """ - When ``probably_alive_range`` raises ``DatabaseError`` (e.g. an - ancestry loop), ``calc_estimates`` must let it escape so the - per-person handler in ``run()`` can log and skip. - """ - tool = _make_tool(ced_module) - - def _boom(*_args, **_kwargs): - raise DatabaseError("loop in Test, Abel's descendants") - - monkeypatch.setattr(ced_module, "probably_alive_range", _boom) - - with pytest.raises(DatabaseError, match="loop"): - tool.calc_estimates(object()) - - -# --------------------------------------------------------------------------- -# Plugin registration — catch metadata breakage early -# --------------------------------------------------------------------------- -def test_gpr_registration_metadata(): - """The .gpr.py file must register a single TOOL with expected keys.""" - gpr_path = os.path.join(ADDON_DIR, "CalculateEstimatedDates.gpr.py") - calls: list[tuple[tuple, dict]] = [] - - namespace = { - "register": lambda *args, **kwargs: calls.append((args, kwargs)), - "TOOL": "TOOL", - "STABLE": "STABLE", - "UNSTABLE": "UNSTABLE", - "TOOL_DBPROC": "TOOL_DBPROC", - "TOOL_MODE_GUI": "TOOL_MODE_GUI", - "_": lambda s: s, - } - with open(gpr_path, encoding="utf-8") as handle: - exec(compile(handle.read(), gpr_path, "exec"), namespace) - - assert len(calls) == 1, "expected exactly one register() call" - args, kwargs = calls[0] - assert args == ("TOOL",) - assert kwargs["id"] == "calculateestimateddates" - assert kwargs["fname"] == "CalculateEstimatedDates.py" - assert kwargs["gramps_target_version"] == "6.0" - assert kwargs["status"] == "STABLE" - assert kwargs["toolclass"] == "CalcToolManagedWindow" - assert kwargs["optionclass"] == "CalcEstDateOptions" - assert kwargs["category"] == "TOOL_DBPROC" - assert kwargs["tool_modes"] == ["TOOL_MODE_GUI"] +# ------------------------------------------------------------ +# +# TestCalcEstimates +# +# ------------------------------------------------------------ +class TestCalcEstimates(unittest.TestCase): + """Regression coverage for bug 7898 — ``calc_estimates`` must let + ``DatabaseError`` from ``probably_alive_range`` escape so the + per-person handler in ``run()`` can log and skip instead of tearing + down the whole tool.""" + + def test_returns_probably_alive_range_result(self) -> None: + """Happy path — the helper is a pass-through to ``probably_alive_range``.""" + tool = _make_tool() + person = object() + expected = (Date(), Date(), "explain", None) + calls: list[tuple] = [] + + def _fake(person_arg: Any, db_arg: Any, max_sib: int, max_age: int, avg_gap: int) -> tuple: + calls.append((person_arg, db_arg, max_sib, max_age, avg_gap)) + return expected + + with mock.patch.object(ced_module, "probably_alive_range", _fake): + result = tool.calc_estimates(person) + + self.assertEqual(result, expected) + self.assertEqual(calls, [(person, tool.db, 20, 110, 20)]) + + def test_propagates_database_error(self) -> None: + """ + When ``probably_alive_range`` raises ``DatabaseError`` (e.g. an + ancestry loop), ``calc_estimates`` must let it escape so the + per-person handler in ``run()`` can log and skip. + """ + tool = _make_tool() + + def _boom(*_args: Any, **_kwargs: Any) -> Any: + raise DatabaseError("loop in Test, Abel's descendants") + + with mock.patch.object(ced_module, "probably_alive_range", _boom): + with self.assertRaisesRegex(DatabaseError, "loop"): + tool.calc_estimates(object()) + + +# ------------------------------------------------------------ +# +# TestGprRegistration +# +# ------------------------------------------------------------ +class TestGprRegistration(unittest.TestCase): + """Catch metadata breakage in the plugin registration file early.""" + + def test_gpr_registration_metadata(self) -> None: + """The .gpr.py file must register a single TOOL with expected keys.""" + gpr_path = os.path.join(ADDON_DIR, "CalculateEstimatedDates.gpr.py") + calls: list[tuple[tuple, dict]] = [] + + namespace: dict[str, Any] = { + "register": lambda *args, **kwargs: calls.append((args, kwargs)), + "TOOL": "TOOL", + "STABLE": "STABLE", + "UNSTABLE": "UNSTABLE", + "TOOL_DBPROC": "TOOL_DBPROC", + "TOOL_MODE_GUI": "TOOL_MODE_GUI", + "_": lambda s: s, + } + with open(gpr_path, encoding="utf-8") as handle: + exec(compile(handle.read(), gpr_path, "exec"), namespace) + + self.assertEqual(len(calls), 1, "expected exactly one register() call") + args, kwargs = calls[0] + self.assertEqual(args, ("TOOL",)) + self.assertEqual(kwargs["id"], "calculateestimateddates") + self.assertEqual(kwargs["fname"], "CalculateEstimatedDates.py") + self.assertEqual(kwargs["gramps_target_version"], "6.0") + self.assertEqual(kwargs["status"], "STABLE") + self.assertEqual(kwargs["toolclass"], "CalcToolManagedWindow") + self.assertEqual(kwargs["optionclass"], "CalcEstDateOptions") + self.assertEqual(kwargs["category"], "TOOL_DBPROC") + self.assertEqual(kwargs["tool_modes"], ["TOOL_MODE_GUI"]) + + +if __name__ == "__main__": + unittest.main() From c5a2a6304b8bb2c97867785e359a31c6ccc3802c Mon Sep 17 00:00:00 2001 From: GaryGriffin Date: Tue, 21 Apr 2026 09:36:34 -0700 Subject: [PATCH 10/21] Merge CalculateEstimatedDates: handle ancestry-loop DatabaseError (bug 0007898)#825 --- CalculateEstimatedDates/CalculateEstimatedDates.gpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CalculateEstimatedDates/CalculateEstimatedDates.gpr.py b/CalculateEstimatedDates/CalculateEstimatedDates.gpr.py index 1a86ecaeb..59cf33989 100644 --- a/CalculateEstimatedDates/CalculateEstimatedDates.gpr.py +++ b/CalculateEstimatedDates/CalculateEstimatedDates.gpr.py @@ -9,7 +9,7 @@ id="calculateestimateddates", name=_("Calculate Estimated Dates"), description=_("Calculates estimated dates for birth and death."), - version = '0.90.41', + version = '0.90.42', gramps_target_version="6.0", status=STABLE, # not yet tested with python 3 fname="CalculateEstimatedDates.py", From f8d0aeef384501d03f626441193a7205cd32ce5c Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Fri, 17 Apr 2026 23:14:33 +0200 Subject: [PATCH 11/21] ImportMerge: fix AttributeError when adding/merging Tag objects Tag is a table object without a gramps_id field, so the generic has__gramps_id / find_next__gramps_id lookups in do_commits raised AttributeError when the user selected Add on a Tag row. Guard both the S_ADD and S_DIFFERS GID-conflict blocks so they skip Tag. Adds integration tests covering both branches; verified they fail without the guard and pass with it. Fixes bug 0014056 Co-Authored-By: Claude Opus 4.7 --- ImportMerge/importmerge.py | 11 +- ImportMerge/tests/__init__.py | 0 .../tests/test_integration_importmerge.py | 173 ++++++++++++++++++ 3 files changed, 179 insertions(+), 5 deletions(-) create mode 100644 ImportMerge/tests/__init__.py create mode 100644 ImportMerge/tests/test_integration_importmerge.py diff --git a/ImportMerge/importmerge.py b/ImportMerge/importmerge.py index 2b49e8276..e789e2def 100644 --- a/ImportMerge/importmerge.py +++ b/ImportMerge/importmerge.py @@ -803,8 +803,8 @@ def do_commits(self, status, obj_type, hndl, action, trans): continue # now check the missing list for a match changed += self.check_miss(r_objtype, r_hndl, item) - # check for GID conflict - if item.gramps_id != item1.gramps_id: + # check for GID conflict (Tag is a table object without gramps_id) + if obj_type != 'Tag' and item.gramps_id != item1.gramps_id: if getattr(self.db1, 'has_' + obj_type.lower() + '_gramps_id')(item.gramps_id): item.gramps_id = getattr(self.db1, 'find_next_' + @@ -823,9 +823,10 @@ def do_commits(self, status, obj_type, hndl, action, trans): continue # now check the differences list for a match self.check_diffs(r_objtype, r_hndl, obj_type, item) - # check for GID conflict - if getattr(self.db1, 'has_' + obj_type.lower() + - '_gramps_id')(item.gramps_id): + # check for GID conflict (Tag is a table object without gramps_id) + if obj_type != 'Tag' and getattr( + self.db1, 'has_' + obj_type.lower() + + '_gramps_id')(item.gramps_id): item.gramps_id = getattr(self.db1, 'find_next_' + obj_type.lower() + '_gramps_id')() diff --git a/ImportMerge/tests/__init__.py b/ImportMerge/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ImportMerge/tests/test_integration_importmerge.py b/ImportMerge/tests/test_integration_importmerge.py new file mode 100644 index 000000000..af1eefc68 --- /dev/null +++ b/ImportMerge/tests/test_integration_importmerge.py @@ -0,0 +1,173 @@ +# +# Gramps - a GTK+/GNOME based genealogy program +# +# Copyright (C) 2026 Eduard Ralph +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# + +"""Integration tests for the Import and Merge tool. + +These tests exercise ``ImportMerge.do_commits`` against a real Gramps SQLite +database. We call the method unbound with a stub ``self`` so the GUI +(ManagedWindow + Gtk dialog) doesn't need to be instantiated — the data-path +logic is what we're verifying. + +Regression coverage: + +* Gramps bug 0014056 — Adding a Tag via the Import Merge tool raised + ``AttributeError: 'SQLite' object has no attribute 'has_tag_gramps_id'`` + because Tag is a table object and has no gramps_id. +""" + +# ------------------------ +# Python modules +# ------------------------ +import os +import shutil +import tempfile +import types +from types import SimpleNamespace + +import pytest + +# The ImportMerge module imports Gtk at module load — skip the whole file if +# gi/Gtk aren't available (headless-without-GTK environments). +pytest.importorskip("gi") + +# ------------------------ +# Gramps modules +# ------------------------ +from gramps.gen.db import DbTxn +from gramps.gen.db.utils import make_database +from gramps.gen.lib import Tag + +# ------------------------ +# Gramps specific +# ------------------------ +from ImportMerge.importmerge import ImportMerge, S_ADD, S_DIFFERS, A_ADD + + +def _make_db(suffix): + """Create a fresh on-disk Gramps SQLite DB inside a temp directory.""" + tmpdir = tempfile.mkdtemp(prefix=f"gramps_test_{suffix}_") + db_path = os.path.join(tmpdir, f"db_{suffix}") + os.makedirs(db_path) + db = make_database("sqlite") + db.load(db_path, None) + return db, tmpdir + + +@pytest.fixture +def db1(): + """Destination Gramps DB (the one being merged into).""" + db, tmpdir = _make_db("db1") + yield db + try: + db.close() + except Exception: + pass + shutil.rmtree(tmpdir, ignore_errors=True) + + +@pytest.fixture +def db2(): + """Source Gramps DB (simulates the XML being imported).""" + db, tmpdir = _make_db("db2") + yield db + try: + db.close() + except Exception: + pass + shutil.rmtree(tmpdir, ignore_errors=True) + + +def _make_stub(db1, db2): + """Stub ``self`` for ``ImportMerge.do_commits`` — no GUI attributes.""" + return SimpleNamespace( + db1=db1, + db2=db2, + added={}, + missing={}, + diffs={}, + ) + + +def _add_tag(db, name): + """Add a Tag to ``db`` and return its handle.""" + tag = Tag() + tag.set_name(name) + with DbTxn("add tag", db, batch=True) as trans: + handle = db.add_tag(tag, trans) + return handle + + +def test_add_tag_does_not_crash(db1, db2): + """Regression for bug 0014056 — adding a Tag via do_commits must succeed. + + Before the fix, this raised ``AttributeError: 'SQLite' object has no + attribute 'has_tag_gramps_id'`` because the generic GID-conflict check + didn't special-case Tag (a table object without a gramps_id field). + """ + tag_handle = _add_tag(db2, "Imported") + stub = _make_stub(db1, db2) + + with DbTxn("import merge", db1, batch=True) as trans: + ImportMerge.do_commits(stub, S_ADD, "Tag", tag_handle, A_ADD, trans) + + # Tag should now exist in the destination DB. + assert db1.get_number_of_tags() == 1 + committed = db1.get_tag_from_handle(tag_handle) + assert committed is not None + assert committed.get_name() == "Imported" + + +def test_differing_tag_does_not_crash(db1, db2): + """S_DIFFERS branch must also skip the gramps_id check for Tag. + + Same root cause as the S_ADD path: the GID-conflict block at the end of + the S_DIFFERS branch dereferences ``item.gramps_id`` and calls + ``has_tag_gramps_id`` — both fail for Tag objects. + """ + tag_handle = _add_tag(db1, "Original") + # Same handle in db2 with a different name — simulates S_DIFFERS. + tag2 = Tag() + tag2.set_handle(tag_handle) + tag2.set_name("Modified") + with DbTxn("seed db2", db2, batch=True) as trans: + db2.add_tag(tag2, trans) + + stub = _make_stub(db1, db2) + + # diff_result is invoked by do_commits on the S_DIFFERS path; stub it to + # return the db2 version as the resolved item (action = A_ADD ⇒ "replace"). + def fake_diff_result(action, obj_type, hndl): + item1 = db1.get_tag_from_handle(hndl) + item2 = db2.get_tag_from_handle(hndl) + return item1, item2, item2 + + stub.diff_result = fake_diff_result + stub.check_diffs = lambda *a, **kw: False + stub.check_added = lambda *a, **kw: False + stub.check_miss = lambda *a, **kw: False + + with DbTxn("import merge", db1, batch=True) as trans: + ImportMerge.do_commits( + stub, S_DIFFERS, "Tag", tag_handle, A_ADD, trans + ) + + committed = db1.get_tag_from_handle(tag_handle) + assert committed is not None + assert committed.get_name() == "Modified" From 21ee942e74851d6d3a0f5fa2991a7a9cbad8a65c Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 04:47:44 +0200 Subject: [PATCH 12/21] ImportMerge: pin Gtk to 3.0 in integration test On systems with both GTK3 and GTK4 installed, PyGObject defaults to GTK4, and importing ImportMerge pulls in gramps.gui which crashes on Gtk.IconSize.MENU (a GTK3-only enum). Mirror the pin that gramps.grampsapp performs at startup so reviewers can run the test without environment tweaks. --- ImportMerge/tests/test_integration_importmerge.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ImportMerge/tests/test_integration_importmerge.py b/ImportMerge/tests/test_integration_importmerge.py index af1eefc68..3c5409765 100644 --- a/ImportMerge/tests/test_integration_importmerge.py +++ b/ImportMerge/tests/test_integration_importmerge.py @@ -44,8 +44,21 @@ import pytest # The ImportMerge module imports Gtk at module load — skip the whole file if -# gi/Gtk aren't available (headless-without-GTK environments). +# gi/Gtk aren't available (headless-without-GTK environments). On systems +# where both GTK3 and GTK4 are present, pin Gtk to 3.0 before any gramps +# import (mirrors what gramps.grampsapp does at startup); otherwise +# PyGObject loads GTK4 and the gramps.gui import chain crashes on +# Gtk.IconSize.MENU (a GTK3-only enum). pytest.importorskip("gi") +import gi # noqa: E402 + +try: + gi.require_version("Gtk", "3.0") + gi.require_version("Gdk", "3.0") +except (ValueError, AttributeError): + pytest.skip( + "GTK 3.0 typelib not available", allow_module_level=True + ) # ------------------------ # Gramps modules From 4ff3bb1c2fbc43effa79496d2d93b1d03a0c5cdb Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 13:58:24 +0200 Subject: [PATCH 13/21] ImportMerge: rewrite integration test with unittest framework AGENTS.md requires the unittest framework for tests. Convert the pytest-based integration test (fixtures + assert statements) to a unittest.TestCase with setUp/tearDown and self.assert* calls. GTK availability is now checked with a module-level try/except raising unittest.SkipTest, which also makes the separate GTK-pin step redundant (still applied here before the gramps import). --- .../tests/test_integration_importmerge.py | 191 +++++++++--------- 1 file changed, 90 insertions(+), 101 deletions(-) diff --git a/ImportMerge/tests/test_integration_importmerge.py b/ImportMerge/tests/test_integration_importmerge.py index 3c5409765..04d7b82e7 100644 --- a/ImportMerge/tests/test_integration_importmerge.py +++ b/ImportMerge/tests/test_integration_importmerge.py @@ -38,26 +38,23 @@ import os import shutil import tempfile -import types +import unittest from types import SimpleNamespace -import pytest - # The ImportMerge module imports Gtk at module load — skip the whole file if # gi/Gtk aren't available (headless-without-GTK environments). On systems # where both GTK3 and GTK4 are present, pin Gtk to 3.0 before any gramps # import (mirrors what gramps.grampsapp does at startup); otherwise # PyGObject loads GTK4 and the gramps.gui import chain crashes on # Gtk.IconSize.MENU (a GTK3-only enum). -pytest.importorskip("gi") -import gi # noqa: E402 - try: + import gi + gi.require_version("Gtk", "3.0") gi.require_version("Gdk", "3.0") -except (ValueError, AttributeError): - pytest.skip( - "GTK 3.0 typelib not available", allow_module_level=True +except (ImportError, ValueError, AttributeError) as err: + raise unittest.SkipTest( + "GTK 3.0 / PyGObject not available: %s" % err ) # ------------------------ @@ -75,49 +72,14 @@ def _make_db(suffix): """Create a fresh on-disk Gramps SQLite DB inside a temp directory.""" - tmpdir = tempfile.mkdtemp(prefix=f"gramps_test_{suffix}_") - db_path = os.path.join(tmpdir, f"db_{suffix}") + tmpdir = tempfile.mkdtemp(prefix="gramps_test_%s_" % suffix) + db_path = os.path.join(tmpdir, "db_%s" % suffix) os.makedirs(db_path) db = make_database("sqlite") db.load(db_path, None) return db, tmpdir -@pytest.fixture -def db1(): - """Destination Gramps DB (the one being merged into).""" - db, tmpdir = _make_db("db1") - yield db - try: - db.close() - except Exception: - pass - shutil.rmtree(tmpdir, ignore_errors=True) - - -@pytest.fixture -def db2(): - """Source Gramps DB (simulates the XML being imported).""" - db, tmpdir = _make_db("db2") - yield db - try: - db.close() - except Exception: - pass - shutil.rmtree(tmpdir, ignore_errors=True) - - -def _make_stub(db1, db2): - """Stub ``self`` for ``ImportMerge.do_commits`` — no GUI attributes.""" - return SimpleNamespace( - db1=db1, - db2=db2, - added={}, - missing={}, - diffs={}, - ) - - def _add_tag(db, name): """Add a Tag to ``db`` and return its handle.""" tag = Tag() @@ -127,60 +89,87 @@ def _add_tag(db, name): return handle -def test_add_tag_does_not_crash(db1, db2): - """Regression for bug 0014056 — adding a Tag via do_commits must succeed. - - Before the fix, this raised ``AttributeError: 'SQLite' object has no - attribute 'has_tag_gramps_id'`` because the generic GID-conflict check - didn't special-case Tag (a table object without a gramps_id field). - """ - tag_handle = _add_tag(db2, "Imported") - stub = _make_stub(db1, db2) - - with DbTxn("import merge", db1, batch=True) as trans: - ImportMerge.do_commits(stub, S_ADD, "Tag", tag_handle, A_ADD, trans) - - # Tag should now exist in the destination DB. - assert db1.get_number_of_tags() == 1 - committed = db1.get_tag_from_handle(tag_handle) - assert committed is not None - assert committed.get_name() == "Imported" - - -def test_differing_tag_does_not_crash(db1, db2): - """S_DIFFERS branch must also skip the gramps_id check for Tag. - - Same root cause as the S_ADD path: the GID-conflict block at the end of - the S_DIFFERS branch dereferences ``item.gramps_id`` and calls - ``has_tag_gramps_id`` — both fail for Tag objects. - """ - tag_handle = _add_tag(db1, "Original") - # Same handle in db2 with a different name — simulates S_DIFFERS. - tag2 = Tag() - tag2.set_handle(tag_handle) - tag2.set_name("Modified") - with DbTxn("seed db2", db2, batch=True) as trans: - db2.add_tag(tag2, trans) - - stub = _make_stub(db1, db2) - - # diff_result is invoked by do_commits on the S_DIFFERS path; stub it to - # return the db2 version as the resolved item (action = A_ADD ⇒ "replace"). - def fake_diff_result(action, obj_type, hndl): - item1 = db1.get_tag_from_handle(hndl) - item2 = db2.get_tag_from_handle(hndl) - return item1, item2, item2 - - stub.diff_result = fake_diff_result - stub.check_diffs = lambda *a, **kw: False - stub.check_added = lambda *a, **kw: False - stub.check_miss = lambda *a, **kw: False - - with DbTxn("import merge", db1, batch=True) as trans: - ImportMerge.do_commits( - stub, S_DIFFERS, "Tag", tag_handle, A_ADD, trans +class ImportMergeTagTestCase(unittest.TestCase): + """Regression tests for bug 0014056 (Tag handling in do_commits).""" + + def setUp(self): + self.db1, self.tmp1 = _make_db("db1") + self.db2, self.tmp2 = _make_db("db2") + + def tearDown(self): + for db in (self.db1, self.db2): + try: + db.close() + except Exception: + pass + shutil.rmtree(self.tmp1, ignore_errors=True) + shutil.rmtree(self.tmp2, ignore_errors=True) + + def _make_stub(self): + """Stub ``self`` for ``ImportMerge.do_commits`` — no GUI attributes.""" + return SimpleNamespace( + db1=self.db1, + db2=self.db2, + added={}, + missing={}, + diffs={}, ) - committed = db1.get_tag_from_handle(tag_handle) - assert committed is not None - assert committed.get_name() == "Modified" + def test_add_tag_does_not_crash(self): + """Regression for bug 0014056 — adding a Tag via do_commits must succeed. + + Before the fix, this raised ``AttributeError: 'SQLite' object has no + attribute 'has_tag_gramps_id'`` because the generic GID-conflict check + didn't special-case Tag (a table object without a gramps_id field). + """ + tag_handle = _add_tag(self.db2, "Imported") + stub = self._make_stub() + + with DbTxn("import merge", self.db1, batch=True) as trans: + ImportMerge.do_commits( + stub, S_ADD, "Tag", tag_handle, A_ADD, trans + ) + + self.assertEqual(self.db1.get_number_of_tags(), 1) + committed = self.db1.get_tag_from_handle(tag_handle) + self.assertIsNotNone(committed) + self.assertEqual(committed.get_name(), "Imported") + + def test_differing_tag_does_not_crash(self): + """S_DIFFERS branch must also skip the gramps_id check for Tag. + + Same root cause as the S_ADD path: the GID-conflict block at the end + of the S_DIFFERS branch dereferences ``item.gramps_id`` and calls + ``has_tag_gramps_id`` — both fail for Tag objects. + """ + tag_handle = _add_tag(self.db1, "Original") + tag2 = Tag() + tag2.set_handle(tag_handle) + tag2.set_name("Modified") + with DbTxn("seed db2", self.db2, batch=True) as trans: + self.db2.add_tag(tag2, trans) + + stub = self._make_stub() + + def fake_diff_result(action, obj_type, hndl): + item1 = self.db1.get_tag_from_handle(hndl) + item2 = self.db2.get_tag_from_handle(hndl) + return item1, item2, item2 + + stub.diff_result = fake_diff_result + stub.check_diffs = lambda *a, **kw: False + stub.check_added = lambda *a, **kw: False + stub.check_miss = lambda *a, **kw: False + + with DbTxn("import merge", self.db1, batch=True) as trans: + ImportMerge.do_commits( + stub, S_DIFFERS, "Tag", tag_handle, A_ADD, trans + ) + + committed = self.db1.get_tag_from_handle(tag_handle) + self.assertIsNotNone(committed) + self.assertEqual(committed.get_name(), "Modified") + + +if __name__ == "__main__": + unittest.main() From a56b4a8cb65521c19ad13d44efee047bdd95cfd3 Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 14:13:27 +0200 Subject: [PATCH 14/21] ImportMerge: apply Black formatting to integration test Addresses AGENTS.md rule requiring Black-formatted Python. Collapses three multi-line function calls that fit on a single line. --- ImportMerge/tests/test_integration_importmerge.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/ImportMerge/tests/test_integration_importmerge.py b/ImportMerge/tests/test_integration_importmerge.py index 04d7b82e7..17c771610 100644 --- a/ImportMerge/tests/test_integration_importmerge.py +++ b/ImportMerge/tests/test_integration_importmerge.py @@ -53,9 +53,7 @@ gi.require_version("Gtk", "3.0") gi.require_version("Gdk", "3.0") except (ImportError, ValueError, AttributeError) as err: - raise unittest.SkipTest( - "GTK 3.0 / PyGObject not available: %s" % err - ) + raise unittest.SkipTest("GTK 3.0 / PyGObject not available: %s" % err) # ------------------------ # Gramps modules @@ -126,9 +124,7 @@ def test_add_tag_does_not_crash(self): stub = self._make_stub() with DbTxn("import merge", self.db1, batch=True) as trans: - ImportMerge.do_commits( - stub, S_ADD, "Tag", tag_handle, A_ADD, trans - ) + ImportMerge.do_commits(stub, S_ADD, "Tag", tag_handle, A_ADD, trans) self.assertEqual(self.db1.get_number_of_tags(), 1) committed = self.db1.get_tag_from_handle(tag_handle) @@ -162,9 +158,7 @@ def fake_diff_result(action, obj_type, hndl): stub.check_miss = lambda *a, **kw: False with DbTxn("import merge", self.db1, batch=True) as trans: - ImportMerge.do_commits( - stub, S_DIFFERS, "Tag", tag_handle, A_ADD, trans - ) + ImportMerge.do_commits(stub, S_DIFFERS, "Tag", tag_handle, A_ADD, trans) committed = self.db1.get_tag_from_handle(tag_handle) self.assertIsNotNone(committed) From 52433a97d6ac5ddd92980b3d8b1b8ad7690c552f Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 14:14:17 +0200 Subject: [PATCH 15/21] ImportMerge: add type hints and class header to integration test Addresses AGENTS.md requirements: - Type hints on all helpers and test methods using Python 3.10+ syntax (``X | None``, ``tuple[X, Y]``). - Sphinx-style ``:param:`` / ``:returns:`` docstring markers on the helper functions. - ``# ------`` class-header divider above the TestCase so it's easy to locate. No behavioural change. --- .../tests/test_integration_importmerge.py | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/ImportMerge/tests/test_integration_importmerge.py b/ImportMerge/tests/test_integration_importmerge.py index 17c771610..26b006dff 100644 --- a/ImportMerge/tests/test_integration_importmerge.py +++ b/ImportMerge/tests/test_integration_importmerge.py @@ -59,6 +59,7 @@ # Gramps modules # ------------------------ from gramps.gen.db import DbTxn +from gramps.gen.db.base import DbReadBase from gramps.gen.db.utils import make_database from gramps.gen.lib import Tag @@ -68,8 +69,13 @@ from ImportMerge.importmerge import ImportMerge, S_ADD, S_DIFFERS, A_ADD -def _make_db(suffix): - """Create a fresh on-disk Gramps SQLite DB inside a temp directory.""" +def _make_db(suffix: str) -> tuple[DbReadBase, str]: + """Create a fresh on-disk Gramps SQLite DB inside a temp directory. + + :param suffix: Short label used in the temp-directory prefix. + :returns: A ``(db, tmpdir)`` tuple — the caller owns ``tmpdir`` and must + remove it. + """ tmpdir = tempfile.mkdtemp(prefix="gramps_test_%s_" % suffix) db_path = os.path.join(tmpdir, "db_%s" % suffix) os.makedirs(db_path) @@ -78,8 +84,13 @@ def _make_db(suffix): return db, tmpdir -def _add_tag(db, name): - """Add a Tag to ``db`` and return its handle.""" +def _add_tag(db: DbReadBase, name: str) -> str: + """Add a Tag to ``db`` and return its handle. + + :param db: The database to add the tag to. + :param name: The tag name. + :returns: The handle of the newly-created tag. + """ tag = Tag() tag.set_name(name) with DbTxn("add tag", db, batch=True) as trans: @@ -87,14 +98,19 @@ def _add_tag(db, name): return handle +# ------------------------------------------------------------ +# +# ImportMergeTagTestCase +# +# ------------------------------------------------------------ class ImportMergeTagTestCase(unittest.TestCase): """Regression tests for bug 0014056 (Tag handling in do_commits).""" - def setUp(self): + def setUp(self) -> None: self.db1, self.tmp1 = _make_db("db1") self.db2, self.tmp2 = _make_db("db2") - def tearDown(self): + def tearDown(self) -> None: for db in (self.db1, self.db2): try: db.close() @@ -103,7 +119,7 @@ def tearDown(self): shutil.rmtree(self.tmp1, ignore_errors=True) shutil.rmtree(self.tmp2, ignore_errors=True) - def _make_stub(self): + def _make_stub(self) -> SimpleNamespace: """Stub ``self`` for ``ImportMerge.do_commits`` — no GUI attributes.""" return SimpleNamespace( db1=self.db1, @@ -113,7 +129,7 @@ def _make_stub(self): diffs={}, ) - def test_add_tag_does_not_crash(self): + def test_add_tag_does_not_crash(self) -> None: """Regression for bug 0014056 — adding a Tag via do_commits must succeed. Before the fix, this raised ``AttributeError: 'SQLite' object has no @@ -131,7 +147,7 @@ def test_add_tag_does_not_crash(self): self.assertIsNotNone(committed) self.assertEqual(committed.get_name(), "Imported") - def test_differing_tag_does_not_crash(self): + def test_differing_tag_does_not_crash(self) -> None: """S_DIFFERS branch must also skip the gramps_id check for Tag. Same root cause as the S_ADD path: the GID-conflict block at the end @@ -147,7 +163,9 @@ def test_differing_tag_does_not_crash(self): stub = self._make_stub() - def fake_diff_result(action, obj_type, hndl): + def fake_diff_result( + action: int, obj_type: str, hndl: str + ) -> tuple[Tag | None, Tag | None, Tag | None]: item1 = self.db1.get_tag_from_handle(hndl) item2 = self.db2.get_tag_from_handle(hndl) return item1, item2, item2 From 72d7634013dd499d2c4f987a1b35436479370b4f Mon Sep 17 00:00:00 2001 From: GaryGriffin Date: Tue, 21 Apr 2026 09:40:57 -0700 Subject: [PATCH 16/21] Merge ImportMerge: fix AttributeError when adding/merging Tag objects (bug 0014056) #826 --- ImportMerge/importmerge.gpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ImportMerge/importmerge.gpr.py b/ImportMerge/importmerge.gpr.py index f4be5dad9..f707a0581 100644 --- a/ImportMerge/importmerge.gpr.py +++ b/ImportMerge/importmerge.gpr.py @@ -32,7 +32,7 @@ description=_( "Compares a Gramps XML database with the current one, and allows merging of the differences." ), - version = '0.0.25', + version = '0.0.26', gramps_target_version="6.0", status=STABLE, fname="importmerge.py", From 4dbdd9b5399031750e1ed5eac74785ac567c00df Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Sat, 18 Apr 2026 00:52:59 +0200 Subject: [PATCH 17/21] Form: fix crash and surface clear errors for malformed XML (bug 0011707) A family section whose title lacked the expected 'X/Y' separator caused a ValueError: not enough values to unpack when the form editor opened, crashing the Forms gramplet. The underlying issue was that the addon trusted the XML definitions and had no schema validation or user-facing error reporting for broken files. Split the validation out of form.py into a pure-Python form_validator module (no GTK/Gramps imports) so it can be unit-tested without a GUI. The Form loader now: * parses each file defensively (ExpatError -> ErrorDialog), * runs the validator before loading (invalid files -> ErrorDialog with the file path, offending form id, and the rule that failed), * skips any
element that fails validation while still loading sibling well-formed forms from the same file. split_family_title() in form_validator belt-and-braces the FamilySection constructor so a missing separator no longer raises, even if validation is bypassed. Also adds diagnostic logging: * INFO log of forms loaded per file, * DEBUG trace of each file parsed and each form id loaded/skipped, * DEBUG when EditForm opens (event/citation handles), * WARNING in FamilySection if its title lacks 'X/Y'. Tests: * Form/tests/test_form_validator.py -- 32 pure-Python unit tests, covers split_family_title, every validation branch, parse_and_validate file handling, and a sanity check that every shipped form_*.xml passes validation. * Form/tests/test_integration_form.py -- unittest integration tests that patch ErrorDialog to verify the loader surfaces syntax errors, invalid family titles, missing role, and invalid section types; partially broken files still load their valid forms; shipped files trigger no dialogs. Partially addresses bug 0011010 (request for user error dialog for unsupported elements) by covering its core ask: clear errors for invalid section types, missing/empty role, missing/empty type, and XML syntax errors. Fixes #11707. Refs #11010. Co-Authored-By: Claude Opus 4.7 --- Form/editform.py | 30 ++- Form/form.py | 118 +++++++-- Form/form_validator.py | 169 ++++++++++++ Form/po/template.pot | 12 + Form/tests/test_form_validator.py | 385 ++++++++++++++++++++++++++++ Form/tests/test_integration_form.py | 258 +++++++++++++++++++ 6 files changed, 946 insertions(+), 26 deletions(-) create mode 100644 Form/form_validator.py create mode 100644 Form/tests/test_form_validator.py create mode 100644 Form/tests/test_integration_form.py diff --git a/Form/editform.py b/Form/editform.py index 910cd827c..7e43fdd16 100644 --- a/Form/editform.py +++ b/Form/editform.py @@ -28,8 +28,11 @@ # Python modules # ------------------------------------------------------------------------- from gi.repository import Gdk +import logging import pickle +LOG = logging.getLogger(".FormGramplet") + # ------------------------------------------------------------------------ # # GTK modules @@ -77,6 +80,7 @@ get_section_columns, get_form_citation, ) +from form_validator import split_family_title from entrygrid import EntryGrid # ------------------------------------------------------------------------ @@ -114,6 +118,12 @@ def __init__(self, dbstate, uistate, track, event, citation, callback): self.citation = citation self.callback = callback + LOG.debug( + "Opening EditForm for event %s, citation %s", + event.get_handle() or "", + citation.get_handle() or "", + ) + ManagedWindow.__init__(self, uistate, track, citation) self.widgets = {} @@ -391,10 +401,10 @@ def close(self, *args): """ Close the editor window. """ - (width, height) = self.window.get_size() + width, height = self.window.get_size() self._config.set("interface.form-width", width) self._config.set("interface.form-height", height) - (width, height) = self.window.get_position() + width, height = self.window.get_position() self._config.set("interface.form-horiz-position", width) self._config.set("interface.form-vert-position", height) self._config.save() @@ -700,7 +710,7 @@ def on_drag_data_received( self, widget, context, pos_x, pos_y, sel_data, info, time ): if sel_data and sel_data.get_data(): - (drag_type, idval, handle, val) = pickle.loads(sel_data.get_data()) + drag_type, idval, handle, val = pickle.loads(sel_data.get_data()) person = self.db.get_person_from_handle(handle) if person: self.__person_added(person) @@ -972,7 +982,7 @@ def on_drag_data_received( self, widget, context, pos_x, pos_y, sel_data, info, time ): if sel_data and sel_data.get_data(): - (drag_type, idval, handle, val) = pickle.loads(sel_data.get_data()) + drag_type, idval, handle, val = pickle.loads(sel_data.get_data()) person = self.db.get_person_from_handle(handle) if person: self.__added(person) @@ -1102,7 +1112,15 @@ def __init__(self, dbstate, uistate, track, event, citation, form_id, section): hbox = Gtk.Box() title = get_section_title(form_id, section) - title1, title2 = title.split("/") + title1, title2 = split_family_title(title) + if not title2: + LOG.warning( + "FamilySection for form '%s' section '%s' has title '%s' " + "without the expected 'X/Y' separator; second label will be empty", + form_id, + section, + title, + ) label = Gtk.Label(label="%s" % title1) label.set_use_markup(True) @@ -1158,7 +1176,7 @@ def on_drag_data_received( self, widget, context, pos_x, pos_y, sel_data, info, time ): if sel_data and sel_data.get_data(): - (drag_type, idval, handle, val) = pickle.loads(sel_data.get_data()) + drag_type, idval, handle, val = pickle.loads(sel_data.get_data()) family = self.db.get_family_from_handle(handle) if family: self.__added(family) diff --git a/Form/form.py b/Form/form.py index a2a4c0cf3..e9d00af3f 100644 --- a/Form/form.py +++ b/Form/form.py @@ -22,6 +22,7 @@ """ Form definitions. """ + # --------------------------------------------------------------- # # Python imports @@ -29,6 +30,8 @@ # --------------------------------------------------------------- import os import xml.dom.minidom +import xml.parsers.expat +import logging # --------------------------------------------------------------- # @@ -37,8 +40,14 @@ # --------------------------------------------------------------- from gramps.gen.datehandler import parser from gramps.gen.config import config -from gramps.gui.dialog import ErrorDialog, WarningDialog -import logging +from gramps.gui.dialog import ErrorDialog + +# --------------------------------------------------------------- +# +# Gramps specific +# +# --------------------------------------------------------------- +from form_validator import validate_form_dom, validate_form_element LOG = logging.getLogger(".FormGramplet") @@ -111,7 +120,14 @@ class Form: A class to read form definitions from an XML file. """ - def __init__(self): + def __init__(self, definition_dir=None): + """ + :param definition_dir: optional override for the directory the + loader scans for ``form_*.xml`` / ``custom.xml`` files. + Defaults to the directory containing this module. Exposed + primarily so tests can point the loader at an isolated + temporary directory. + """ self.__references = {} self.__dates = {} self.__headings = {} @@ -122,27 +138,88 @@ def __init__(self): self.__names = {} self.__section_types = {} + base_dir = definition_dir or os.path.dirname(__file__) + LOG.debug("Loading form definitions from %s", base_dir) for file_name in definition_files: - full_path = os.path.join(os.path.dirname(__file__), file_name) + full_path = os.path.join(base_dir, file_name) if os.path.exists(full_path): - try: - self.__load_definitions(full_path) - except Exception as e: - WarningDialog( - _("Failed to load Form definition file:\n%s\n") % full_path, - ) - LOG.warning( - "\nERROR: failed to load Form definition file.\n%s\nException:\n%s", - full_path, - str(e), - ) - - def __load_definitions(self, definition_file): - dom = xml.dom.minidom.parse(definition_file) + self.__load_file(full_path) + else: + LOG.debug("Form definition file not present: %s", full_path) + LOG.info( + "Loaded %d form definition(s) from %s", + len(self.__names), + base_dir, + ) + + def __load_file(self, full_path): + """ + Parse and validate a single form definition file, then load any + well-formed ```` elements it contains. + + Parse errors and structural validation errors are reported to the + user through an :class:`ErrorDialog` and logged; malformed forms + are skipped while valid forms from the same file are still loaded. + """ + LOG.debug("Parsing form definition file %s", full_path) + try: + dom = xml.dom.minidom.parse(full_path) + except xml.parsers.expat.ExpatError as exc: + ErrorDialog( + _("XML syntax error in Form definition file"), + "%s\n\n%s" % (full_path, exc), + ) + LOG.warning( + "XML syntax error in Form definition file %s: %s", + full_path, + exc, + ) + return + except Exception as exc: + ErrorDialog( + _("Failed to read Form definition file"), + "%s\n\n%s" % (full_path, exc), + ) + LOG.warning("Failed to read Form definition file %s: %s", full_path, exc) + return + + errors = validate_form_dom(dom) + if errors: + ErrorDialog( + _("Invalid Form definition file"), + "%s\n\n%s" % (full_path, "\n".join(errors)), + ) + LOG.warning( + "Invalid Form definition file %s:\n%s", + full_path, + "\n".join(errors), + ) + + try: + self.__load_definitions(dom) + finally: + dom.unlink() + + def __load_definitions(self, dom): top = dom.getElementsByTagName("forms") + if not top: + return for form in top[0].getElementsByTagName("form"): + if validate_form_element(form): + # Errors for this form were already surfaced by the + # file-level validator — skip it so __load_definitions + # never touches a malformed element. + skipped_id = ( + form.attributes["id"].value + if "id" in form.attributes + else "" + ) + LOG.debug("Skipping invalid '%s'", skipped_id) + continue + id = form.attributes["id"].value + LOG.debug("Loading form '%s'", id) self.__names[id] = form.attributes["title"].value self.__types[id] = form.attributes["type"].value if "reference" in form.attributes: @@ -194,7 +271,6 @@ def __load_definitions(self, definition_file): self.__columns[id][role].append( (attr_text, long_text, int(size_text)) ) - dom.unlink() def get_form_ids(self): """Return a list of ids for all form definitions.""" @@ -205,7 +281,7 @@ def get_title(self, form_id): return self.__names[form_id] def get_reference(self, form_id): - """ Return the reference for a given form. """ + """Return the reference for a given form.""" return self.__references[form_id] def get_date(self, form_id): @@ -258,12 +334,14 @@ def get_form_title(form_id): """ return FORM.get_title(form_id) + def get_form_reference(form_id): """ Return the reference for a given form. """ return FORM.get_reference(form_id) + def get_form_date(form_id): """ Return the date for a given form. diff --git a/Form/form_validator.py b/Form/form_validator.py new file mode 100644 index 000000000..4614026d8 --- /dev/null +++ b/Form/form_validator.py @@ -0,0 +1,169 @@ +# +# Gramps - a GTK+/GNOME based genealogy program +# +# Copyright (C) 2026 Eduard Ralph +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# + +""" +Pure-Python validation for Form addon XML definition files. + +Kept free of GTK/Gramps imports so it can be unit-tested without a GUI +environment. +""" + +# ------------------------ +# Python modules +# ------------------------ +import xml.dom.minidom +import xml.parsers.expat + +VALID_SECTION_TYPES = frozenset({"person", "family", "multi"}) +REQUIRED_FORM_ATTRS = ("id", "title", "type") + + +def split_family_title(title: str) -> tuple[str, str]: + """ + Split a family-section title of the form ``'X/Y'`` into ``(X, Y)``. + + Falls back gracefully when the separator is absent or the input is + empty, so callers never raise ``ValueError`` on malformed XML. + + :param title: the raw title string from the XML (may be empty) + :returns: a two-tuple of title parts; the second element is empty + when the title contains no ``'/'`` + """ + if not title: + return "", "" + parts = title.split("/", 1) + if len(parts) == 2: + return parts[0], parts[1] + return parts[0], "" + + +def validate_form_element(form) -> list[str]: + """ + Validate a single ```` DOM element against the form schema. + + :param form: a DOM element for a single form definition + :returns: a list of human-readable error messages scoped to this + form; empty when the form is valid + """ + errors: list[str] = [] + + form_id = form.attributes["id"].value if "id" in form.attributes else "" + for required in REQUIRED_FORM_ATTRS: + if required not in form.attributes: + errors.append( + "Form '%s': missing required attribute '%s'" % (form_id, required) + ) + + for section in form.getElementsByTagName("section"): + role = section.attributes["role"].value if "role" in section.attributes else "" + if not role: + errors.append( + "Form '%s':
is missing required attribute 'role'" % form_id + ) + continue + + if "type" not in section.attributes: + errors.append( + "Form '%s': section '%s' is missing required attribute 'type'" + % (form_id, role) + ) + continue + + section_type = section.attributes["type"].value + if not section_type: + errors.append( + "Form '%s': section '%s' has an empty 'type' attribute" + % (form_id, role) + ) + continue + + if section_type not in VALID_SECTION_TYPES: + errors.append( + "Form '%s': section '%s' has invalid type '%s' " + "(expected one of: %s)" + % ( + form_id, + role, + section_type, + ", ".join(sorted(VALID_SECTION_TYPES)), + ) + ) + continue + + title = ( + section.attributes["title"].value if "title" in section.attributes else "" + ) + if section_type == "family": + parts = title.split("/") + if len(parts) != 2 or not parts[0].strip() or not parts[1].strip(): + errors.append( + "Form '%s': family section '%s' requires a title " + "of the form 'Name1/Name2' (got '%s')" % (form_id, role, title) + ) + + return errors + + +def validate_form_dom(dom: xml.dom.minidom.Document) -> list[str]: + """ + Validate the structure of a parsed form definitions DOM. + + Checks that: + + * a ```` root element exists; + * each ```` element has ``id``, ``title`` and ``type`` attributes; + * each ``
`` element has non-empty ``role`` and ``type`` + attributes; + * each section's ``type`` is one of ``person``, ``family`` or ``multi``; + * ``family``-type sections declare a title of the form ``'X/Y'`` with + two non-empty parts. + + :param dom: a parsed ``xml.dom.minidom.Document`` + :returns: a list of human-readable error messages; empty when the + document is valid + """ + top = dom.getElementsByTagName("forms") + if not top: + return ["Missing root element"] + + errors: list[str] = [] + for form in top[0].getElementsByTagName("form"): + errors.extend(validate_form_element(form)) + return errors + + +def parse_and_validate(path: str) -> tuple[xml.dom.minidom.Document | None, list[str]]: + """ + Parse ``path`` as XML and validate it against the form schema. + + :param path: filesystem path to a form definitions XML file + :returns: a ``(dom, errors)`` tuple. When parsing fails, ``dom`` is + ``None`` and ``errors`` contains a single description of + the syntax error. When parsing succeeds, ``dom`` is the + parsed document and ``errors`` lists any structural + problems (empty when the file is valid). + """ + try: + dom = xml.dom.minidom.parse(path) + except xml.parsers.expat.ExpatError as exc: + return None, ["XML syntax error: %s" % exc] + except (OSError, ValueError) as exc: + return None, ["Failed to read file: %s" % exc] + return dom, validate_form_dom(dom) diff --git a/Form/po/template.pot b/Form/po/template.pot index 91af5203a..9662fd535 100644 --- a/Form/po/template.pot +++ b/Form/po/template.pot @@ -9045,3 +9045,15 @@ msgstr "" #: Form/form_fr.xml.h:144 msgid "Arrived since" msgstr "" + +#: Form/form.py:154 +msgid "XML syntax error in Form definition file" +msgstr "" + +#: Form/form.py:165 +msgid "Failed to read Form definition file" +msgstr "" + +#: Form/form.py:176 +msgid "Invalid Form definition file" +msgstr "" diff --git a/Form/tests/test_form_validator.py b/Form/tests/test_form_validator.py new file mode 100644 index 000000000..c65536a35 --- /dev/null +++ b/Form/tests/test_form_validator.py @@ -0,0 +1,385 @@ +# +# Gramps - a GTK+/GNOME based genealogy program +# +# Copyright (C) 2026 Eduard Ralph +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# + +""" +Unit tests for ``form_validator`` — the pure-Python validation layer +for Form addon XML definition files. + +These tests do not touch Gramps or GTK, so they run in every CI job. + +Run with:: + + python3 -m unittest Form.tests.test_form_validator -v +""" + +# ------------------------ +# Python modules +# ------------------------ +import os +import sys +import tempfile +import textwrap +import unittest +import xml.dom.minidom + +# ------------------------ +# Gramps specific +# ------------------------ +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from form_validator import ( + parse_and_validate, + split_family_title, + validate_form_dom, + VALID_SECTION_TYPES, +) + + +def _dom_from_string(xml_text: str) -> xml.dom.minidom.Document: + """Parse an XML string into a DOM for validator testing.""" + return xml.dom.minidom.parseString(xml_text) + + +# --------------------------------------------------------------------------- +# split_family_title — belt-and-braces helper used by FamilySection +# --------------------------------------------------------------------------- +class TestSplitFamilyTitle(unittest.TestCase): + """ + Regression coverage for Gramps bug 11707 — ``FamilySection`` used to + crash with ``ValueError: not enough values to unpack (expected 2, + got 1)`` when a form's ``
`` title did not + contain a ``/`` separator. + """ + + def test_two_parts(self): + self.assertEqual(split_family_title("Groom/Bride"), ("Groom", "Bride")) + + def test_no_separator_returns_empty_second(self): + self.assertEqual(split_family_title("Couple"), ("Couple", "")) + + def test_empty_string_returns_two_empties(self): + self.assertEqual(split_family_title(""), ("", "")) + + def test_only_separator(self): + self.assertEqual(split_family_title("/"), ("", "")) + + def test_leading_separator(self): + self.assertEqual(split_family_title("/Bride"), ("", "Bride")) + + def test_trailing_separator(self): + self.assertEqual(split_family_title("Groom/"), ("Groom", "")) + + def test_multiple_separators_only_split_once(self): + self.assertEqual(split_family_title("A/B/C"), ("A", "B/C")) + + def test_whitespace_preserved(self): + self.assertEqual( + split_family_title(" Groom / Bride "), + (" Groom ", " Bride "), + ) + + +# --------------------------------------------------------------------------- +# validate_form_dom — happy path +# --------------------------------------------------------------------------- +class TestValidateFormDomValid(unittest.TestCase): + + def test_minimal_valid_form(self): + dom = _dom_from_string(textwrap.dedent("""\ + + +
+ + + """)) + self.assertEqual(validate_form_dom(dom), []) + + def test_valid_family_section_with_slashed_title(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + self.assertEqual(validate_form_dom(dom), []) + + def test_valid_person_section_without_title(self): + """Person sections may legitimately omit the title attribute.""" + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + self.assertEqual(validate_form_dom(dom), []) + + def test_empty_forms_element_is_valid(self): + dom = _dom_from_string("") + self.assertEqual(validate_form_dom(dom), []) + + +# --------------------------------------------------------------------------- +# validate_form_dom — structural errors +# --------------------------------------------------------------------------- +class TestValidateFormDomErrors(unittest.TestCase): + + def test_missing_forms_root(self): + dom = _dom_from_string("") + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + self.assertIn("", errors[0]) + + def test_form_missing_id_attribute(self): + dom = _dom_from_string("
") + errors = validate_form_dom(dom) + self.assertTrue(any("missing required attribute 'id'" in e for e in errors)) + + def test_form_missing_title_attribute(self): + dom = _dom_from_string("") + errors = validate_form_dom(dom) + self.assertTrue(any("missing required attribute 'title'" in e for e in errors)) + + def test_form_missing_type_attribute(self): + dom = _dom_from_string("") + errors = validate_form_dom(dom) + self.assertTrue(any("missing required attribute 'type'" in e for e in errors)) + + def test_section_missing_role_attribute(self): + dom = _dom_from_string(textwrap.dedent("""\ + + +
+ + + """)) + errors = validate_form_dom(dom) + self.assertTrue(any("'role'" in e for e in errors)) + + def test_section_empty_role_attribute(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertTrue(any("'role'" in e for e in errors)) + + def test_section_missing_type_attribute(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertTrue(any("'type'" in e for e in errors)) + + def test_section_empty_type_attribute(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertTrue(any("'type'" in e for e in errors)) + + def test_section_invalid_type(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + self.assertIn("invalid type 'bogus'", errors[0]) + for valid in VALID_SECTION_TYPES: + self.assertIn(valid, errors[0]) + + def test_section_type_is_case_sensitive(self): + """ + A real custom.xml in the wild used ``type='Person'`` instead of + ``type='person'``. The validator rejects it so the user sees a + clear error rather than silently loading a broken form. + """ + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + self.assertIn("invalid type 'Person'", errors[0]) + + def test_family_section_without_title_is_rejected(self): + """ + Reproduction of bug 11707 at the validation layer: a family + section without a slashed title produces a clear error instead + of a ValueError crash in the GUI. + """ + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + self.assertIn("Name1/Name2", errors[0]) + + def test_family_section_with_single_part_title_is_rejected(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + self.assertIn("Couple", errors[0]) + + def test_family_section_with_blank_part_is_rejected(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + + def test_family_section_with_three_parts_is_rejected(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ + + """)) + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + + def test_multiple_errors_aggregated(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+
+ +
+ + """)) + errors = validate_form_dom(dom) + # 1 for invalid type + 1 for missing role + 1 for missing id + self.assertGreaterEqual(len(errors), 3) + + +# --------------------------------------------------------------------------- +# parse_and_validate — file-level entry point +# --------------------------------------------------------------------------- +class TestParseAndValidate(unittest.TestCase): + + def _write(self, content: str) -> str: + """Write a temporary XML file and return its path.""" + fd, path = tempfile.mkstemp(suffix=".xml") + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(content) + self.addCleanup(os.remove, path) + return path + + def test_valid_file(self): + path = self._write(textwrap.dedent("""\ + + +
+ + + """)) + dom, errors = parse_and_validate(path) + self.assertIsNotNone(dom) + self.assertEqual(errors, []) + + def test_xml_syntax_error_reports_cleanly(self): + path = self._write("
") + dom, errors = parse_and_validate(path) + self.assertIsNone(dom) + self.assertEqual(len(errors), 1) + self.assertIn("XML syntax error", errors[0]) + + def test_missing_file_reports_cleanly(self): + dom, errors = parse_and_validate("/nonexistent/path.xml") + self.assertIsNone(dom) + self.assertEqual(len(errors), 1) + + def test_invalid_structure_returns_errors_and_dom(self): + path = self._write(textwrap.dedent("""\ + + +
+ + + """)) + dom, errors = parse_and_validate(path) + self.assertIsNotNone(dom) + self.assertEqual(len(errors), 1) + + +# --------------------------------------------------------------------------- +# Sanity: the shipped built-in definition files must validate +# --------------------------------------------------------------------------- +class TestShippedDefinitionFilesValidate(unittest.TestCase): + """ + Every built-in ``form_*.xml`` file shipped with the addon must pass + validation. If this test fails, the addon would refuse to load one + of its own definition files for end users. + """ + + FORM_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + def test_all_builtin_form_files_validate(self): + import glob + + files = sorted(glob.glob(os.path.join(self.FORM_DIR, "form_*.xml"))) + self.assertGreater(len(files), 0, "no form_*.xml files discovered") + for path in files: + with self.subTest(form_file=os.path.basename(path)): + dom, errors = parse_and_validate(path) + self.assertIsNotNone(dom, f"failed to parse {path}") + self.assertEqual( + errors, + [], + f"validation errors in {path}:\n" + "\n".join(errors), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/Form/tests/test_integration_form.py b/Form/tests/test_integration_form.py new file mode 100644 index 000000000..46a0822ba --- /dev/null +++ b/Form/tests/test_integration_form.py @@ -0,0 +1,258 @@ +# +# Gramps - a GTK+/GNOME based genealogy program +# +# Copyright (C) 2026 Eduard Ralph +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# + +""" +Integration tests for the Form addon loader — covers +``gramps-project/gramps#11707`` (*ValueError: not enough values to +unpack* when a form's ``
`` title lacks the +``X/Y`` separator). + +Scenarios covered: + +* Malformed XML produces an ``ErrorDialog`` rather than a bare traceback. +* A partially-broken file still loads its well-formed ``
`` entries. +* The shipped built-in definition files load cleanly without any error + dialogs being raised. + +Run with:: + + python3 -m unittest Form.tests.test_integration_form -v +""" + +# ------------------------ +# Python modules +# ------------------------ +import os +import shutil +import sys +import tempfile +import textwrap +import unittest +from unittest.mock import patch + +# ------------------------ +# Gramps modules +# ------------------------ +try: + import gi # noqa: F401 + import gramps +except ImportError as exc: + raise unittest.SkipTest( + "Form integration tests require 'gi' and 'gramps': %s" % exc + ) + +if "GRAMPS_RESOURCES" not in os.environ: + os.environ["GRAMPS_RESOURCES"] = os.path.dirname(os.path.dirname(gramps.__file__)) + +# ------------------------ +# Gramps specific +# ------------------------ +ADDON_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if ADDON_DIR not in sys.path: + sys.path.insert(0, ADDON_DIR) + + +# --------------------------------------------------------------------------- +# Shared harness +# --------------------------------------------------------------------------- +class FormLoaderTestCase(unittest.TestCase): + """ + Base class that imports the Form addon's ``form`` module and + redirects its ``ErrorDialog`` to an in-memory list instead of + opening a GTK dialog. + + Subclasses access :attr:`shown` to inspect captured dialog calls + and use :meth:`_write` to drop XML files into an isolated + temporary directory. + """ + + def setUp(self) -> None: + import form + + self.form = form + self.shown: list[tuple[str, str]] = [] + + def _fake_error_dialog(*args, **kwargs): + title = str(args[0]) if args else "" + body = str(args[1]) if len(args) > 1 else "" + self.shown.append((title, body)) + + error_patch = patch.object(form, "ErrorDialog", _fake_error_dialog) + error_patch.start() + self.addCleanup(error_patch.stop) + + self.tmp_dir = tempfile.mkdtemp(prefix="form-integration-") + self.addCleanup(shutil.rmtree, self.tmp_dir, True) + + def _write(self, filename: str, content: str) -> None: + """Write an XML fixture into the test's temporary directory.""" + path = os.path.join(self.tmp_dir, filename) + with open(path, "w", encoding="utf-8") as handle: + handle.write(content) + + def _patch_definition_files(self, files: list[str]) -> None: + """Redirect the loader to the given filenames (inside tmp_dir).""" + files_patch = patch.object(self.form, "definition_files", files) + files_patch.start() + self.addCleanup(files_patch.stop) + + +# --------------------------------------------------------------------------- +# Error-dialog wiring +# --------------------------------------------------------------------------- +class TestErrorDialogWiring(FormLoaderTestCase): + """ + Exercises the four failure modes the loader now surfaces via + :class:`ErrorDialog` instead of an unhandled traceback. + """ + + def test_malformed_xml_shows_error_dialog(self) -> None: + """XML syntax errors should raise an ErrorDialog, not a traceback.""" + self._write("custom.xml", "") + self._patch_definition_files(["custom.xml"]) + + instance = self.form.Form(definition_dir=self.tmp_dir) + + self.assertTrue(self.shown, "no ErrorDialog was displayed") + title, body = self.shown[0] + self.assertIn("XML syntax error", title) + self.assertIn("custom.xml", body) + self.assertEqual(list(instance.get_form_ids()), []) + + def test_invalid_family_title_shows_error_dialog(self) -> None: + """ + The exact condition from bug 11707 — a family section with a + non-``X/Y`` title — must be surfaced as an ErrorDialog at load + time rather than an unhandled exception when the user later + opens the form. + """ + self._write( + "custom.xml", + textwrap.dedent("""\ + + +
+ + + """), + ) + self._patch_definition_files(["custom.xml"]) + + self.form.Form(definition_dir=self.tmp_dir) + + self.assertTrue( + self.shown, "no ErrorDialog was displayed for invalid family title" + ) + title, body = self.shown[0] + self.assertIn("Invalid Form definition file", title) + self.assertIn("Name1/Name2", body) + + def test_partially_broken_file_still_loads_valid_forms(self) -> None: + """A broken
must not stop sibling elements loading.""" + self._write( + "custom.xml", + textwrap.dedent("""\ + + +
+ +
+
+ + + """), + ) + self._patch_definition_files(["custom.xml"]) + + instance = self.form.Form(definition_dir=self.tmp_dir) + loaded_ids = list(instance.get_form_ids()) + + self.assertIn("GOOD", loaded_ids, "valid form should still load") + self.assertNotIn("BAD", loaded_ids, "invalid form should be skipped") + self.assertTrue(self.shown, "the broken form should have been reported") + + def test_missing_role_attribute_shows_error_dialog(self) -> None: + """A section missing its ``role`` attribute is reported clearly.""" + self._write( + "custom.xml", + textwrap.dedent("""\ + +
+
+ + + """), + ) + self._patch_definition_files(["custom.xml"]) + + self.form.Form(definition_dir=self.tmp_dir) + + self.assertTrue(self.shown) + _, body = self.shown[0] + self.assertIn("role", body) + + def test_invalid_section_type_shows_error_dialog(self) -> None: + """Unknown section types produce a clear error, not a later crash.""" + self._write( + "custom.xml", + textwrap.dedent("""\ + +
+
+ + + """), + ) + self._patch_definition_files(["custom.xml"]) + + instance = self.form.Form(definition_dir=self.tmp_dir) + + self.assertTrue(self.shown) + _, body = self.shown[0] + self.assertIn("bogus", body) + self.assertNotIn("F1", list(instance.get_form_ids())) + + +# --------------------------------------------------------------------------- +# Shipped files load cleanly +# --------------------------------------------------------------------------- +class TestShippedFilesLoadCleanly(FormLoaderTestCase): + """ + The built-in definition files that ship with the addon must load + without triggering a single ErrorDialog, otherwise end users would + see a popup every time they opened Gramps. + """ + + def test_shipped_files_load_without_errors(self) -> None: + instance = self.form.Form() + + self.assertFalse( + self.shown, + "Built-in definition files triggered ErrorDialog calls:\n" + + "\n".join("%s: %s" % (t, b) for t, b in self.shown), + ) + self.assertTrue( + list(instance.get_form_ids()), + "no forms loaded from built-in definition files", + ) + + +if __name__ == "__main__": + unittest.main() From 9528414ebe8c8a92f6a20286546135970a32ad25 Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Tue, 21 Apr 2026 18:49:46 +0200 Subject: [PATCH 18/21] Form: detect empty definition files and warn on column-size mismatches (bug 0011010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A form definition file whose root contained no
elements used to load silently — the loader iterated zero children and returned, leaving the user with no feedback. validate_form_dom now reports this as an error so the ErrorDialog wiring added for bug 11707 surfaces the problem on load. get_form_warnings is a new non-fatal check for sections whose values do not sum to 100. 78 shipped definition files violate this rule today without breaking rendering (the size field is parsed but never read by the layout code), so treating it as an error would flag correctly-working forms. The loader logs the warnings via LOG.warning instead, making authoring mistakes in user-authored custom.xml files diagnosable without harassing users of the built-in forms. Unit coverage: rejected empty , forms-root-with-only-comments, and every branch of the sized-column check (no columns, no sizes, summing to 100, summing to other totals, partial sizing, independent of errors, multiple sections). Integration coverage: empty-forms triggers ErrorDialog; size mismatch is logged but does not block the form from loading. Tests use stdlib unittest to match Gramps' own conventions. --- Form/form.py | 9 +- Form/form_validator.py | 73 +++++++++++++- Form/tests/test_form_validator.py | 143 +++++++++++++++++++++++++++- Form/tests/test_integration_form.py | 76 ++++++++++++++- 4 files changed, 292 insertions(+), 9 deletions(-) diff --git a/Form/form.py b/Form/form.py index e9d00af3f..81b99352f 100644 --- a/Form/form.py +++ b/Form/form.py @@ -47,7 +47,11 @@ # Gramps specific # # --------------------------------------------------------------- -from form_validator import validate_form_dom, validate_form_element +from form_validator import ( + get_form_warnings, + validate_form_dom, + validate_form_element, +) LOG = logging.getLogger(".FormGramplet") @@ -195,6 +199,9 @@ def __load_file(self, full_path): "\n".join(errors), ) + for warning in get_form_warnings(dom): + LOG.warning("In %s: %s", full_path, warning) + try: self.__load_definitions(dom) finally: diff --git a/Form/form_validator.py b/Form/form_validator.py index 4614026d8..5b82cd98c 100644 --- a/Form/form_validator.py +++ b/Form/form_validator.py @@ -127,7 +127,8 @@ def validate_form_dom(dom: xml.dom.minidom.Document) -> list[str]: Checks that: - * a ```` root element exists; + * a ```` root element exists and contains at least one + ```` definition; * each ```` element has ``id``, ``title`` and ``type`` attributes; * each ``
`` element has non-empty ``role`` and ``type`` attributes; @@ -144,11 +145,79 @@ def validate_form_dom(dom: xml.dom.minidom.Document) -> list[str]: return ["Missing root element"] errors: list[str] = [] - for form in top[0].getElementsByTagName("form"): + forms = top[0].getElementsByTagName("form") + if not forms: + errors.append(" root element contains no definitions") + for form in forms: errors.extend(validate_form_element(form)) return errors +def get_form_warnings(dom: xml.dom.minidom.Document) -> list[str]: + """ + Collect non-fatal warnings about a parsed form definitions DOM. + + Warnings describe likely authoring mistakes that do not prevent the + form from loading. Currently covers Gramps bug 11010's observation + that a section's ```` ```` values are expected to sum + to 100 — sections that declare explicit sizes on every column but do + not sum to 100 are reported as warnings so callers can log them + without blocking the form from loading. + + Sections without any sized columns, or with only some columns + sized, are skipped because the intent is ambiguous. + + :param dom: a parsed ``xml.dom.minidom.Document`` + :returns: a list of human-readable warning messages; empty when + nothing questionable is detected + """ + warnings: list[str] = [] + top = dom.getElementsByTagName("forms") + if not top: + return warnings + + for form in top[0].getElementsByTagName("form"): + form_id = ( + form.attributes["id"].value + if "id" in form.attributes + else "" + ) + for section in form.getElementsByTagName("section"): + role = ( + section.attributes["role"].value + if "role" in section.attributes + else "" + ) + columns = section.getElementsByTagName("column") + if not columns: + continue + + sizes: list[int] = [] + all_sized = True + for column in columns: + size_nodes = column.getElementsByTagName("size") + if not size_nodes or not size_nodes[0].childNodes: + all_sized = False + break + try: + sizes.append(int(size_nodes[0].childNodes[0].data)) + except ValueError: + all_sized = False + break + if not all_sized: + continue + + total = sum(sizes) + if total != 100: + warnings.append( + "Form '%s': section '%s' column sizes sum to %d " + "(expected 100); form will still load but column " + "widths may not render as intended" + % (form_id, role, total) + ) + return warnings + + def parse_and_validate(path: str) -> tuple[xml.dom.minidom.Document | None, list[str]]: """ Parse ``path`` as XML and validate it against the form schema. diff --git a/Form/tests/test_form_validator.py b/Form/tests/test_form_validator.py index c65536a35..0efc350eb 100644 --- a/Form/tests/test_form_validator.py +++ b/Form/tests/test_form_validator.py @@ -44,6 +44,7 @@ # ------------------------ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from form_validator import ( + get_form_warnings, parse_and_validate, split_family_title, validate_form_dom, @@ -131,11 +132,6 @@ def test_valid_person_section_without_title(self): """)) self.assertEqual(validate_form_dom(dom), []) - def test_empty_forms_element_is_valid(self): - dom = _dom_from_string("") - self.assertEqual(validate_form_dom(dom), []) - - # --------------------------------------------------------------------------- # validate_form_dom — structural errors # --------------------------------------------------------------------------- @@ -147,6 +143,23 @@ def test_missing_forms_root(self): self.assertEqual(len(errors), 1) self.assertIn("", errors[0]) + def test_empty_forms_element_is_rejected(self): + """ + Regression for Gramps bug 11010 — a file whose ```` root + contains no ```` definitions used to load silently, leaving + users with no feedback. The validator now reports it as an error. + """ + dom = _dom_from_string("") + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + self.assertIn("no ", errors[0]) + + def test_forms_root_with_only_comments_is_rejected(self): + dom = _dom_from_string("") + errors = validate_form_dom(dom) + self.assertEqual(len(errors), 1) + self.assertIn("no ", errors[0]) + def test_form_missing_id_attribute(self): dom = _dom_from_string("") errors = validate_form_dom(dom) @@ -381,5 +394,125 @@ def test_all_builtin_form_files_validate(self): ) +# --------------------------------------------------------------------------- +# get_form_warnings — non-fatal authoring warnings (Gramps bug 11010) +# --------------------------------------------------------------------------- +class TestGetFormWarnings(unittest.TestCase): + """ + Column sizes are expected to sum to 100. Rather than reject the + form, ``get_form_warnings`` flags suspect sections so the caller can + log them — 78 shipped definition files currently violate this rule + without breaking rendering, so escalating to an error would be + user-hostile. + """ + + def test_section_without_columns_has_no_warning(self): + dom = _dom_from_string(textwrap.dedent("""\ + + +
+ + + """)) + self.assertEqual(get_form_warnings(dom), []) + + def test_columns_without_any_size_have_no_warning(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ <_attribute>Name + <_attribute>Age +
+
+ + """)) + self.assertEqual(get_form_warnings(dom), []) + + def test_columns_summing_to_100_have_no_warning(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ <_attribute>A60 + <_attribute>B40 +
+
+ + """)) + self.assertEqual(get_form_warnings(dom), []) + + def test_columns_not_summing_to_100_emit_warning(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ <_attribute>A25 +
+
+ + """)) + warnings = get_form_warnings(dom) + self.assertEqual(len(warnings), 1) + self.assertIn("sum to 25", warnings[0]) + self.assertIn("F1", warnings[0]) + self.assertIn("Primary", warnings[0]) + + def test_partially_sized_columns_have_no_warning(self): + """ + When only some columns declare a ````, the author's intent + is ambiguous (mixed relative/absolute sizing) so the check is + skipped to avoid false positives. + """ + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ <_attribute>A25 + <_attribute>B +
+
+ + """)) + self.assertEqual(get_form_warnings(dom), []) + + def test_warnings_are_independent_of_errors(self): + """ + Warnings are structurally orthogonal to errors: a malformed form + still produces warnings for its well-formed sibling. + """ + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ +
+
+ <_attribute>A30 +
+
+ + """)) + warnings = get_form_warnings(dom) + self.assertEqual(len(warnings), 1) + self.assertIn("GOOD", warnings[0]) + + def test_multiple_misaligned_sections_all_reported(self): + dom = _dom_from_string(textwrap.dedent("""\ + +
+
+ <_attribute>X40 +
+
+ <_attribute>Y70 +
+
+ + """)) + warnings = get_form_warnings(dom) + self.assertEqual(len(warnings), 2) + + if __name__ == "__main__": unittest.main() diff --git a/Form/tests/test_integration_form.py b/Form/tests/test_integration_form.py index 46a0822ba..82839c0cf 100644 --- a/Form/tests/test_integration_form.py +++ b/Form/tests/test_integration_form.py @@ -22,12 +22,16 @@ Integration tests for the Form addon loader — covers ``gramps-project/gramps#11707`` (*ValueError: not enough values to unpack* when a form's ``
`` title lacks the -``X/Y`` separator). +``X/Y`` separator) and ``gramps-project/gramps#11010`` (empty +```` roots used to load silently, and column-size mismatches +had no diagnostic). Scenarios covered: * Malformed XML produces an ``ErrorDialog`` rather than a bare traceback. * A partially-broken file still loads its well-formed ``
`` entries. +* Empty ```` roots surface as an ``ErrorDialog`` (bug 11010 item a). +* Column-size mismatches are logged as WARNINGs only (bug 11010 item b). * The shipped built-in definition files load cleanly without any error dialogs being raised. @@ -39,6 +43,7 @@ # ------------------------ # Python modules # ------------------------ +import logging import os import shutil import sys @@ -230,6 +235,75 @@ def test_invalid_section_type_shows_error_dialog(self) -> None: self.assertNotIn("F1", list(instance.get_form_ids())) +# --------------------------------------------------------------------------- +# Empty / content-less definition files (Gramps bug 11010 item a) +# --------------------------------------------------------------------------- +class TestEmptyFormsValidation(FormLoaderTestCase): + """ + A definition file whose ```` root contains no ```` + children used to load silently. The validator now surfaces it as an + ErrorDialog so the user notices the empty file. + """ + + def test_empty_forms_element_shows_error_dialog(self) -> None: + """Empty ```` must raise an ErrorDialog, not load silently.""" + self._write("custom.xml", "") + self._patch_definition_files(["custom.xml"]) + + instance = self.form.Form(definition_dir=self.tmp_dir) + + self.assertTrue(self.shown, "no ErrorDialog was displayed for empty ") + title, body = self.shown[0] + self.assertIn("Invalid Form definition file", title) + self.assertIn("no ", body) + self.assertEqual(list(instance.get_form_ids()), []) + + +# --------------------------------------------------------------------------- +# Column-size warnings (Gramps bug 11010 item b) — WARNING only, not errors +# --------------------------------------------------------------------------- +class TestColumnSizeWarnings(FormLoaderTestCase): + """ + Sections whose ```` sizes do not sum to 100 must be logged + as warnings only — 78 shipped forms trip this check, so escalating + to an ErrorDialog would harass users on every launch. + """ + + def test_column_size_sum_warning_is_logged_not_dialog(self) -> None: + """Column-size mismatch → WARNING log entry, no ErrorDialog.""" + self._write( + "custom.xml", + textwrap.dedent("""\ + + +
+ <_attribute>A25 +
+ + + """), + ) + self._patch_definition_files(["custom.xml"]) + + with self.assertLogs(".FormGramplet", level=logging.WARNING) as log_ctx: + instance = self.form.Form(definition_dir=self.tmp_dir) + + self.assertFalse( + self.shown, + "column-size mismatch must not produce an ErrorDialog:\n" + + "\n".join("%s: %s" % (t, b) for t, b in self.shown), + ) + self.assertIn( + "F1", + list(instance.get_form_ids()), + "the form must still load despite the size mismatch", + ) + self.assertTrue( + any("sum to 25" in message for message in log_ctx.output), + "expected a column-size warning in the log", + ) + + # --------------------------------------------------------------------------- # Shipped files load cleanly # --------------------------------------------------------------------------- From d0bdef65aff616a719365cd2b3f8094f77588ff8 Mon Sep 17 00:00:00 2001 From: GaryGriffin Date: Tue, 21 Apr 2026 10:03:18 -0700 Subject: [PATCH 19/21] Merge Form: #821 and (bug 11010)#822 to 6.1 branch --- Form/CensusCheckQuickview.gpr.py | 4 ++-- Form/formgramplet.gpr.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Form/CensusCheckQuickview.gpr.py b/Form/CensusCheckQuickview.gpr.py index 81ac06e22..9f3cc2b52 100644 --- a/Form/CensusCheckQuickview.gpr.py +++ b/Form/CensusCheckQuickview.gpr.py @@ -8,7 +8,7 @@ id = 'censuscheckquickview', name = _("CensusCheck"), description= _("Check whether any Census events are missing for a person and some of their descendents"), - version = '1.0.1', + version = '1.0.2', gramps_target_version = '6.0', status = STABLE, fname = 'CensusCheckQuickview.py', @@ -22,7 +22,7 @@ id = 'censuscheckupquickview', name = _("CensusCheckUp"), description= _("Check whether any Census events are missing for a person and some of their ancestors"), - version = '1.0.1', + version = '1.0.2', gramps_target_version = '6.0', status = STABLE, fname = 'CensusCheckUpQuickview.py', diff --git a/Form/formgramplet.gpr.py b/Form/formgramplet.gpr.py index bcdfed128..55d6312d0 100644 --- a/Form/formgramplet.gpr.py +++ b/Form/formgramplet.gpr.py @@ -31,7 +31,7 @@ name=_("Form Gramplet"), description=_("Gramplet interface for Forms"), status=STABLE, - version = '2.0.51', + version = '2.0.52', gramps_target_version="6.0", navtypes=["Person"], fname="formgramplet.py", From 6a21db4c62be18f1dcd701fbead9d99e5087801f Mon Sep 17 00:00:00 2001 From: Eduard Ralph Date: Wed, 6 May 2026 22:46:32 +0200 Subject: [PATCH 20/21] WebSearch: fix bare imports in test_filetable for dotted-path loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WebSearch/tests/test_filetable.py` imports `models`, `constants` and `db_file_table` without a package prefix. Those resolve only when `WebSearch/` itself is on sys.path — i.e. when the test is loaded via `unittest discover` from inside `tests/`. Under the dotted-path form that addons-source's own ci.yml uses (`python3 -m unittest WebSearch.tests.test_filetable` from the addons-source root), the imports look for a top-level `models` module and the test fails to load: ImportError: Failed to import test module: test_filetable ModuleNotFoundError: No module named 'models' Add the same `sys.path.insert(0, …parent dir…)` prologue that TMGimporter and Form already use for the same pattern, so the test loads under either form. No behavioural change beyond the imports. Co-Authored-By: Claude Opus 4.7 (1M context) --- WebSearch/tests/test_filetable.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/WebSearch/tests/test_filetable.py b/WebSearch/tests/test_filetable.py index d88d05666..909a1f4fe 100644 --- a/WebSearch/tests/test_filetable.py +++ b/WebSearch/tests/test_filetable.py @@ -37,8 +37,18 @@ Each test ensures that the file table behaves correctly according to its configuration. """ -import unittest import os +import sys +import unittest + +# Make sure addon modules are importable from the parent directory +# (matches the convention used by TMGimporter/Form tests). Required when +# the test is loaded via its dotted path — e.g. +# `python3 -m unittest WebSearch.tests.test_filetable` from +# addons-source/, the form used by addons-source's own ci.yml — where +# `models`/`constants`/`db_file_table` are otherwise resolved against +# the addons-source root rather than against `WebSearch/`. +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from models import DBFileTableConfig from constants import DuplicateHandlingMode, DB_FILE_TABLE_DIR From 6ce592709a37542dec8096c6b08aaa186fb760fe Mon Sep 17 00:00:00 2001 From: GaryGriffin Date: Fri, 8 May 2026 08:56:09 -0700 Subject: [PATCH 21/21] Merge WebSearch: fix bare imports in test_filetable for dotted-path loading #833 --- WebSearch/WebSearch.gpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WebSearch/WebSearch.gpr.py b/WebSearch/WebSearch.gpr.py index 3c9e12de9..968e254b0 100644 --- a/WebSearch/WebSearch.gpr.py +++ b/WebSearch/WebSearch.gpr.py @@ -37,7 +37,7 @@ "Person, Place, Family, or Source record" ), status=STABLE, - version = '1.10.8', + version = '1.10.9', fname="WebSearch.py", height=20, detached_width=400,