cameronlee314 commented on a change in pull request #918: SAMZA-2094: Implement the StartpointVisitor for the KafkaSystemConsumer. URL: https://github.com/apache/samza/pull/918#discussion_r257063620
########## File path: samza-kafka/src/test/java/org/apache/samza/system/kafka/TestKafkaSystemConsumer.java ########## @@ -205,6 +215,156 @@ public void testFetchThresholdBytesDiabled() { consumer.stop(); } + @Test + public void testStartpointSpecificOffsetVisitorShouldUpdateTheFetchOffsetInConsumer() { + // Define dummy variables for testing. + final Integer testPartitionId = 0; + final String offset = "0"; + final TopicPartition testTopicPartition = new TopicPartition(TEST_STREAM, testPartitionId); + final Partition testPartition = new Partition(testPartitionId); + final SystemStreamPartition testSystemStreamPartition = new SystemStreamPartition(TEST_SYSTEM, TEST_STREAM, testPartition); + + final KafkaConsumer consumer = Mockito.mock(KafkaConsumer.class); + KafkaStartpointRegistrationHandler + kafkaStartpointRegistrationHandler = new KafkaSystemConsumer.KafkaStartpointRegistrationHandler(consumer); + + final StartpointSpecific testStartpointSpecific = new StartpointSpecific(offset); + + // Mock the consumer interactions. + Mockito.doNothing().when(consumer).seek(testTopicPartition, Long.valueOf(offset)); + + // Invoke the consumer with startpoint. + kafkaStartpointRegistrationHandler.visit(testSystemStreamPartition, testStartpointSpecific); + + // Mock verifications. + Mockito.verify(consumer).seek(testTopicPartition, Long.valueOf(offset)); + } + + @Test + public void testStartpointTimestampVisitorShouldUpdateTheFetchOffsetInConsumer() { + // Define dummy variables for testing. + final Integer testPartitionId = 0; + final TopicPartition testTopicPartition = new TopicPartition(TEST_STREAM, testPartitionId); + final Partition testPartition = new Partition(testPartitionId); + final SystemStreamPartition testSystemStreamPartition = new SystemStreamPartition(TEST_SYSTEM, TEST_STREAM, testPartition); + final Long testTimeStamp = 10L; + final String testOffset = "10"; + + final KafkaConsumer consumer = Mockito.mock(KafkaConsumer.class); + KafkaStartpointRegistrationHandler + kafkaStartpointRegistrationHandler = new KafkaSystemConsumer.KafkaStartpointRegistrationHandler(consumer); + + final StartpointTimestamp startpointTimestamp = new StartpointTimestamp(testTimeStamp); + final Map<TopicPartition, OffsetAndTimestamp> offsetForTimesResult = ImmutableMap.of(testTopicPartition, new OffsetAndTimestamp(Long.valueOf(testOffset), testTimeStamp)); + + // Mock the consumer interactions. + Mockito.when(consumer.offsetsForTimes(Mockito.anyMap())).thenReturn(offsetForTimesResult); + Mockito.doNothing().when(consumer).seek(testTopicPartition, Long.valueOf(testOffset)); + + kafkaStartpointRegistrationHandler.visit(testSystemStreamPartition, startpointTimestamp); + + // Mock verifications. + Mockito.verify(consumer).seek(testTopicPartition, Long.valueOf(testOffset)); + Mockito.verify(consumer).offsetsForTimes(Mockito.anyMap()); + } + + @Test + public void testStartpointOldestVisitorShouldUpdateTheFetchOffsetInConsumer() { + // Define dummy variables for testing. + final Integer testPartitionId = 0; + final TopicPartition testTopicPartition = new TopicPartition(TEST_STREAM, testPartitionId); + final Partition testPartition = new Partition(testPartitionId); + final SystemStreamPartition testSystemStreamPartition = new SystemStreamPartition(TEST_SYSTEM, TEST_STREAM, testPartition); + + final KafkaConsumer consumer = Mockito.mock(KafkaConsumer.class); + final KafkaStartpointRegistrationHandler + kafkaStartpointRegistrationHandler = new KafkaSystemConsumer.KafkaStartpointRegistrationHandler(consumer); + + final StartpointOldest testStartpointSpecific = new StartpointOldest(); + + // Mock the consumer interactions. + Mockito.doNothing().when(consumer).seekToBeginning(ImmutableList.of(testTopicPartition)); + + // Invoke the consumer with startpoint. + kafkaStartpointRegistrationHandler.visit(testSystemStreamPartition, testStartpointSpecific); + + // Mock verifications. + Mockito.verify(consumer).seekToBeginning(ImmutableList.of(testTopicPartition)); + } + + @Test + public void testStartpointUpcomingVisitorShouldUpdateTheFetchOffsetInConsumer() { + // Define dummy variables for testing. + final Integer testPartitionId = 0; + final TopicPartition testTopicPartition = new TopicPartition(TEST_STREAM, testPartitionId); + final Partition testPartition = new Partition(testPartitionId); + final SystemStreamPartition testSystemStreamPartition = new SystemStreamPartition(TEST_SYSTEM, TEST_STREAM, testPartition); + + final KafkaConsumer consumer = Mockito.mock(KafkaConsumer.class); + final KafkaSystemConsumer.KafkaStartpointRegistrationHandler + kafkaStartpointRegistrationHandler = new KafkaSystemConsumer.KafkaStartpointRegistrationHandler(consumer); + + final StartpointUpcoming testStartpointSpecific = new StartpointUpcoming(); + + // Mock the consumer interactions. + Mockito.doNothing().when(consumer).seekToEnd(ImmutableList.of(testTopicPartition)); + + // Invoke the consumer with startpoint. + kafkaStartpointRegistrationHandler.visit(testSystemStreamPartition, testStartpointSpecific); + + // Mock verifications. + Mockito.verify(consumer).seekToEnd(ImmutableList.of(testTopicPartition)); + } + + @Test + public void testStartInvocationAfterStartPointsRegistrationShouldInvokeTheStartPointApplyMethod() { + // Initialize the constants required for the test. + Consumer mockConsumer = Mockito.mock(Consumer.class); + String testSystemName = "testSystem"; + String testStreamName = "testStream"; + Config testConfig = new MapConfig(); + String testClientId = "testClientId"; + KafkaSystemConsumerMetrics kafkaSystemConsumerMetrics = new KafkaSystemConsumerMetrics(testSystemName, new NoOpMetricsRegistry()); + + // Test system stream partitions. + SystemStreamPartition testSystemStreamPartition1 = new SystemStreamPartition(testSystemName, testStreamName, new Partition(0)); + SystemStreamPartition testSystemStreamPartition2 = new SystemStreamPartition(testSystemName, testStreamName, new Partition(1)); + SystemStreamPartition testSystemStreamPartition3 = new SystemStreamPartition(testSystemName, testStreamName, new Partition(2)); + SystemStreamPartition testSystemStreamPartition4 = new SystemStreamPartition(testSystemName, testStreamName, new Partition(3)); + + // Different kinds of {@code Startpoint}. + StartpointSpecific startPointSpecific = new StartpointSpecific("100"); + StartpointTimestamp startpointTimestamp = new StartpointTimestamp(100L); + StartpointOldest startpointOldest = new StartpointOldest(); + StartpointUpcoming startpointUpcoming = new StartpointUpcoming(); + + // Mock the visit methods of KafkaStartpointRegistrationHandler. + KafkaSystemConsumer.KafkaStartpointRegistrationHandler + mockStartPointVisitor = Mockito.mock(KafkaStartpointRegistrationHandler.class); + Mockito.doNothing().when(mockStartPointVisitor).visit(testSystemStreamPartition1, startPointSpecific); + Mockito.doNothing().when(mockStartPointVisitor).visit(testSystemStreamPartition2, startpointTimestamp); + Mockito.doNothing().when(mockStartPointVisitor).visit(testSystemStreamPartition3, startpointOldest); + Mockito.doNothing().when(mockStartPointVisitor).visit(testSystemStreamPartition4, startpointUpcoming); + + // Instantiate KafkaSystemConsumer for testing. + KafkaSystemConsumer kafkaSystemConsumer = new KafkaSystemConsumer(mockConsumer, testSystemName, testConfig, + testClientId, kafkaSystemConsumerMetrics, new TestClock(), mockStartPointVisitor); + kafkaSystemConsumer.proxy = Mockito.mock(KafkaConsumerProxy.class); Review comment: Could you please try to avoid directly accessing and mutating fields? Maybe include the proxy in your VisibleForTesting constructor. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services