> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote: > > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 109 > > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line109> > > > > These 3 statements are repeated in many tests. Can we create a helper > > method to create topic successfully? > > Guozhang Wang wrote: > Have thought about that, the reason I did not do so is that if I do this > may goes to testUtils. But I think this is actually fine now, will fix.
> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote: > > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 211 > > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line211> > > > > Shouldn't the rest of the tests have acks set to -1 as well? > > Guozhang Wang wrote: > ack = -1 is only required when I check the produced message content using > a consumer. Makes sense. > On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote: > > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 219 > > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line219> > > > > 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. > > Guozhang Wang wrote: > For this test I only checks one partition, 0, which I think is > sufficient. Not quite sure what you mean by "time dependent"? I think I misread the test and thought that it used partition 1 without waiting for leader to be election on partition 1. But in your latest patch, the helper method take care of this properly. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17918/#review34158 ----------------------------------------------------------- On Feb. 11, 2014, 8:28 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17918/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2014, 8:28 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1233 > https://issues.apache.org/jira/browse/KAFKA-1233 > > > Repository: kafka > > > Description > ------- > > Four test cases included in Part I: > > Incorporated Jay's comments. > > Rebased on new commits. > > 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 broken after some new commits ... > > 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 d9d6e6a91e6f37c0247107b0655f58a2db9688cd > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 500eeca2f95d901536b1363b8c4b485c4893179f > > Diff: https://reviews.apache.org/r/17918/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
