> On March 25, 2015, 9:51 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala, > > lines 85-106 > > <https://reviews.apache.org/r/31606/diff/2/?file=881935#file881935line85> > > > > Can we use TestUtils.sendMessages to replace this function?
My understanding was that we wanted to test kafka.javaapi.producer.Producer here (not necessarily kafka.producer.Producer). In kafka.javaapi.consumer.ZookeeperConsumerConnectorTest.scala is the intention to test the Java API integration, or the actual functionality of sendMessages? > On March 25, 2015, 9:51 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala, > > lines 116-134 > > <https://reviews.apache.org/r/31606/diff/2/?file=881935#file881935line116> > > > > Can we get rid of this function since it is only called once, and hence > > we can just put the logic there? Same as above: In kafka.javaapi.consumer.ZookeeperConsumerConnectorTest.scala is the intention to test the Java API integration, or the actual functionality of getMessages in this case? > On March 25, 2015, 9:51 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/utils/TestUtils.scala, line 735 > > <https://reviews.apache.org/r/31606/diff/2/?file=881937#file881937line735> > > > > Can we replace this function also? Done - consolidated the two methods (sendMessages() and sendMessagesToPartition()) > On March 25, 2015, 9:51 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/utils/TestUtils.scala, lines 761-773 > > <https://reviews.apache.org/r/31606/diff/2/?file=881937#file881937line761> > > > > Compression code is no longer used anymore, which seems not correct? The compression.codec is now set on sendMessages() On March 25, 2015, 9:51 p.m., Flutra Osmani wrote: > > A few more general comments: > > > > 1. Could you rebase the patch? > > 2. Could we also replace LogRecoverTest.sendMessages? > > 3. Could we also replace TestLogCleaning.produceMessages? > > 4. Could we also replace UncleanLeaderElectionTest.produceMessage? 1. Done. 2. Done. 3. This seems very specific, and it appears to not be very convenient to consolidate with the existing methods in TestUtils 4. Done. - Flutra ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31606/#review77804 ----------------------------------------------------------- On March 2, 2015, 1:25 a.m., Flutra Osmani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31606/ > ----------------------------------------------------------- > > (Updated March 2, 2015, 1:25 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1416 > https://issues.apache.org/jira/browse/KAFKA-1416 > > > Repository: kafka > > > Description > ------- > > Initial Patch > > > Diffs > ----- > > > core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala > a17e8532c44aadf84b8da3a57bcc797a848b5020 > core/src/test/scala/unit/kafka/integration/FetcherTest.scala > 25845abbcad2e79f56f729e59239b738d3ddbc9d > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala > ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala > d6248b09bb0f86ee7d3bd0ebce5b99135491453b > core/src/test/scala/unit/kafka/metrics/MetricsTest.scala > 111e4a26c1efb6f7c151ca9217dbe107c27ab617 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 6ce18076f6b5deb05b51c25be5bed9957e6b4339 > > Diff: https://reviews.apache.org/r/31606/diff/ > > > Testing > ------- > > > Thanks, > > Flutra Osmani > >
