Hi Kishore,
This looks great!
I'll plan to use the zookeeper integration in some tests at the application
level, i.e. in the core subproject (probably extending existing ones). I'll
try to provide more feedback then.
A few comments:
- zkclient: I've noticed you are using an external client library for
avoiding quite a bit of boilerplate code when using zookeeper. I had already
added things to facilitate usage of zookeeper in core/TestUtils, but if
zkclient really "makes life easier", as it says, and if you recommend using
it, I'll refactor my own code to use that client. Looks nice!
- junit version: it seems you are using the api from junit3. I think it's
better to use junit 4, since there is already such a dependency in existing
tests. There are also more features available.
- zookeeper data directories in tests: you seem to be doing like myself in
my initialization code for zookeeper fixtures, i.e. using directories in
{user.dir}/tmp . Leo suggested to use java.io.tmpdir... so we should both
use that directory!
- test package names: I started using prefixing by test.s4, in order to
avoid any confusion with other classes from code. I'm ok for prefixing tests
with org.apache.s4, but I think it should at least be org.apache.s4.test.
Which naming scheme do we choose?
- The design is very similar to what we had in earlier version of s4. There
will be more changes on this in the next release.
--> you are referring to release 0.5 right?
Matthieu
On Sat, Oct 22, 2011 at 7:31 PM, kishore g <g.kish...@gmail.com> wrote:
> Hi,
>
> I pushed in the initial code for s4-zk integration for s4-piper.
>
> https://github.com/leoneu/s4-piper/compare/17a2547499...9ba3013a01
>
> Please review and provide feedback.
>
> The design is very similar to what we had in earlier version of s4. There
> will be more changes on this in the next release.
>
> thanks,
> Kishore G
>
