-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24006/#review49049
-----------------------------------------------------------



core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85879>

    Could you group kafka imports together before java/scala/other-libs imports?



core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85878>

    Could we use 
    
    val numPartitions = 12
    val replicationFactor = 3
    
    and then create expectedReplicaAssignment and leaderForPartitionMap based 
on these two variables, and re-use them here?



core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85880>

    expectedReplicaAssignment seems not used any more.



core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85888>

    Could you add a comment here for bouncing server 1?



core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85890>

    Is there a specific reason we want to use 10 seconds instead of default 5 
seconds?



core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85889>

    Is this println intended?



core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/24006/#comment85891>

    Do we still need expectedReplicaAssignment?



core/src/test/scala/unit/kafka/utils/TestUtils.scala
<https://reviews.apache.org/r/24006/#comment85892>

    Could we just set the default value of configs parameter to null, instead 
of creating a separate function?


- Guozhang Wang


On July 28, 2014, 8:52 p.m., Jonathan Natkins wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24006/
> -----------------------------------------------------------
> 
> (Updated July 28, 2014, 8:52 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1420
>     https://issues.apache.org/jira/browse/KAFKA-1420
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1420 Replace AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK 
> with TestUtils.createTopic in all unit tests
> 
> 
> Diffs
> -----
> 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 
> e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 29cc01bcef9cacd8dec1f5d662644fc6fe4994bc 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala 
> f44568cb25edf25db857415119018fd4c9922f61 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> c4e13c5240c8303853d08cc3b40088f8c7dae460 
> 
> Diff: https://reviews.apache.org/r/24006/diff/
> 
> 
> Testing
> -------
> 
> Automated
> 
> 
> Thanks,
> 
> Jonathan Natkins
> 
>

Reply via email to