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


Reply via email to