Github user chtyim commented on a diff in the pull request:

    https://github.com/apache/twill/pull/16#discussion_r94688950
  
    --- Diff: 
twill-core/src/test/java/org/apache/twill/kafka/client/KafkaTest.java ---
    @@ -170,11 +172,57 @@ public void testKafkaClient() throws Exception {
         Cancellable cancel = kafkaClient.getConsumer().prepare().add(topic, 0, 
0).consume(new KafkaConsumer
           .MessageCallback() {
           @Override
    -      public void onReceived(Iterator<FetchedMessage> messages) {
    +      public long onReceived(Iterator<FetchedMessage> messages, long 
startOffset) {
    +        long nextOffset = startOffset;
             while (messages.hasNext()) {
    -          
LOG.info(Charsets.UTF_8.decode(messages.next().getPayload()).toString());
    +          FetchedMessage message = messages.next();
    +          LOG.info(Charsets.UTF_8.decode(message.getPayload()).toString());
               latch.countDown();
             }
    +        return nextOffset;
    +      }
    +
    +      @Override
    +      public void finished() {
    +        stopLatch.countDown();
    +      }
    +    });
    +
    +    Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
    +    cancel.cancel();
    +    Assert.assertTrue(stopLatch.await(1, TimeUnit.SECONDS));
    +  }
    +
    +  @Test
    +  public void testKafkaClientSkipNext() throws Exception {
    +    String topic = "testClient";
    +    // Publish 30 messages with indecies the same as offsets within the 
range 0 - 29
    +    Thread t1 = createPublishThread(kafkaClient, topic, Compression.GZIP, 
"GZIP Testing message", 10);
    +    t1.start();
    +    t1.join();
    +    Thread t2 = createPublishThread(kafkaClient, topic, Compression.NONE, 
"Testing message", 10, 10);
    +    t2.start();
    +    t2.join();
    +    Thread t3 = createPublishThread(kafkaClient, topic, 
Compression.SNAPPY, "Snappy Testing message", 10, 20);
    +    t3.start();
    +    t3.join();
    +
    +    // 15 messages will be counted since onReceived returns 
`message.getNextOffset() + 1` as next offset to read
    --- End diff --
    
    I don't think the test is correct. You published 30 messages in three 
message set, hence the `onReceived` method will be called three times. The 
first time with messages 0-9 and you return 11. The second call with 11-19, and 
you return 20. The last call with 21-29. So in total there will be more than 15 
messages.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to