Skip to content

Convert lettuce-5.0 tests from groovy to java#9547

Merged
trask merged 10 commits into
open-telemetry:mainfrom
jaydeluca:lettuce-5.0
Sep 29, 2023
Merged

Convert lettuce-5.0 tests from groovy to java#9547
trask merged 10 commits into
open-telemetry:mainfrom
jaydeluca:lettuce-5.0

Conversation

@jaydeluca

Copy link
Copy Markdown
Member

Related to #7195

Notes:

  • I had to modify the assertions in LettuceSyncClientTest.testConnectException() slightly because the groovy test references a class directly that is not accessible in java (io.netty.channel.AbstractChannel.AnnotatedConnectException)
  • I created a helper method to handle provisioning the containers and connections for the individual tests that require them. I am using the cleanup extension to defer the closing of the connection, and then also using try with resources in the individual tests. If this seems redundant let me know and I can remove one of them.

@jaydeluca jaydeluca requested a review from a team September 24, 2023 12:25
@laurit

laurit commented Sep 28, 2023

Copy link
Copy Markdown
Contributor

@jaydeluca see #9574

});
});

await().untilAsserted(() -> assertThat(future).isCompletedWithValue(defaultVal));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usually we use

assertThat(future.get(10, TimeUnit.SECONDS)).isEqualTo(defaultVal));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated, thanks!

@trask trask merged commit 2c40224 into open-telemetry:main Sep 29, 2023
@jaydeluca jaydeluca deleted the lettuce-5.0 branch September 30, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants