-
Notifications
You must be signed in to change notification settings - Fork 119
MOSIP-44097 - Globalized variable for Dependency json path and added property in kernel for generating dependency json #1864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ernel for generating dependency json Signed-off-by: Mohanachandran S <mohanachandran.s@technoforte.co.in>
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new public static field in BaseTestCase to hold a test-case inter-dependency JSON path and a new Kernel.properties flag Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java (1)
194-196: Consider aligning path construction with existing patterns for run-context awareness.The path is constructed using
user.dirand assumes an IDE directory structure (src/main/resources/config/). The codebase already hasgetGlobalResourcePath()(lines 228-249) that handles JAR vs IDE execution contexts differently. Since the related propertygenerateDependencyJsonin Kernel.properties is documented to work "only in case of local run using IDE," consider:
- Adding a run-context check to prevent misuse in JAR mode
- Following the existing pattern used by
getGlobalResourcePath()for consistency- Deferring path construction until it's needed (lazy initialization) rather than at class load time
🔎 Example approach using existing patterns:
- public static String testCaseInterDependencyPath = Paths - .get(System.getProperty("user.dir"), "src", "main", "resources", "config", "testCaseInterDependency.json") - .toString(); + private static String testCaseInterDependencyPath = null; + + public static String getTestCaseInterDependencyPath() { + if (testCaseInterDependencyPath == null) { + if (runTypeS.equalsIgnoreCase("IDE")) { + testCaseInterDependencyPath = Paths + .get(System.getProperty("user.dir"), "src", "main", "resources", "config", "testCaseInterDependency.json") + .toString(); + } else { + logger.warn("testCaseInterDependencyPath is only supported in IDE mode"); + testCaseInterDependencyPath = ""; + } + } + return testCaseInterDependencyPath; + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java(2 hunks)apitest-commons/src/main/resources/config/Kernel.properties(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-maven-apitest-commons / maven-build
- GitHub Check: build-maven-authentication-demo-service / maven-build
- GitHub Check: Analyze (java-kotlin)
Signed-off-by: Mohanachandran S <165888272+mohanachandran-s@users.noreply.github.com>
MOSIP-44097 - Globalized variable for Dependency json path and added property in kernel for flag for generating dependency json
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.