----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17918/#review34158 -----------------------------------------------------------
core/src/test/resources/log4j.properties <https://reviews.apache.org/r/17918/#comment64169> this is probably included in the patch by accident right? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64231> Naming it topic1 implies there will be topic2 as well. Can we stick to just topic? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64225> we probably don't want to print anything from unit tests. core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64227> These 3 statements are repeated in many tests. Can we create a helper method to create topic successfully? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64233> What is the advantage of sending 10 times numRecords instead of just numRecords? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64234> testClose->testSendToPartition core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64235> Shouldn't the rest of the tests have acks set to -1 as well? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64236> why only wait until leader for partition 0 is elected? We should also probably wait until leader for partition 1 is elected so the test is not time dependent. core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64238> and check they has numRecords+1 messages with -> and check they have numRecords+1 messages core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64240> The variable records for partition 0 is named record0. This is true for response as well. To be consistent, should this be fetchResponse0 for partition 0? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64242> can we pull this logic in a method and do the same check for partition 1 as well? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64241> messageSet0 doesn't exist in this patch. core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/17918/#comment64244> testCreateTopic -> testAutoCreateTopic project/Build.scala <https://reviews.apache.org/r/17918/#comment64170> since we intend to check this in on trunk, we don't need to touch the sbt files. Just depend on gradle to build/test/run tools etc. and it seems to work for each of these actions already. - Neha Narkhede On Feb. 11, 2014, 1:51 a.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17918/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2014, 1:51 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1233 > https://issues.apache.org/jira/browse/KAFKA-1233 > > > Repository: kafka > > > Description > ------- > > Three test cases included in Part I: > > Incorporated Jay's comments. > > 1. Send API, with null key/value/partitionId/topic tested, check returned > offset. Currently passed. > > 2. All messages drained before close, also check that messages with the same > key go to the same partition. Currently passed. > > 3. The default round robin partitioner, and the specified partition id is > respected. Currently broken with ack = -1. Filed KAFKA-1255. > > 4. Send with auto-topic-creation. Currently passed. > > > Diffs > ----- > > build.gradle 858d297b9e8bf8a2bca54c4817f9ca2affd0d3f2 > core/src/test/resources/log4j.properties > d7d03ea1b13299d4ee41b9907720d014c81faf66 > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala > PRE-CREATION > project/Build.scala ddcfc4176e68933377590e095c07283083249f4a > > Diff: https://reviews.apache.org/r/17918/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
