[GitHub] [kafka] C0urante commented on a diff in pull request #12444: KAFKA-14101: Improve documentation for consuming from embedded Kafka cluster topics in Connect integration testing framework

[GitBox <gi...@apache.org>]

C0urante commented on code in PR #12444:
URL: https://github.com/apache/kafka/pull/12444#discussion_r933894953


##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/ExampleConnectIntegrationTest.java:
##########
@@ -157,7 +157,7 @@ public void testSinkConnector() throws Exception {
 
         // consume all records from the source topic or fail, to ensure that they were correctly produced.

Review Comment:
   Ah, good catch! This comment has been copy+pasted across a large number of files:
   
   - `ExampleConnectIntegrationTest`
   - `ErrorHandlingIntegrationTest`
   - `TransformationIntegrationTest`
   - `RebalanceSourceConnectorsIntegrationTest`
   
   In some cases, it's accurate. For example, at this part, we've just manually produced 2000 records into the topic, and we're now re-reading all of those records back to make sure that they were produced successfully.
   
   In other cases, like line 218 in this file, the word "all" is inaccurate since we're consuming from a topic that's being actively written to by a running source connector, which has probably written more than the expected number of records by this point. I think removing "all" from the comment in this case would make it a little ambiguous as to what would cause the method invocation to fail. What do you think about "consume all expected records..." instead of "consume all records"?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org