Move ZK test server setup into its own package

...because other packages will want to test against real States, and I don't
want to duplicate the code.

https://code.launchpad.net/~fwereade/juju/go-testutil/+merge/96541

(do not edit description out of merge proposal)

[fwereade, 2012-03-08 10:50:57]

Please take a look.

[fwereade, 2012-03-08 14:20:31]

Please take a look.

[fwereade, 2012-03-08 14:38:40]

Please take a look.

[niemeyer, 2012-03-12 19:47:53]

On 2012/03/08 14:38:40, fwereade wrote:
> Please take a look.

There's something going wrong with this change set. Clicking on "View" on any of
the _test files yields "error: old chunk mismatch". :-(

[fwereade, 2012-03-12 23:04:57]

Please take a look.

[niemeyer, 2012-03-12 23:20:46]

https://codereview.appspot.com/5786051/diff/9002/state/internal_test.go
File state/internal_test.go (right):

https://codereview.appspot.com/5786051/diff/9002/state/internal_test.go#newco...
state/internal_test.go:22: _      = Suite(&TopologySuite{})
For some reason this doesn't look very nice as part of a var block. Maybe it
highlights the fact that we're really abusing the var syntax to get the Suite
function executed for registering the suite.

https://codereview.appspot.com/5786051/diff/9002/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/5786051/diff/9002/state/state_test.go#newcode22
state/state_test.go:22: state.ZkAddr = srv.Addr()
This looks a bit awkward. Can we maybe call it TestingZkAddr or similar, to
highlight the fact that this isn't a real variable of state for some en passant
reader?

https://codereview.appspot.com/5786051/diff/9002/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5786051/diff/9002/testing/zk.go#newcode16
testing/zk.go:16: dir, err := ioutil.TempDir("", "test-zk")
Where is this directory removed?

[fwereade, 2012-03-13 09:41:25]

https://codereview.appspot.com/5786051/diff/9002/state/internal_test.go
File state/internal_test.go (right):

https://codereview.appspot.com/5786051/diff/9002/state/internal_test.go#newco...
state/internal_test.go:22: _      = Suite(&TopologySuite{})
On 2012/03/12 23:20:46, niemeyer wrote:
> For some reason this doesn't look very nice as part of a var block. Maybe it
> highlights the fact that we're really abusing the var syntax to get the Suite
> function executed for registering the suite.

Done; would probably be good to move them to init()s but that'd be another
branch.

https://codereview.appspot.com/5786051/diff/9002/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/5786051/diff/9002/state/state_test.go#newcode22
state/state_test.go:22: state.ZkAddr = srv.Addr()
On 2012/03/12 23:20:46, niemeyer wrote:
> This looks a bit awkward. Can we maybe call it TestingZkAddr or similar, to
> highlight the fact that this isn't a real variable of state for some en
passant
> reader?

Yeah, much nicer. Done.

https://codereview.appspot.com/5786051/diff/9002/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5786051/diff/9002/testing/zk.go#newcode16
testing/zk.go:16: dir, err := ioutil.TempDir("", "test-zk")
On 2012/03/12 23:20:46, niemeyer wrote:
> Where is this directory removed?

By the deferred Destroy() in state_test.

(This is the other bit that depends on rog's changes in
https://codereview.appspot.com/5787068/ -- we can create the zk server in an
empty directory, rather than asking gozk to create a server dir in an empty dir
that then itself needs to be deleted separately.

[fwereade, 2012-03-13 09:42:46]

Please take a look.

[niemeyer, 2012-03-13 21:08:06]

LGTM, with only a few trivials:

https://codereview.appspot.com/5786051/diff/9002/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5786051/diff/9002/testing/zk.go#newcode16
testing/zk.go:16: dir, err := ioutil.TempDir("", "test-zk")
On 2012/03/13 09:41:25, fwereade wrote:
> On 2012/03/12 23:20:46, niemeyer wrote:
> > Where is this directory removed?
> 
> By the deferred Destroy() in state_test.
> 
> (This is the other bit that depends on rog's changes in
> https://codereview.appspot.com/5787068/ -- we can create the zk server in an
> empty directory, rather than asking gozk to create a server dir in an empty
dir
> that then itself needs to be deleted separately.

Can you please add a comment pointing out that the directory will be removed by
Destroy?

https://codereview.appspot.com/5786051/diff/9004/testing/zk.go
File testing/zk.go (right):

https://codereview.appspot.com/5786051/diff/9004/testing/zk.go#newcode21
testing/zk.go:21: if err != nil {
Must remove temporary directory here.

https://codereview.appspot.com/5786051/diff/9004/testing/zk.go#newcode25
testing/zk.go:25: if err != nil {
Must Destroy the server here.

[fwereade, 2012-03-15 12:05:53]

*** Submitted:

Move ZK test server setup into its own package

...because other packages will want to test against real States, and I don't
want to duplicate the code.

R=niemeyer
CC=
https://codereview.appspot.com/5786051