test: shared test harness + plugin-registration tests#980
Merged
GaryGriffin merged 4 commits intoJul 9, 2026
Merged
Conversation
Member
|
Tried |
Member
|
Reran and the output was much more understandable. Great progress - this was sorta what I was looking for when we started - a way to test the addons from my local machine. Something weird on the Gexiv2. I see the following but only one generated error. The former passes, the latter fails. Is there a syntax issue if there is a comma string in the versions? My current error log is below. Do you get the same as I get running locally? I want to set my expectations for Mac usability. Did this actually load the requires_mod/requires_gi packages? |
…py runner Adds a repo-root test suite for addons, runnable via `make.py <series> test`: - tests/gramps_test_env.py — GrampsTestCase / GrampsDbTestCase base classes that boot the real Gramps plugin manager and registry once per process. - tests/test_plugin_registration.py — registers every addon through Gramps' PluginRegister, validates id/name/version and the target Gramps series, loads each addon in a crash-isolated subprocess, and smoke-tests import/export entry points. - tests/test_plugin_load_gate.py — regression test that the load check gates (self.fail) on a genuine load failure rather than only logging it. - tests/test_addon_dependency_loading.py — exercises Gramps' depends_on mechanism over the tree: every declared dependency resolves to a registered plugin, the dependency graph is acyclic, and each declared dependency is load-bearing — proven by an isolated subprocess import so sys.modules caching can't mask a missing/undeclared dependency. Verified against Gramps 6.0 and 6.1. - make.py — new `test` command that runs the suite via unittest discovery, gated on GRAMPSPATH like the other commands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-ups from repeated adversarial review of the suite: - Load test: display/GTK/environment failures are classified consistently (anchored signatures, unit-tested) and advisory by default; a genuine hard failure — or an import hang, detected via a REGISTRY_READY marker rather than a slow registry scan — always gates. One slow addon load can no longer abort the whole test. - Strict mode (GRAMPS_ADDON_TEST_STRICT=1) turns the smoke run into a full gate: dependency skips, environment failures and unverified addon dependencies become hard failures; only slow registry-scan timeouts stay advisory. Covered by synthetic regression tests that don't boot the registry. - Import/export smoke tests fail (not silently skip) when a dependency-satisfied plugin fails to load. - Unlisted-addon manifest: source-based and per-registration (parses each .gpr.py with ast), so an accidental include_in_listing=False — even on one plugin inside a mixed directory like TMGimporter — fails the test. - Neutralise blocking modal dialogs an addon may pop at import (e.g. lxml without python3-lxml) so a load can't hang or scatter UI. - Silence expected locale / PyGI noise so results are readable. - Failure messages lead with the real failures; make.py's test docstring and the load-test docstring document default (smoke) vs strict (full gate) modes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
eduralph
force-pushed
the
test/shared-test-harness
branch
from
July 9, 2026 01:50
01f184a to
6b83be2
Compare
eduralph
marked this pull request as ready for review
July 9, 2026 01:53
Contributor
Author
|
@GaryGriffin - it was abit more work than I originally thought but it should be clean now. There were some tricky items to differentiate between loading hangs and other things, but it should all be good. Let me know what you think about it. |
Contributor
Author
|
@dsblank - I took your idea and ran with it. The PR 820 already had some tests in place, I just needed to wire up your addition and change it a little bit. |
Member
|
Confused. I ran Also, still get warning on Photo Tagging but not EditExIfMetadata, both checking Gexiv2 (but different strings). Seems strange there are Warnings for exactly the ones that have a comma-separated version in the requires_gi but not for the ones without. |
…heck Gramps accepts a comma-separated list of acceptable versions in a plugin's requires_gi: Requirements.check_gi() in gramps/gen/utils/requirements.py splits on "," and takes the first version that resolves. _check_dependencies() passed the raw string straight to gi.require_version(), which raises ValueError — so the two addons in the tree that use the form, GraphView (GooCanvas "2.0,3.0") and PhotoTaggingGramplet (GExiv2 "0.10,0.12,0.14,0.16"), were reported as having an unmet dependency on hosts that in fact satisfy it. Advisory in the default smoke run; a false hard failure under GRAMPS_ADDON_TEST_STRICT=1. Neither .gpr.py is wrong. Reported by Gary Griffin on PR 980. Delegate to Gramps' own Requirements rather than re-implement the checks, so the test agrees with what Gramps does at runtime. That also drops two smaller deviations: requires_mod was import_module()'d where Gramps uses find_spec (no import side effects), and requires_exe hand-rolled a PATH scan where Gramps' search_for also tries name + ".exe" for Windows. The gi.repository import probe is kept, now run after check_gi() has pinned the matching version, so a typelib that resolves but fails to import stays a dependency skip instead of a hard failure blamed on the addon. TestDependencyCheck covers the comma form (red before this change: reports a spurious gi:Gtk-3.0,4.0), the single-version form, an absent namespace, and an absent module/executable. It boots no registry and needs no display. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
neutralize_gui_dialogs() replaced gramps.gui.dialog's dialogs with silent
no-ops, so an addon that constructs one at import time — e.g. lxmlGramplet on a
host without python3-lxml — loaded clean and the suite said nothing. That kept
the load from hanging on a modal (Gramps' ErrorDialog.__init__ calls .run()),
but it also hid the addon bug the stub exists to survive: before the stub the
same addon was reported as "hung during import", after it, as nothing at all.
Raised by Gary Griffin on PR 980 ("Is this now hidden?"). It was.
Each stub now records its construction, on gramps_test_env.dialogs_raised and as
a DIALOG_MARKER line on stdout. The isolated load already captures the
subprocess' stdout, so test_load_all_addon_modules scans it and reports a dialog
at import as a hard failure in both modes, naming the addon and the dialog
class. The marker is matched only at the start of a line, so a traceback quoting
it cannot forge a hit.
The load-failure header now reads "failed to load cleanly ... real defects"
rather than "failed to load ... real load failures": an addon caught this way
did load, and the message must not claim otherwise.
Verified against the real addon: with lxml unimportable, lxml/lxmlGramplet.py
raises ErrorDialog at import (dialogs_raised == ['ErrorDialog'], hard failure);
the same probe against PR 981's lxmlGramplet raises none. The gate itself is
covered synthetically by test_dialog_raised_at_import_gates, which is red
without the stdout scan even though the load exits 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Good points ... I buried the GUI issues (including lxml) and that is probably not the right approach to take. It protects the test from choking but also starts hiding issues. It now records the dialog, and the load test reports it as a hard failure in both modes, naming the addon and the dialog class. So lxml goes back to red — with an accurate message this time, "raised ErrorDialog at import" rather than "hung" — stays red until #981 merges, and any future addon that does the same gets caught rather than silently neutralised. GExiv2 — that's my bug, Gramps supports a comma-separated version list in requires_gi. It splits, and accepts if any listed version resolves. My _check_dependencies() in tests/test_plugin_registration.py didn't. To your earlier question — "did this actually load the requires_mod/requires_gi packages?" — no. The dependency check only probes (find_spec / gi.require_version / PATH lookup) to decide whether it's fair to attempt the load; the actual import happens in the isolated subprocess afterwards. So a missing typelib is a skip, never a failure, outside strict mode. |
Member
|
Not cherry picking to gramps61 unless you explicitly tell me to check it. |
Member
|
One suggestion - running this creates a file debug.log . If at the end of the run, it is zero length, suggest you delete it. Only leave it if it has messages. |
Member
|
FYI - I ran this PR (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Carved out of #820 so the addon-facing tests can land before the CI pipeline itself. These four items only need Gramps — they have zero dependency on the
.github/scripts/CI code — so they are reviewable and mergeable on their own. Splitting them out also shrinks #820 toward the "one concern per PR" shape the maintainers asked for.What's in this PR
tests/gramps_test_env.py—GrampsTestCase/GrampsDbTestCasebase classes (stdlibunittest, mirroring upstream Gramps' own test style). Boots the plugin manager once per process and hands out fresh in-memory DBs.tests/test_plugin_registration.py— registers every addon through the real GrampsPluginRegister/BasePluginManager, verifiesgramps_target_version, valid id/name/version, and smoke-tests import/export entry functions.tests/test_plugin_load_gate.py— regression test that drives the plugin-load gating path (a hard, non-dependency load failure must fail the test rather than pass silently).make.py— a newtestcommand that runs the repo-roottests/suite viaunittestdiscovery (the exact discovery the CI job uses), gated onGRAMPSPATHlike the other commands.tests/__init__.py(the GI version pin) is already onmaintenance/gramps60, so it isn't part of this diff.Running the tests
GRAMPSPATH=/path/to/gramps python3 make.py gramps60 testGRAMPSPATHmust point at a Gramps 6.0 checkout so the addon target versions match.test_plugin_registrationboots the full plugin registry and needs the GTK typelibs present;test_plugin_load_gateis import-light and runs anywhere.Relationship to #820
The three remaining root tests in #820 —
test_addon_system_deps.py,test_requires_mod_dedup.py,test_run_addon_tests_paths.py— stay there because they import modules out of.github/scripts/, which doesn't exist until the CI PR merges. Once this PR lands, #820 rebases and sheds these files from its diff, leaving it strictly CI infrastructure.🤖 Generated with Claude Code