----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review76157 -----------------------------------------------------------
This looks like a very good start. I think the framework is flexible enough to allow us to add a variety of upgrade tests. I'm looking forward to it. I have few comments, but mostly I'm still confused on how this will be used. Perhaps more comments or even a readme is in order You wrote that we invoke "test.sh <dir1> <dir2>", what should each directory contain? just the kafka jar of different versions? or an entire installation (including bin/ and conf/)? Which one of the directories should be the newer and which is older? does it matter? Which version of clients will be used. Perhaps a more descriptive name for test.sh can help too. I'm guessing we'll have a whole collection of those test scripts soon. Gwen build.gradle <https://reviews.apache.org/r/30809/#comment123636> This should probably be a test dependency (if needed at all) Packaging Guava will be a pain, since so many systems use different versions of Guava and they are all incompatible. core/src/main/scala/kafka/tools/ContinuousValidationTest.java <https://reviews.apache.org/r/30809/#comment123635> Do we really want to do this? We are using joptsimple for a bunch of other tools. It is easier to read, maintain, nice error messages, help screen, etc. system_test/broker_upgrade/bin/kafka-run-class.sh <https://reviews.apache.org/r/30809/#comment123638> Why did we decide to duplicate this entire file? - Gwen Shapira On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30809/ > ----------------------------------------------------------- > > (Updated March 9, 2015, 11:55 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1888 > https://issues.apache.org/jira/browse/KAFKA-1888 > > > Repository: kafka > > > Description > ------- > > Fixing the tests based on Mayuresh comments, code cleanup after proper IDE > setup > > > Diffs > ----- > > build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af > core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION > system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION > system_test/broker_upgrade/bin/test.sh PRE-CREATION > system_test/broker_upgrade/configs/server1.properties PRE-CREATION > system_test/broker_upgrade/configs/server2.properties PRE-CREATION > system_test/broker_upgrade/configs/zookeeper_source.properties PRE-CREATION > > Diff: https://reviews.apache.org/r/30809/diff/ > > > Testing > ------- > > Scripted it to run 20 times without any failures. > Command-line: broker-upgrade/bin/test.sh <dir1> <dir2> > > > Thanks, > > Abhishek Nigam > >
