Repository navigation
Conversation
Introduces the syncAddonWithTemplate.py automation tool to streamline synchronizing add-on metadata and infrastructure with upstream template updates. Detailed changes: - Added `syncAddonWithTemplate.py` at repository root to handle AST-aware merging of buildVars.py and pyproject.toml configuration. - Configured `PROTECTED_ELEMENTS` in the sync script to prevent overwriting template-specific files (e.g., `.github/workflows/unitTests.yml` and `tests/`). - Added `.addonmergeignore` support for defining project-specific file exclusion rules during synchronization. - Added dependencies for the sync tool to `pyproject.toml`. - Added `tests/unit/test_syncAddonWithTemplate.py` to validate metadata parsing, AST transformations, TOML formatting, and execution ordering. - Updated `pyproject.toml` to exclude `syncAddonWithTemplate.py` alongside the `tests/` directory from ruff and pyright checks. - Updated `docs/managementFromGit/updatingExistingAddons.md` with full usage instructions, CLI flags, and execution modes for the sync script. - Updated `docs/unitTesting.md` with guidelines for running unit tests locally using unittest and uv.
Update testFormatAuthorList in test_syncAddonWithTemplate.py to assert that empty author email keys are omitted rather than expecting an empty string, matching syncAddonWithTemplate.py behavior.
…umentation Update description of testFormatAuthorList to document that empty email fields are omitted for PEP 621 compliance.
There was a problem hiding this comment.
Pull request overview
Adds a developer-facing synchronization tool to help NVDA add-on authors update their repositories to the latest AddonTemplate structure, including AST-aware metadata handling and TOML merging, with accompanying unit tests and documentation updates.
Changes:
- Introduces
syncAddonWithTemplate.pyto sync template infrastructure while mergingbuildVars.pyandpyproject.toml. - Adds unit tests for key merge/formatting behavior and a small template sanity suite.
- Updates dependency groups/docs to support running the sync tool and the unit test suite.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
syncAddonWithTemplate.py |
New sync/merge engine (AST + tomlkit) for updating add-on repos from the template. |
tests/unit/test_syncAddonWithTemplate.py |
Unit tests for metadata extraction, buildVars merge, TOML indentation, and dependency merging. |
tests/unit/template/sanity.py |
Minimal sanity tests intended to validate CI test discovery/execution. |
docs/managementFromGit/updatingExistingAddons.md |
Expanded guidance for updating add-ons (automated tool + manual merge). |
docs/unitTesting.md |
Updated instructions for running the unit tests and describing the new suites. |
pyproject.toml |
Adds tomlkit, updates pyright, and excludes the new sync script/tests from ruff/pyright. |
uv.lock |
Locks tomlkit and bumps pyright to match pyproject.toml. |
tests/__init__.py |
Updates package docstring. |
tests/unit/__init__.py |
Updates package docstring. |
.addonmergeignore |
Present at repo root (empty in this PR context). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ty test - Move test infrastructure docstring from root unit tests to template submodule. - Revert root `tests/__init__.py` docstring to reflect all test types (system and unit). - Remove obsolete `sanity.py` test file.
|
Hi @seanbudd,
|
- Replace underscores with hyphens in `getBasePackageName` and `legacyToolingBases` to handle equivalent Python package name formats during dependency merges. - Bootstrap `.addonmergeignore` from template on first sync if absent locally, allowing it to self-reference and manage its own persistence dynamically.
|
@Copilot,
|
- Replace `methodOrder.index()` with `orderIndex.get(a, defaultOrder)` map. - Prevent `ValueError` when running tests on inherited or dynamic methods.
|
@Copilot,
|
- Fix incorrect path for `test_syncAddonWithTemplate.py` in `unitTesting.md`. - Clarify `tomlkit` installation requirement and fix no-argument command example in `updatingExistingAddons.md`.
|
@Copilot,
|
- Add testSetupAddonMergeIgnore and testAddonMergeIgnore to test_syncAddonWithTemplate.py - Document both new tests in docs/unitTesting.md
|
Hi @seanbudd, @Copilot, Regarding the test coverage for
|
|
Hi @seanbudd, When running I bumped the lock version to Let me know if you would still prefer me to revert it to keep the PR scope strictly minimal! |
- Fix casing for AddonTemplate repository URL in updatingExistingAddons.md - Remove detailed syncAddonWithTemplate test suite breakdown in unitTesting.md to keep coverage in docstrings
|
Thanks for the review and all these helpful suggestions, @seanbudd! I have addressed the quick fixes and documentation updates:
Regarding your suggestion to split I'm currently refactoring the code into submodules, writing the requested additional test cases, and running all necessary tests. I will address all of your remaining feedback and suggestions throughout this process and post a full update over the weekend. I'll leave the PR in draft mode until everything is ready for review. |
|
Sean, regarding your comment on I'm currently implementing a solution for this and will push it in my next commit over the weekend, as I need to finish up some work commitments first. The previous Replacing it with a dynamic lookup alongside a minimal mapping dictionary for non-supported/replaced dependencies (like The updated logic now dynamically collects all tooling packages directly from the template's Additionally, for deprecated tools that were replaced in the template (such as This maps replaced tools to their modern template equivalents so they aren't accidentally preserved as custom dependencies. Here is a snippet of how the dynamic collection works: # Dynamically collect all template packages (dependencies + dependency-groups)
templateBases: set[str] = {
getBasePackageName(d) for d in tplDeps if isinstance(d, str)
}
if "dependency-groups" in mergedData and isinstance(mergedData["dependency-groups"], MutableMapping):
for grp in mergedData["dependency-groups"].values():
if isinstance(grp, (list, MutableSequence)):
for grpItem in grp:
if isinstance(grpItem, str):
templateBases.add(getBasePackageName(grpItem)) |
- Add syncAddonTool package directory to lighten syncAddonWithTemplate.py. - Import submodules from syncAddonTool into syncAddonWithTemplate.py for improved code readability and structure. - Add test fixtures directory to decouple legacy dictionaries and sample files from test_syncAddonWithTemplate.py. - Add unit tests verifying version priority handling between user dependencies and template dependencies. - Add syncAddonTool.spec at repository root to streamline executable generation with PyInstaller.
…tandalone executable - Update docs/managementFromGit/updatingExistingAddon.md to document the syncAddonTool directory. - Add instructions for building and running the standalone executable using PyInstaller and `uv run --with pyinstaller`.
|
Hi @abdel792 . Thanks for considering the suggestion about end to end tests. I think that it will be better to have a PR with end to end test, merge it, and then merge this one, so that we can ensure that test past. For reference, see |
|
Hi @nvdaes, Thanks for the clarification and for sharing the reference. Following the same approach as my previous Pull Request that introduced the minimal unit test structure under I will keep this PR in Draft status, create a separate branch, and submit a dedicated Pull Request for the E2E test infrastructure first. Once that E2E test PR is merged into Thanks again for your guidance and support! |
|
Hi @nvdaes, Following up on your feedback regarding establishing the testing infrastructure first, I have submitted a dedicated Pull Request to set up the E2E foundation: 👉 PR #59 Summary of changes included in this PR:
Once this foundational PR is reviewed and merged into Thanks again for your time and review! |
…nd update docs - Replace docs/unitTesting.md with docs/testing.md, consolidating documentation for both unit and E2E testing guides. - Introduce tests/e2e/template/test_syncAddonTool.py with 10 comprehensive E2E tests covering core CLI workflows, flags, options, and PyInstaller packaging. - Remove legacy and modern buildVars migration tests from unit tests as they are now fully covered by the E2E suite.
|
Hi @nvdaes, I have pushed the final updates incorporating your feedback, completing the E2E testing infrastructure, and updating the PR description:
The test suite structure now mirrors the unit tests perfectly, with a total of 20 tests (10 E2E and 10 unit tests) passing successfully! Ready for final review. |
- Update .github/workflows/tests.yml to run on windows-latest. - Ensure PyInstaller can correctly build and validate the Windows executable (.exe) within the CI environment.
|
Following a test failure in the CI pipeline regarding the PyInstaller executable build on Linux, I have updated the GitHub Actions workflow (".github/workflows/tests.yml") to run on "windows-latest". This allows the full test suite—including the PyInstaller standalone packaging and E2E CLI integration tests—to execute successfully in a native Windows environment where building All 20 tests are now passing successfully! |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved migration and exclusion defects can corrupt or overwrite configuration, and cleanup steps can delete developer files.
12 open findings
Escape control characters in generated double-quoted literals · New Handle annotated addon_info assignments · New Preserve or reject nonliteral metadata expressions · New Preserve imports and definitions for global expressions · New Honor exclusions during configuration merges · New Restrict package matching to dependency arrays · New Remove existing static versions from merged dynamic metadata · New Preserve i18nSources during build variable extraction · New Match directory exclusions with trailing separators · New Populate repository metadata before merging template defaults · New Use temporary paths for packaging tests · New Avoid deleting untracked files during synchronization · New
9 resolved since last review
Report synchronization failures with nonzero exit status Abort synchronization on syntax errors before modifying project files Preserve or reject non-constant legacy dictionary values Avoid altering TOML multiline string contents during substitution Use standards-aware requirement parsing and merging Map addon_sourceURL to the repository URL Implement documented fnmatch and negation ignore patterns Protect existing .addonmergeignore files from template overwrites Avoid requiring tomlkit when it is unavailable or unnecessary
🧠 Review effort: Balanced
|
@abdel792, there are comments from Copilot. You may want to review them, and I'll review the final result if you need my feedback. |
- Use unicode_escape encoding in formatStringLiteral to correctly handle newlines and control characters inside double-quoted string literals.
- Update AnnAssign AST node processing in parseBuildvars to check for addon_info. - Support extraction of metadata from both annotated dictionary assignments and annotated AddonInfo call keywords.
- Check if AddonInfo keyword values are AST constants before formatting. - Retain original source expressions (such as variables or function calls) without forcing string literals or quotation marks.
- Add usesGlobModule helper in astUtils.py to detect glob references in extracted global expressions. - Automatically inject 'from glob import glob' into generated buildVars.py when required to prevent NameError. - Add testMergeBuildvarsAutoImportsGlob unit test to verify automatic import injection.
- Guard buildVars.py and pyproject.toml merges with isPathProtected checks. - Report excluded configuration files as skipped (protected scope) when matched in .addonmergeignore.
- Update mergeDependencyLists to check the contextName and bypass package-name parsing for non-dependency arrays (such as classifiers, include, and exclude paths). - Apply a safe value-based union and preservation policy for non-dependency lists to prevent unintended truncation or overriding of values like project classifiers.
- Filter out fields from project.dynamic if they are already statically defined in the project metadata section, ensuring strict compliance with PEP 621 packaging rules. - Prevent build failures caused by conflicting static and dynamic declarations during template synchronization.
…tion - Add "i18nSources" to topLevelVarsSet in extractBuildvarsMetadata to prevent template defaults from overwriting custom translation sources.
- Strip trailing separators when matching directory-only exclusion rules to ensure directories like build/ are properly recognized and excluded. - Prevent directory-only patterns from matching regular files.
…defaults - Inject repository URL from buildVars.py into project metadata before executing deepMergeDicts. - Prevent template default repository URLs from incorrectly overwriting custom add-on repository URLs when project.urls is initially missing.
- Replace destructive removal of local build/ and dist/ directories with a temporary directory context for PyInstaller builds. - Configure custom --workpath and --distpath arguments to isolate packaging outputs and protect unrelated developer artifacts.
|
I have reviewed and processed all 12 Copilot review findings for this pull request. Each of the 11 code-based corrections has been implemented via a precise, dedicated commit.
This finding was resolved with a reason. The documentation referencing "git clean -f" pre-exists on the master branch and is out of scope for the current pull request. This update must be handled in a separate, dedicated PR. What are your thoughts on this? Thanks @nvdaes for offering your help! Feel free to run some tests on your side whenever you like. |
|
Hi @abdel792 , I've performed the test suite, and all test pass! Thanks for this impressive work. |
- Update updatingExistingAddons.md documentation to remove risky git clean -f command. - Recommend checking changes with git status --short before staging and committing infrastructure updates.
|
@seanbudd You are completely right, and I apologize for initially marking this as "won't fix" by mistake. This section is indeed part of the documentation for syncAddonTool. Using "git clean -f" can accidentally delete newly generated untracked files (such as a new "pyproject.toml" in legacy add-ons) before they are staged. I have just pushed a commit to fix this and update the documentation to use "git status --short" instead. @nvdaes Thank you so much for offering your help, your testing, and your encouragement! I also wanted to take a moment to express my sincere gratitude to you: you are the one who introduced me to creating NVDA add-ons in the first place, and I am still deeply grateful to you for that. It is a true honor for me to contribute to the NVDA project with my modest contributions, and I am really proud to be part of this wonderful community! |



Link to issue number:
Replaces #41.
Summary of the issue:
While upgrading an existing add-on using standard Git commands remains fully supported, some add-on developers prefer an automated and dedicated tool to streamline the process.
Manual step-by-step migration of metadata (from
buildVars.pytopyproject.toml) and file merges can require extra care and verification.Providing an automated script offers a convenient alternative for developers who wish to perform these updates quickly and with built-in AST-based metadata handling.
Description of developer facing changes:
tomlkitfor parsing and mergingpyproject.tomlconfigurations (can be run without prior installation viauv run --no-project --with tomlkit).syncAddonTool/syncAddonTool.spec) to build a standalone executable (syncAddonTool.exe)..github/workflows/tests.ymlandtests/, ensuring they are excluded from add-on synchronization.tests/unit/template/test_syncAddonTool.py) and an End-to-End (E2E) CLI integration test suite (tests/e2e/template/test_syncAddonTool.py).docs/testing.mdto provide unified execution guidelines for both unit and End-to-End (E2E) tests.Description of development approach:
syncAddonToolusing an AST-aware approach andtomlkitto seamlessly migrate legacybuildVars.pymetadata or modernAddonInfosetups intopyproject.toml.It supports timestamped backups, custom exclusions via
.addonmergeignore, and includes.github/workflows/tests.ymlinPROTECTED_ELEMENTS.syncAddonTool/syncAddonTool.specfor building a standalone executable usinguv run --with pyinstaller pyinstaller syncAddonTool/syncAddonTool.spec, eliminating external Python environment requirements for end users.tests/unit/template/test_syncAddonTool.py) for internal AST/synchronization logic, and an E2E test suite (tests/e2e/template/test_syncAddonTool.py) executing isolated subprocess workflows, CLI flags, buildVars migrations, and PyInstaller binary packaging.docs/testing.mdto detail CLI usage, available options, and instructions for executing both unit and E2E test suites.Testing strategy:
Validated the synchronization script and unit test suite locally using
uv:Executed syncAddonTool across various CLI modes:
-adis optional):Validated Standalone Executable Generation via PyInstaller:
Executed
dist/syncAddonTool.exe -ad /path/to/my-nvda-addon --dry-runand confirmed expected behavior.Verified that backup folders (
<addon>_bak_<timestamp>) were generated,.addonmergeignorerules were respected, and protected elements (including.github/workflows/tests.ymlandtests/) were not pushed to target add-on repositories.Executed the complete verification test suite (both unit and E2E tests) via
unittest:Confirmed all tests (10 unit tests and 10 E2E tests) passed successfully.
Confirmed ruff and pyright ignore syncAddonTool and tests/.
Known issues with pull request:
None.
Code Review Checklist
Steps to reproduce are detailed in the testing strategy.