testing: add ZkRemoveTree function.

It's useful for cleaning zk state between tests and it was in danger of
having three separate implementations.

https://code.launchpad.net/~rogpeppe/juju/go-testing-remove-tree/+merge/97865

(do not edit description out of merge proposal)

[rog, 2012-03-16 12:51:52]

Please take a look.

[fwereade, 2012-03-16 13:03:40]

LGTM apart from deleting "/", which I think we really should be able to do.

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go
File testing/zk.go (left):

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go#oldcode16
testing/zk.go:16: func StartZkServer(t TestingT) *zookeeper.Server {
As discussed on IRC, TestingT should be renamed to Fatalfer.

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go#newcode44
testing/zk.go:44: // calling zkRemoveTree(zk, "/")) we silently ignore it.
ZkRemoveTree

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go#newcode60
testing/zk.go:60: err = zk.Delete(path, -1)
Can we really delete "/" without getting an error?

[rog, 2012-03-16 13:09:19]

Please take a look.

[rog, 2012-03-16 13:10:58]

Please take a look.

[rog, 2012-03-16 13:12:01]

Please take a look.

[rog, 2012-03-16 13:12:04]

PTAL

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go#newcode44
testing/zk.go:44: // calling zkRemoveTree(zk, "/")) we silently ignore it.
On 2012/03/16 13:03:40, fwereade wrote:
> ZkRemoveTree

Done.

https://codereview.appspot.com/5841047/diff/2001/testing/zk.go#newcode60
testing/zk.go:60: err = zk.Delete(path, -1)
On 2012/03/16 13:03:40, fwereade wrote:
> Can we really delete "/" without getting an error?

Done.

[fwereade, 2012-03-16 13:48:44]

LGTM

[niemeyer, 2012-03-16 18:04:26]

LGTM, but please fix the docs or implement what it suggests.

https://codereview.appspot.com/5841047/diff/7001/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5841047/diff/7001/testing/zk.go#newcode64
testing/zk.go:64: if err != nil {
"and it does not consider deleting a nonexistent node to be an error."?

[rog, 2012-03-16 18:15:35]

https://codereview.appspot.com/5841047/diff/7001/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5841047/diff/7001/testing/zk.go#newcode64
testing/zk.go:64: if err != nil {
On 2012/03/16 18:04:26, niemeyer wrote:
> "and it does not consider deleting a nonexistent node to be an error."?

we should never get here if the node does not exist, because zk.Children will
fail above.

if we're running two ZkRemoveTrees concurrently, i think we deserve what we get.

[rog, 2012-03-16 18:36:55]

Please take a look.

[rog, 2012-03-16 18:38:45]

*** Submitted:

testing: add ZkRemoveTree function.

It's useful for cleaning zk state between tests and it was in danger of
having three separate implementations.

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/5841047