jihoonson commented on a change in pull request #10383:
URL: https://github.com/apache/druid/pull/10383#discussion_r520286004
##########
File path:
indexing-service/src/test/java/org/apache/druid/indexing/overlord/sampler/InputSourceSamplerTest.java
##########
@@ -1060,6 +1062,107 @@ public void testWithFilter() throws IOException
);
}
+ @Test
+ public void testIndexParseException() throws IOException
Review comment:
I think we need such tests because `KafkaIndexTaskTest` only covers the
actual indexing side, but not the sampling side. Without those tests, we could
break the sampler without being noticed.
> For test cases for `InputSourceSampler` here, the input source is
`InlineInputSource` which always uses `InputEntityIteratingReader` even when
input format is `JsonInputFormat`, there's no chance to test that. So I think
we don't need add test cases here.
Maybe this is not a good place for the new tests, but you don't have to use
the `InlineInputSource` for them. Rather, you should use
`RecordSupplierInputSource` as you mentioned. Since `RecordSupplierInputSource`
accepts the `RecordSupplier` interface as a parameter which is the data
supplier, I think you could implement a mock for testing (I don't think we
should test `Firehose` as it's deprecated). Or do you see some reason you
cannot? If it's hard, I'm also OK with adding them in a follow-up.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
