mjsax commented on code in PR #18816:
URL: https://github.com/apache/kafka/pull/18816#discussion_r2029593389
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStoreTest.java:
##########
@@ -89,8 +95,81 @@ public class ChangeLoggingKeyValueBytesStoreTest {
public void before() {
context = mockContext();
context.setTime(0);
+ store = new ChangeLoggingKeyValueBytesStore(innerMock);
store.init(context, store);
}
+ private void mockPosition() {
Review Comment:
nit: insert empty lines between methods; above here, and also down below.
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStoreTest.java:
##########
@@ -62,18 +63,23 @@
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasEntry;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@SuppressWarnings("rawtypes")
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.STRICT_STUBS)
-public class ChangeLoggingKeyValueBytesStoreTest {
+class ChangeLoggingKeyValueBytesStoreTest {
Review Comment:
Can a test class be package private?
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStoreTest.java:
##########
@@ -111,29 +190,38 @@ public void after() {
}
@Test
- public void shouldDelegateInit() {
- final InternalMockProcessorContext context = mockContext();
- final KeyValueStore<Bytes, byte[]> innerMock =
mock(InMemoryKeyValueStore.class);
+ void shouldDelegateInit() {
Review Comment:
Same question as above for the class: I thought test methods must be
`public`?
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStoreTest.java:
##########
@@ -89,8 +95,81 @@ public class ChangeLoggingKeyValueBytesStoreTest {
public void before() {
context = mockContext();
context.setTime(0);
+ store = new ChangeLoggingKeyValueBytesStore(innerMock);
store.init(context, store);
}
+ private void mockPosition() {
+ when(innerMock.getPosition()).thenReturn(Position.emptyPosition());
+ }
+ private void mockGet(final Map<Bytes, byte[]> mockMap) {
Review Comment:
Why are we using `mockMap`? It seems unnecessarily complex? -- It seems much
more straightforward to just "expect" calls into the `innerMock` store?
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStoreTest.java:
##########
@@ -89,8 +95,81 @@ public class ChangeLoggingKeyValueBytesStoreTest {
public void before() {
context = mockContext();
context.setTime(0);
+ store = new ChangeLoggingKeyValueBytesStore(innerMock);
store.init(context, store);
}
+ private void mockPosition() {
+ when(innerMock.getPosition()).thenReturn(Position.emptyPosition());
+ }
+ private void mockGet(final Map<Bytes, byte[]> mockMap) {
+ when(innerMock.get(any(Bytes.class))).thenAnswer(invocation ->
mockMap.get(invocation.getArgument(0)));
+ }
+ private void mockPut(final Map<Bytes, byte[]> mockMap) {
+ doAnswer(invocation -> {
+ mockMap.put(invocation.getArgument(0), invocation.getArgument(1));
+ StoreQueryUtils.updatePosition(innerMock.getPosition(), context);
+ return null;
+ }).when(innerMock).put(any(Bytes.class), any(byte[].class));
Review Comment:
Why do we use `doAnswer(...).when(...)` here?
In `mockGet` we use `when(...).thanAnswer(...)` what I find much easier to
read.
--
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
