MLE-24733 Merging the two test ml-gradle projects#1026
Merged
Conversation
Needs two separate invocations to handle data/modules going to different locations.
rjrudin
requested review from
BillFarber,
anu3990 and
stevebio
as code owners
|
Copyright Validation Results ⏭️ Skipped (Excluded) Files
✅ All files have valid copyright headers! |
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates two separate ml-gradle test projects by merging test-complete-app-mlDeploy into test-app. The changes eliminate the duplicate deployment project and configure test-app to handle both standard and e2e test deployments through separate invocations with different property configurations.
- Removed the standalone
test-complete-app-mlDeployproject directory and all its files - Enhanced
test-appto support e2e test data/module deployment via a newgradle-e2e.propertiesconfiguration - Updated CI/CD and documentation to reflect the consolidated approach
Reviewed Changes
Copilot reviewed 25 out of 77 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test-complete-app-mlDeploy/* | Entire directory removed including security configs, gradle wrapper, and build files |
| test-app/gradle-e2e.properties | New properties file defining e2e-specific data/module paths and database configuration |
| test-app/gradle.properties | Added properties plugin configuration for environment-based property loading |
| test-app/build.gradle | Added properties plugin version update and curl tasks previously in mlDeploy project |
| test-app/src/main/ml-config/servers/node-client-api-ssl-server.json | Updated SSL certificate template reference |
| Jenkinsfile | Streamlined e2e test setup by removing separate mlDeploy project deployment |
| CONTRIBUTING.md | Updated deployment instructions to include e2e data/module loading step |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "authentication": "digestbasic", | ||
| "ssl-certificate-template": "ssl-project-template" | ||
| } No newline at end of file | ||
| "ssl-certificate-template": "node-client-ssl-template" |
There was a problem hiding this comment.
The SSL certificate template reference has been changed from 'ssl-project-template' to 'node-client-ssl-template', but this new template is not included in the visible configuration files. Ensure this template is defined elsewhere in the project or deployed separately.
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.
Needs two separate invocations to handle data/modules going to different locations.