Add a CI-CD Pipeline - #1
Merged
Merged
Conversation
Introduce GitHub Actions CI workflow with Docker-based jobs for lint, compile-check, unit tests, integration tests, and build. Add shared pytest fixtures for Gramps-backed testing and repo-wide integration tests that verify plugin registration, loading, metadata, and import/export smoke tests. - Two Docker images: gramps-headless (GLib) and gramps-gtk (xvfb) - Linux container + Windows bare runner coverage - Subprocess-isolated plugin loading (crash-safe) - @pytest.mark.gui auto-skip for headless environments - docker-build.yml rebuilds images on Dockerfile changes
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
eduralph
pushed a commit
that referenced
this pull request
Jul 6, 2026
* first working version of excludesubtree person filter * Add boolean option to include the matched persons themselves * fix d63958f exception upon filter building > File "/usr/lib/python3/dist-packages/gramps/gui/editors/filtereditor.py", line 594, in init > t = v1 > TypeError: init() missing 2 required positional arguments: 'uistate' and 'track' * fix #1 proper implementation of boolean filter parameter * add debug logging, slightly optimize runtime * gramps 6.0 compat: rename function apply -> apply_to_one * chore: rename filter result to self.selected_handles as requested in PR * fix issues noted by review in PR gramps-project#933 2. Missing type imports 3. Mutable class-level attribute 4. reset() never tears down the sub-filter 5. Potential AttributeError when Gramps ID is not found 6. Progress counter can exceed its declared total 7. Name/description mismatch between gpr.py and the class 8. Both .py files are missing the required GPL-2.0-or-later license header block. 9. Import sections need the standard comment headers 10. Each class needs a navigation header comment: 11. The ExcludeSubtree class has no docstring. 12. set([]) → set(). (14) get_relatives yields None handles (missing father/mother); filtering inside the generator rather than at the call site would be cleaner. * feat: pass "include matched" param as enum to avoid GUI imports Instead of adding a checkbox-widget for the boolean parameter, avoid GUI code in this non-GUI addon by receiving a generic string and using empty/false as "include" and any other content (e.g. "exclude") as true * chore: run `black` formatter * fix: copy gettext(translation) boilerplate from howto; bump plugin version * fix type annotations & possible AttributeError on None * fix: add `from __future__ import annotations` for backward compatibility * fix: guard log.debug statements to only evaluate when logging enabled * feat: remove "include_stopfilter_matches:bool", default to "exclude" stringly-typed enums / magic values are bad UX. Include can simply be realized by adding the MatchesFilter to the containing Filter, too * remove initialization of class variable selected_handles "confusing" according to review. I still think initializing the empty set is correct: should `apply_to_one` ever be called before `prepare` this will raise an AttributeError.
eduralph
pushed a commit
that referenced
this pull request
Jul 6, 2026
* first working version of excludesubtree person filter * Add boolean option to include the matched persons themselves * fix d63958f exception upon filter building > File "/usr/lib/python3/dist-packages/gramps/gui/editors/filtereditor.py", line 594, in init > t = v1 > TypeError: init() missing 2 required positional arguments: 'uistate' and 'track' * fix #1 proper implementation of boolean filter parameter * add debug logging, slightly optimize runtime * gramps 6.0 compat: rename function apply -> apply_to_one * chore: rename filter result to self.selected_handles as requested in PR * fix issues noted by review in PR gramps-project#933 2. Missing type imports 3. Mutable class-level attribute 4. reset() never tears down the sub-filter 5. Potential AttributeError when Gramps ID is not found 6. Progress counter can exceed its declared total 7. Name/description mismatch between gpr.py and the class 8. Both .py files are missing the required GPL-2.0-or-later license header block. 9. Import sections need the standard comment headers 10. Each class needs a navigation header comment: 11. The ExcludeSubtree class has no docstring. 12. set([]) → set(). (14) get_relatives yields None handles (missing father/mother); filtering inside the generator rather than at the call site would be cleaner. * feat: pass "include matched" param as enum to avoid GUI imports Instead of adding a checkbox-widget for the boolean parameter, avoid GUI code in this non-GUI addon by receiving a generic string and using empty/false as "include" and any other content (e.g. "exclude") as true * chore: run `black` formatter * fix: copy gettext(translation) boilerplate from howto; bump plugin version * fix type annotations & possible AttributeError on None * fix: add `from __future__ import annotations` for backward compatibility * fix: guard log.debug statements to only evaluate when logging enabled * feat: remove "include_stopfilter_matches:bool", default to "exclude" stringly-typed enums / magic values are bad UX. Include can simply be realized by adding the MatchesFilter to the containing Filter, too * remove initialization of class variable selected_handles "confusing" according to review. I still think initializing the empty set is correct: should `apply_to_one` ever be called before `prepare` this will raise an AttributeError.
eduralph
added a commit
that referenced
this pull request
Jul 9, 2026
Addresses a second-pass adversarial review whose theme was that advisory buckets can hide real defects, plus a clearer failure message. Failure message (test_load_all_addon_modules): on failure, lead with only the real load failures (each with its error), then one plain-language line per advisory category explaining the cause and the fix. The full advisory lists stay in the warning log instead of burying the actionable failures under a wall of text. #1 Environment classifier is now specific and anchored. Dropped the over-broad signatures ("DISPLAY", bare "load_icon") that could match a genuine addon exception; kept anchored ones ("Gtk couldn't be initialized", "gtk-icon-theme-error-quark", the NoneType-load_icon phrase, missing-namespace) and only match them on a real error line, not an indented traceback frame or echoed source. Added TestEnvLoadClassifier with positive and negative cases. #2 A real import hang no longer passes as advisory. The isolated-load subprocess prints REGISTRY_READY after registration; on timeout, if the marker was emitted the addon's own import hung -> hard failure, otherwise the registry scan was slow -> advisory. (subprocess.TimeoutExpired captures partial stdout, so this needs no signals and works cross-platform.) #4 Opt-in strict mode for the depends_on load test. Environmentally inconclusive dependents stay advisory by default (headless local runs legitimately can't import GUI addons); setting GRAMPS_ADDON_TEST_STRICT=1 in a CI job with a full runtime promotes any unverified dependent to a failure. The i18n finding was again out of scope: test_i18n_extraction.py is pre-existing on maintenance/gramps60, not part of this PR. Verified against Gramps 6.0.8: classifier unit tests pass, the headless load test still passes with ClipboardGramplet kept advisory, hang-vs-slow-scan and strict-mode gating behave as intended. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Introduce GitHub Actions CI with container-based jobs for lint, compile-check, unit tests, integration tests, and build
Add two Docker images (headless + GTK) hosted on ghcr.io, rebuilt when Dockerfiles change
Add shared pytest fixtures (
tests/conftest.py) for Gramps-backed integration testingAdd repo-wide integration tests verifying plugin registration, loading, metadata, and import/export smoke tests across all 175 addons
Add
@pytest.mark.guimarker with auto-skip for headless environmentsAdd Linux (container) + Windows (bare runner) test coverage
Add CI/CD pipeline with container-based testing
Remove accidentally committed debug.log