feat: Add --manager flag to CLI project create command (#1410)#606
feat: Add --manager flag to CLI project create command (#1410)#606sd66dar wants to merge 13 commits into
Conversation
Add 11 template files for manager project generation: - Manager annotation, interfaces, and exception - Manager implementation with lifecycle methods - Resource implementation - Field injection handler - Unit test template - Maven pom.xml and Gradle build files - OSGi bnd.bnd configuration Part of issue #1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
Add 7 new unit tests for manager functionality: - TestProjectCreateWithManagerFlagCreatesManagerProject - TestProjectCreateWithManagerFlagAndNoManagerNameUsesPackageName - TestProjectCreateWithManagerAndOBRCreatesManagerInOBR - TestProjectCreateManagerWithInvalidManagerNameFails - TestProjectCreateManagerCommandLineWithManagerFlag - TestProjectCreateManagerCommandLineWithoutManagerNameUsesDefault - TestProjectCreateManagerAndTestsTogetherFails Also includes helper function assertManagerProjectCreated() to verify generated manager project structure. Signed-off-by: Sheriell Dar <asd638796@gmail.com>
Simplified generated manager templates based on code review feedback: Template Changes: - Removed example methods from interfaces (getManagerInfo, getSampleValue) - Stripped down ManagerImpl to essential lifecycle methods only - Removed verbose logging and comments - Added null validation to ResourceImpl constructor - Updated to JUnit 5 (jupiter) from JUnit 4 - Simplified unit tests to focus on core functionality - Added Private-Package directive to bnd.bnd for internal package - Removed commons-logging import from bnd.bnd (not used) Philosophy: - Templates should be minimal and production-ready - No opinionated demo logic that users might forget to remove - Clear TODO markers where users should add their own code - Modern tooling (JUnit 5) Part of issue #1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
Updated projectCreate_test.go to use capitalizeFirst() helper instead of deprecated strings.Title function. Part of issue #1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
Complete implementation of issue #1410 to create manager projects. Implementation: - Added capitalizeFirst() helper (replaces deprecated strings.Title) - Implemented createManagerProject() with full manager generation - Fixed OBR generation to use feature names correctly - Integrated with existing project creation flow Documentation: - Updated README.md with --manager flag usage and examples - Auto-generated CLI documentation updated - Added stress test script for validation Build & Test: - Updated build-locally.sh with manager project tests - Tests manager creation with Maven, Gradle, and both - All 7 unit tests passing Part of issue #1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
eamansour
left a comment
There was a problem hiding this comment.
Thanks for taking on this story! The changes are looking great, I've just added a few comments/suggestions that would be great to go through whenever you can :)
jadecarino
left a comment
There was a problem hiding this comment.
Thanks very much for this PR Sheriell! Really keen to get this feature into Galasa. Changes look great overall, I have a few additional minor comments in addition to Eamonn's. Let us know if you'd like to discuss any requested changes!
- Updated build.gradle to use Java 17 toolchain syntax - Updated pom.xml maven-compiler-plugin to Java 17 - Removed non-existent Maven repository URL from build.gradle Addresses review comments from PR #1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
- Fixed duplicate OBR entries in pom.xml and settings.gradle The parent files were adding OBR to childModules array AND the templates were also adding it via IsOBRRequired flag - Added GALASA_ERROR_INVALID_MANAGER_NAME error code (GAL1055E) - Updated manager validation to use correct error message - Updated secrets baseline Addresses critical review comments from PR #1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
- Removed mutual exclusivity between --manager and --features - Refactored createProject to support creating both manager and test projects - Added createCombinedParentFolderContents functions for unified parent files - Updated test to verify both flags work together (like SimBank) - Added early manager name validation to maintain error handling - Updated README.md with example of combined usage This allows creating projects with both managers and tests in a single command, matching the SimBank project structure. Addresses review comment from PR #1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
Script tests creating multiple manager projects with different configurations to validate the implementation. Signed-off-by: Sheriell Dar <asd638796@gmail.com>
jadecarino
left a comment
There was a problem hiding this comment.
Thanks for addressing my comments Sheriell! I've resolved all of my original comments and I have one 1-line change remaining that I just noticed.
In terms of Eamonn's comments, as part of this PR, please can we also get the build issue resolved as well as add the missing Javadoc to ManagerImpl and the missing properties and resource management template files.
We would be happy for you to add the documentation to docs and tackle the more complex issue of adding a manager to an existing test project in a second PR.
| } | ||
| } | ||
|
|
||
| repositories { |
There was a problem hiding this comment.
We are missing mavenLocal() which should be at the top of this list of repositories
- Enhanced ManagerImpl.java with detailed documentation for: - Class overview explaining manager lifecycle - All lifecycle methods (initialise, youAreRequired, provisionGenerate, provisionDiscard) - Resource generation method - NAMESPACE constant - Enhanced ResourceImpl.java with documentation for: - Class overview and purpose - Constructor with parameter validation - getTag() method - Provides clear guidance for developers extending the generated manager code Addresses review feedback from galasa-dev/projectmanagement#1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
- Added PropertiesSingleton for CPS access - Added ExampleProperty demonstrating CPS property patterns - Added ResourceManagement skeleton with comprehensive TODOs - Updated ManagerImpl to initialize CPS on startup - All files include detailed Javadoc and guidance comments Addresses review feedback from galasa-dev/projectmanagement#1410 Signed-off-by: Sheriell Dar <asd638796@gmail.com>
…nerator The template files were added in the previous commit but weren't being generated because they weren't registered in projectCreate.go. - Register ResourceManagement.java in file generation list - Register PropertiesSingleton.java in file generation list - Register ExampleProperty.java in file generation list - Create properties subdirectory before generating files - Update tests to verify new files are created Fixes file generation for CPS properties and resource management templates. Signed-off-by: Sheriell Dar <asd638796@gmail.com>
|
|
||
| // Make --manager and --features mutually exclusive | ||
| projectCreateCmd.MarkFlagsMutuallyExclusive("manager", "features") | ||
| // Note: --manager and --features can now be used together (like SimBank project) |
There was a problem hiding this comment.
Very minor - can we remove this comment as it's not explaining anything new (the --features flag help text already tells us that --manager can be used with it)
| return err | ||
| } | ||
| // createCombinedParentFolderContents creates parent pom.xml/settings.gradle with specified module names | ||
| // This allows combining manager and test modules in a single project (like SimBank) |
There was a problem hiding this comment.
Very minor - can we remove the (like SimBank) mentions from comments in this PR
| if isOBRRequired { | ||
| childModules = append(childModules, packageName+".obr") | ||
| } | ||
| // Note: Do not add OBR to childModules here - the template handles it via IsOBRRequired flag |
There was a problem hiding this comment.
Can we remove this comment as well? It's talking about code that has been removed
| if isOBRRequired { | ||
| childModules = append(childModules, packageName+".obr") | ||
| } | ||
| // Note: Do not add OBR to childModules here - the template handles it via IsOBRRequired flag |
There was a problem hiding this comment.
Can we remove this comment as well? It's talking about code that has been removed
| } | ||
| } | ||
|
|
||
| repositories { |
| Bundle-Name: {{.BundleName}} | ||
| Bundle-SymbolicName: {{.BundleName}} | ||
| Bundle-Version: 0.0.1.qualifier | ||
| Export-Package: \ | ||
| {{.BundleName}},\ | ||
| {{.BundleName}}.spi | ||
| Private-Package: \ | ||
| {{.BundleName}}.internal | ||
| Import-Package: \ | ||
| dev.galasa,\ | ||
| dev.galasa.framework.spi,\ | ||
| org.osgi.service.component.annotations,\ | ||
| javax.validation.constraints,\ | ||
| * | ||
| -includeresource: \ | ||
| META-INF/services=src/main/resources/META-INF/services No newline at end of file |
There was a problem hiding this comment.
When running the build-locally script, the build fails with a different reason now that the duplicate .obr issue has been fixed.
The error is due to the includeresource line which isn't needed:
> Task :dev.galasa.example.docker.manager:jar FAILED
warning: Unused -privatepackage instructions , no such package(s) on the class path: [dev.galasa.example.docker.manager.internal]
error : Input file does not exist: src/main/resources/META-INF/services
[Incubating] Problems report is available at: file:///Users/em/projects/temp/galasa/modules/cli/temp/dev.galasa.example.docker/build/reports/problems/problems-report.html
FAILURE: Build failed with an exception.There's also a problem with the packages listed in the bnd.bnd file. A warning appears in the build output because the wrong package is provided in the Private-Package section. Currently:
dev.galasa.example.docker.manager.internal
but the source code has:
dev.galasa.example.docker.internal
The exported packages have the same problem - they all mention dev.galasa.example.docker.manager, but the code doesn't have a dev.galasa.example.docker.manager package.
Can we adapt the bnd.bnd file from the tokenised example in galasa-dev/projectmanagement#1410 (comment), something like:
Bundle-Name: {{ .BundleName }}
Export-Package: {{ .BundleName }},\
{{ .BundleName }}.spi
Import-Package: !javax.validation.constraints,\
*
There was a problem hiding this comment.
It's also worth running the build-locally script before pushing changes so that you can spot these issues and get the build to update any auto-generated docs as well :)
| @@ -0,0 +1,220 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Since the build-locally script will test the generated manager, is this script being used anywhere? If not, it might be worth removing or combining parts of it into the tests that run as part of the build-locally script
- Added PackageName field to BndParameters struct - Export-Package now uses PackageName (e.g., dev.galasa.example.docker) instead of BundleName (e.g., dev.galasa.example.docker.manager) - Removed Private-Package section (not needed) - Removed -includeresource line that referenced non-existent META-INF/services directory - Fixed Import-Package to exclude javax.validation.constraints with ! prefix This fixes build errors where OSGi was looking for packages with .manager suffix that don't exist in the Java code structure. Signed-off-by: Sheriell Dar <asd638796@gmail.com>
Refer to galasa-dev/projectmanagement#1410.
Summary
Implements #1410 to add manager project creation capability to the Galasa CLI. The new
--managerflag allows developers to generate a complete manager project structure with all necessary components.CLI Enhancement
--managerflag togalasactl project createcommand--managerNameparameter for specifying manager name--managerand--featuresflagsManager Project Generation
The tool generates a complete manager project including:
IManager.java) - Public API for the managerManagerImpl.java) - Core manager logic with lifecycle methodsManager.java) - For injecting the manager into test classesIManagerResource.java) - Public API for managed resourcesResourceImpl.java) - Resource implementation with null safetyManagerImplTest.java) - JUnit 5 test suiteTesting
build-locally.shDocumentation
--managerflag usage and examples