fix : issue with starting in cluster mode #1596
Conversation
WalkthroughThis pull request introduces several modifications to the Changes
Sequence DiagramsequenceDiagram
participant Test as TestUltimateRedisApplication
participant Config as TestcontainersConfiguration
participant Container as RedisClusterContainer
Test->>Config: Configure Redis Cluster
Config->>Container: Create Container
Config->>Config: Register Dynamic Properties
Test->>Test: Add Cluster Profile
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
type: dependency-upgrade
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
boot-ultimate-redis/src/test/java/com/example/ultimateredis/common/TestcontainersConfiguration.java (1)
13-13: Consider referencingDynamicPropertyRegistryin class javadoc or method doc.
ImportingDynamicPropertyRegistrarclarifies the usage. For better maintainability and discovery, you may want to add a note in class-level or method-level documentation to guide unfamiliar readers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
boot-ultimate-redis/pom.xml(1 hunks)boot-ultimate-redis/src/test/java/com/example/ultimateredis/TestUltimateRedisApplication.java(1 hunks)boot-ultimate-redis/src/test/java/com/example/ultimateredis/common/AbstractIntegrationTest.java(2 hunks)boot-ultimate-redis/src/test/java/com/example/ultimateredis/common/TestcontainersConfiguration.java(1 hunks)boot-ultimate-redis/src/test/java/com/example/ultimateredis/controller/RedisControllerTest.java(3 hunks)
🔇 Additional comments (14)
boot-ultimate-redis/src/test/java/com/example/ultimateredis/common/TestcontainersConfiguration.java (3)
21-26: Validate cluster container readiness for large integration test suites.
The cluster container configuration looks good, but consider verifying that five startup attempts and a four-minute startup timeout are sufficient for large test suites or resource-constrained environments. If test instability is observed, revisit these configuration values or provide environment-based overrides.
28-37: Good separation of responsibilities with a dedicated DynamicPropertyRegistrar.
Separating property registration from container instantiation enhances readability and reusability. If you foresee more dynamic properties in the future, this design will help maintain a clean test configuration setup.
44-44: Keep an eye on Redis image stability.
Upgrading to the 7.4.1-alpine image is a minor bump, but always confirm that there are no known container-level regressions in the updated tag. Regularly check Redis release notes and test stability in your CI environment.
boot-ultimate-redis/src/test/java/com/example/ultimateredis/common/AbstractIntegrationTest.java (2)
11-11: Adoption of MockMvcTester
Switching the import to MockMvcTester indicates a move toward a more fluent and expressive style for writing integration tests. This is a good step for improved readability and maintainability.
20-20: Verify compatibility with @AutoConfigureMockMvc
While MockMvcTester can work in tandem with Spring Boot’s test configuration, ensure that @AutoConfigureMockMvc is still relevant if the project fully migrates to a MockMvcTester-based workflow. If future changes remove direct usage of MockMvc, consider adjusting or removing @AutoConfigureMockMvc.
boot-ultimate-redis/src/test/java/com/example/ultimateredis/controller/RedisControllerTest.java (6)
3-3: Assertion with AssertJ
The import of assertThat from AssertJ further streamlines test readability and expressiveness. Great choice for fluent assertions.
8-8: Import of GenericResponse
Using GenericResponse simplifies response parsing. Ensure the class handles or extends potential error states if future code returns additional fields or error details.
14-14: Use of HttpStatus
Referencing HttpStatus constants helps keep tests self-explanatory. Good practice.
24-33: Transition to mockMvcTester in addRedisKeyValue
This code block cleanly demonstrates the new fluent approach with mockMvcTester. The readability and chainable .assertThat() approach are improvements. The test logic is clear, with correct HTTP status checks and assertion on the boolean response.
38-47: Consistent usage of mockMvcTester in getFromCache
Continuing the same fluent pattern for GET requests aligns well with the new testing style. Verifying that the response matches "JunitValue" is straightforward. Looks good.
58-68: expireFromCache test logic
Using Awaitility to poll until the key expires is well-structured. The combination with mockMvcTester and fluent assertions ensures clarity. The approach effectively tests Redis TTL behavior.
boot-ultimate-redis/pom.xml (1)
95-95: Upgrading Google Java Format plugin
Upgrading to version 1.25.2 may introduce new formatting rules or improvements. Run mvn spotless:apply to ensure the codebase aligns with the new rules before merge.
boot-ultimate-redis/src/test/java/com/example/ultimateredis/TestUltimateRedisApplication.java (2)
4-4: Import of cluster profile constant looks good.
The import statement is necessary to reference the PROFILE_CLUSTER constant when activating the cluster profile.
12-12: Verify the cluster profile configuration.
Adding the cluster profile is crucial for testing in cluster mode. Please confirm that the cluster profile is defined and properly configured (e.g., properties or environment variables) so that tests run correctly when this profile is active.
✅ Verification successful
Let me gather more information about the cluster profile configuration.
Let me check the cluster configuration properties.
The cluster profile is properly configured and verified
The cluster profile is well-defined and properly configured in the codebase:
- The profile constant
PROFILE_CLUSTERis defined inAppConstants.java - Cluster-specific configuration exists in
application-cluster.propertieswith proper Redis cluster nodes and settings - The profile is correctly used in
TestcontainersConfiguration.javawith appropriate@Profileannotations for Redis cluster container and property registration - The test application correctly activates this profile using
withAdditionalProfiles(AppConstants.PROFILE_CLUSTER)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verifying presence and usage of PROFILE_CLUSTER across the codebase
rg -A 3 'PROFILE_CLUSTER'
Length of output: 2431
Script:
#!/bin/bash
# Check for cluster-related configuration files and properties
fd -e yml -e yaml -e properties | xargs rg -l 'cluster|redis'
# Check for Redis configuration classes
ast-grep --pattern 'class $_ {
$$$
@Bean
$$$
Redis$_($$$) {
$$$
}
}'
Length of output: 911
Script:
#!/bin/bash
# Check the cluster configuration properties
cat boot-ultimate-redis/src/main/resources/application-cluster.properties
# Check the cluster docker compose configuration
cat boot-ultimate-redis/docker/docker-compose.yml
Length of output: 683
|
Qodana Community for JVM101 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at [email protected]
|
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores