cadonna commented on code in PR #13927:
URL: https://github.com/apache/kafka/pull/13927#discussion_r1258012878


##########
streams/src/test/java/org/apache/kafka/streams/integration/NamedTopologyIntegrationTest.java:
##########
@@ -173,9 +173,9 @@ public static void closeCluster() {
     private static final List<KeyValue<String, Long>> STANDARD_INPUT_DATA =
         asList(pair("A", 100L), pair("B", 200L), pair("A", 300L), pair("C", 
400L), pair("C", -50L));
     private static final List<KeyValue<String, Long>> COUNT_OUTPUT_DATA =
-        asList(pair("B", 1L), pair("A", 2L), pair("C", 2L)); // output of 
count operation with caching

Review Comment:
   @wcarlson5 Could you elaborate what is worrying you here?
   
   I ran the tests with disabled state updater and the disabled cache and all 
tests pass. That tells me that the state updater does not change any results.
   Additionally, the results with enabled and disabled cache are semantically 
equivalent.
   ```
   pair("B", 1L), pair("A", 2L), pair("C", 2L)
   ```
   is equivalent to
   ```
   pair("A", 1L), pair("B", 1L), pair("A", 2L), pair("C", 1L), pair("C", 2L)
   ```
   The latter just produces more intermediate results.
   
   In general, if a test verifies the correctness of results, disabling the 
cache makes the test more robust, because the results do not depend on commit 
time intervals or production rates.
   
   Let me know if I missed something here.  
   



-- 
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