-
Notifications
You must be signed in to change notification settings - Fork 21
[ES-2393] Mock plugin java11 to java21 migration #176
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
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
WalkthroughUpgraded mock-plugin build from Java 11 to Java 21, updated many Maven plugins and dependency versions (including Spring Boot/Cloud and Lombok), added Mockito test dependency, removed an illegal-access JVM flag, changed workflow template references to master-java21, and removed a test Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mock-plugin/pom.xml(3 hunks)mock-plugin/src/test/java/io/mosip/esignet/plugin/mock/service/MockHelperServiceTest.java(1 hunks)mock-plugin/src/test/java/io/mosip/signup/plugin/mock/service/MockIdentityVerifierPluginImplTest.java(5 hunks)
🔇 Additional comments (5)
mock-plugin/pom.xml (1)
165-168: Good practice: version inherited from dependencyManagement.The spring-web dependency correctly inherits its version from the spring-boot-dependencies BOM defined in dependencyManagement. This ensures version consistency across Spring dependencies.
mock-plugin/src/test/java/io/mosip/esignet/plugin/mock/service/MockHelperServiceTest.java (1)
7-10: Import organization looks good.The imports are properly organized and align with the test requirements. No issues detected with these standard DTO imports.
mock-plugin/src/test/java/io/mosip/signup/plugin/mock/service/MockIdentityVerifierPluginImplTest.java (3)
10-11: Imports correctly support async Kafka testing.The new imports (ProducerRecord, RecordMetadata, SendResult, CompletableFuture) are appropriate for mocking asynchronous Kafka send operations in the test.
Also applies to: 24-24, 33-33
76-83: Proper async Kafka mock implementation.The test correctly mocks KafkaTemplate's asynchronous send method by returning a completed CompletableFuture. This allows synchronous testing of async behavior and is a best practice with Mockito 5.x.
92-94: Assertions properly verify async Kafka sends.The test correctly captures and validates multiple Kafka send operations, confirming both the count (2 sends) and the content (transaction ID). This provides good coverage of the async behavior.
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
mock-plugin/src/test/java/io/mosip/esignet/plugin/mock/service/MockHelperServiceTest.java
Show resolved
Hide resolved
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
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: 0
🧹 Nitpick comments (1)
mock-plugin/pom.xml (1)
260-270: Optional: Add explicit version tag to surefire plugin for consistency.The
maven-surefire-pluginhas a version property defined at line 79 (${maven.surefire.plugin.version}) but doesn't use it in the plugin declaration. Other plugins in the POM do specify versions explicitly. Consider adding<version>${maven.surefire.plugin.version}</version>for consistency with plugins likesonar-maven-pluginandspring-boot-maven-plugin.<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> + <version>${maven.surefire.plugin.version}</version> <configuration>This applies similarly to
maven-jar-plugin,maven-war-plugin, andmaven-javadoc-pluginif you wish to standardize the approach across all plugins.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mock-plugin/pom.xml(3 hunks)
🔇 Additional comments (4)
mock-plugin/pom.xml (4)
70-90: Java 21 migration is solid and comprehensive.All Java version properties, Maven plugins, and Spring framework components have been appropriately updated for Java 21. Spring Boot 3.2.12 and Spring Cloud 2023.0.8 are the current stable releases in their respective lines, and all versions are compatible with Java 21.
106-106: Lombok version is current.Lombok 1.18.42 is the latest stable release and fully compatible with Java 21.
148-153: Mockito dependency addition is appropriate.Mockito 5.20.0 (latest 5.x release as of November 2025) has been added with correct test scope, enabling modern mocking capabilities for Java 21.
260-270: Surefire configuration is correct for Java 21.The
--illegal-access=permitflag has been properly removed, and the--add-opens java.xml/jdk.xml.internal=ALL-UNNAMEDdirective is retained for module access. This configuration is compatible with Java 21.
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.