edoardocomar commented on code in PR #15910:
URL: https://github.com/apache/kafka/pull/15910#discussion_r1608896699
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/OffsetSyncStoreTest.java:
##########
@@ -82,77 +86,131 @@ public void testOffsetTranslation() {
@Test
public void testNoTranslationIfStoreNotStarted() {
- try (FakeOffsetSyncStore store = new FakeOffsetSyncStore()) {
- // no offsets exist and store is not started
- assertEquals(OptionalLong.empty(), store.translateDownstream(null,
tp, 0));
- assertEquals(OptionalLong.empty(), store.translateDownstream(null,
tp, 100));
- assertEquals(OptionalLong.empty(), store.translateDownstream(null,
tp, 200));
+ FakeOffsetSyncStore store = new FakeOffsetSyncStore() {
Review Comment:
Hi @gharris1727
I use IntelliJ too and saw the warning.
I could have used a `@suppress` annotation but I am very reluctant to make
code less readable because of limited insight by linters. Similarly to make the
fake store more complex.
Using try-with-resource with a local class results in horrible indentation
as you said.
I don't share a strong worry of future leaks in testing - seems speculative
to me.
In this instance unless you have very strong feelings, I'd really leave the
test as-is
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
