Convert Lettuce 4.0 tests from groovy to java#9419
Merged
Conversation
Member
Awesome! 🚀
We usually use awaitility for these kinds of things. AFAIR it should even be automatically imported in the |
|
|
||
| @Test | ||
| void testCommandChainedWithThenAccept() throws Throwable { | ||
| AsyncConditions conditions = new AsyncConditions(); |
Member
There was a problem hiding this comment.
Instead of that you could use either an AtomicReference or a CompletableFuture that you would set from the redisFuture.thenAccept() callback; and then assert on that ref/future using Awaitility.await()
Member
Author
There was a problem hiding this comment.
thank you for this suggestion, I believe I have implemented the proposed change
…mentation into lettuce-4.0
…d utilize AutoCleanupExtension
mateuszrzeszutek
approved these changes
Sep 15, 2023
mateuszrzeszutek
left a comment
Member
There was a problem hiding this comment.
This is looking great, thanks! Left just a couple small comments
| static RedisCommands<String, String> syncCommands; | ||
|
|
||
| @BeforeAll | ||
| public static void setUp() { |
Member
There was a problem hiding this comment.
Nit: JUnit 5 test lifecycle methods need not be public
Suggested change
| public static void setUp() { | |
| static void setUp() { |
| equalTo(SemanticAttributes.NET_PEER_NAME, host), | ||
| equalTo(SemanticAttributes.NET_PEER_PORT, port), | ||
| equalTo(SemanticAttributes.DB_SYSTEM, "redis")))); | ||
| testConnection.close(); |
Member
There was a problem hiding this comment.
It'd be safer to use the AutoCleanupExtension; the connection would get closed even if the assertion fails.
Closed
6 tasks
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
2 participants
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.
Related to #7195
A few notes:
The groovy tests used a spock helper classUpdated to use Awaitibilityspock.util.concurrent.AsyncConditions, which I ended up re-creating, but if there's another existing class or strategy I should use in place of that, please let me know and I can make those adjustments