mjsax commented on a change in pull request #11367: URL: https://github.com/apache/kafka/pull/11367#discussion_r718176753
########## File path: streams/src/test/java/org/apache/kafka/streams/state/internals/ListValueStoreTest.java ########## @@ -136,8 +138,9 @@ public void shouldGetAllNonDeletedRecords() { listStore.put(4, "four"); // Delete some records - listStore.put(1, null); - listStore.put(3, null); + listStore.putIfAbsent(1, null); + listStore.putIfAbsent(3, null); + listStore.putIfAbsent(5, null); Review comment: Not sure if I understand this change? Why `putIfAbsent`? Why adding one more case? ########## File path: streams/src/test/java/org/apache/kafka/streams/state/internals/ListValueStoreTest.java ########## @@ -178,26 +181,46 @@ public void shouldGetAllReturnTimestampOrderedRecords() { @Test public void shouldAllowDeleteWhileIterateRecords() { - listStore.put(0, "zero1"); - listStore.put(0, "zero2"); - listStore.put(1, "one"); + final Random rand = new Random(); Review comment: Should we get a `seed` value and log it (to allow reproducing if the test fails)? ``` final long seed = new Random(System.currentTimeMs).nextLong(); log(seed); final Random rand = new Random(seed); ``` -- 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