Implement Generate Functionality: fix ProjectWorker config structure, add CLAP support, cleanup, and test coverage - #57
Conversation
… add tests Agent-Logs-Url: https://github.com/DirektDSP/PluginConfiguratorApp/sessions/f55a91fa-1e9b-460c-be8b-fccf0d53ca9a Co-authored-by: SeamusMullan <43112447+SeamusMullan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/DirektDSP/PluginConfiguratorApp/sessions/f55a91fa-1e9b-460c-be8b-fccf0d53ca9a Co-authored-by: SeamusMullan <43112447+SeamusMullan@users.noreply.github.com>
📊 PR SummaryChanges Overview
CI ChecksThis PR will trigger the following checks:
Please ensure all checks pass before merging. |
|
@copilot fix lint fails |
📊 PR SummaryChanges Overview
CI ChecksThis PR will trigger the following checks:
Please ensure all checks pass before merging. |
There was a problem hiding this comment.
Pull request overview
This PR fixes ProjectWorker project-generation behavior so user-provided configuration is consistently honored (version, plugin code, format selection), hardens template cloning defaults/validation, improves failure cleanup, and adds dedicated unit test coverage for these behaviors.
Changes:
- Fix configuration precedence in
prepare_project_variables()(version and plugin code) and add missing CLAP format emission. - Harden
clone_template_repo()(output directory validation + default template URL fallback) and add cleanup-on-failure behavior inrun(). - Add comprehensive unit tests for the updated
ProjectWorkerbehaviors, including git author env injection during the initial commit.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/core/project_worker.py |
Adds default template URL fallback, cleanup-on-failure, fixes params precedence for version/plugin code, and adds CLAP format support plus git author env injection. |
tests/test_project_worker.py |
Introduces a new test suite covering the corrected generation logic, clone validation/fallback, cleanup behavior, and git env handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if os.path.exists(output_dir): | ||
| raise RuntimeError(f"Output directory already exists: {output_dir}") | ||
|
|
||
| fork_url = self.params.get("fork_url", "") or DEFAULT_TEMPLATE_URL |
There was a problem hiding this comment.
fork_url falls back to DEFAULT_TEMPLATE_URL only when the param is falsy. A whitespace-only value (e.g. " ") will bypass the fallback and be passed to git clone, which will fail. Consider normalizing with .strip() (similar to plugin_code/version) before applying the default URL.
| fork_url = self.params.get("fork_url", "") or DEFAULT_TEMPLATE_URL | |
| fork_url = str(self.params.get("fork_url", "")).strip() or DEFAULT_TEMPLATE_URL |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
_cleanup_on_failure() suppresses all exceptions during shutil.rmtree(...) with a bare except Exception: pass. If cleanup fails (permissions, locked files, etc.) it will be silently ignored, leaving partial output behind with no visibility. Consider at least emitting a progress/error message (or logging) with the exception details so failures are diagnosable.
| except Exception: | |
| pass | |
| except Exception as e: | |
| self.error.emit( | |
| f"Failed to clean up partial output directory '{output_dir}': {e!s}" | |
| ) |
| # The commit call is the third subprocess.run call (init, add, commit) | ||
| commit_call = mock_run.call_args_list[2] |
There was a problem hiding this comment.
This test assumes the git commit invocation is always the 3rd subprocess.run call (call_args_list[2]). That’s brittle if init_git_repo() adds another git command later (e.g., setting config, creating branches). Consider locating the commit call by scanning call_args_list for an entry whose argv contains "commit", then asserting on its kwargs.
| # The commit call is the third subprocess.run call (init, add, commit) | |
| commit_call = mock_run.call_args_list[2] | |
| commit_call = next( | |
| ( | |
| call | |
| for call in mock_run.call_args_list | |
| if call.args | |
| and call.args[0] | |
| and "commit" in call.args[0] | |
| ), | |
| None, | |
| ) | |
| assert commit_call is not None |
ProjectWorkerwas silently discarding user-configured values and missing format support, causing generated projects to not reflect the actual configuration filled in by the user.Fixes in
src/core/project_worker.pyversionignored – params value now takes precedence over VERSION file; file/default only consulted as fallbackplugin_codealways regenerated – user-supplied code is now used;generate_plugin_id()is only called when the field is blankoptions["clap"] → "CLAP"branch (was silently dropped from CMakeLists.txt)clone_template_repo()hardened – raisesRuntimeErrorwhenoutput_directoryis empty or already exists; falls back toDEFAULT_TEMPLATE_URLwhenfork_urlis blank_cleanup_on_failure()(removes partial output dir);run()now calls it before emitting the error signalinit_git_repo()now passesGIT_AUTHOR_*/GIT_COMMITTER_*env var fallbacks so the initial commit succeeds in CI or unconfigured environmentsNew
tests/test_project_worker.py39 unit tests covering: version/plugin-code param precedence, CLAP inclusion, clone pre-flight validation, default URL fallback, cleanup-on-failure, and git author env var injection.