cmd/juju: simplify testing infrastructure.

cmd/juju: simplify testing infrastructure.

The deploy tests were running 4 times each!

https://code.launchpad.net/~rogpeppe/juju-core/033-cmd-juju-test-refactor/+merge/120966

(do not edit description out of merge proposal)

[rog, 2012-08-23 12:44:12]

Please take a look.

[niemeyer, 2012-08-27 13:29:10]

Nice cleanups!

A few suggestions and one question to understand the direction we're going, and
LGTM.

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/addunit_test.go
File cmd/juju/addunit_test.go (right):

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/addunit_test.go#new...
cmd/juju/addunit_test.go:8: coretesting "launchpad.net/juju-core/testing"
Why renaming it here? If we're renaming it even when there are no conflicts, the
package seems misnamed.

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/cmd_test.go
File cmd/juju/cmd_test.go (right):

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/cmd_test.go#newcode24
cmd/juju/cmd_test.go:24: // JujuConnSuite.
// because ...

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go
File juju/testing/conn.go (right):

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode25
juju/testing/conn.go:25: // The name of the dummy environment is "jujutest".
Can we use "dummyenv" here, so one can tell it's a dummy environment being
played with? (everything is "jujutest", in a sense)

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode51
juju/testing/conn.go:51: environs.VarDir = c.MkDir()
How about having a temporary root instead, so we can have 

s.rootDir = c.MkDir()
$HOME = filepath.Join(s.rootDir, "/home/ubuntu")
VarDir = filepath.Join(s.rootDir, environs.VarDir)

Also, let's please use s.oldVarDir and s.oldHome for those names. Right now they
look like things one should be using in a test.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode56
juju/testing/conn.go:56: err = ioutil.WriteFile(filepath.Join(home, ".juju",
"environments.yaml"), config, 0666)
0600

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode64
juju/testing/conn.go:64: c.Assert(conn.Bootstrap(false), IsNil)
How long does it take to run just JujuConnSutie.SetUpTest by itself today? I'm
curious because what's being done here sounds slightly heavy for a permanent
base.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode88
juju/testing/conn.go:88: err := ioutil.WriteFile(path, []byte(config), 0666)
0600

[rog, 2012-08-28 17:36:07]

Please take a look.

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/addunit_test.go
File cmd/juju/addunit_test.go (right):

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/addunit_test.go#new...
cmd/juju/addunit_test.go:8: coretesting "launchpad.net/juju-core/testing"
On 2012/08/27 13:29:10, niemeyer wrote:
> Why renaming it here? If we're renaming it even when there are no conflicts,
the
> package seems misnamed.

there was an intermediate state where both packages were imported. done.

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/cmd_test.go
File cmd/juju/cmd_test.go (right):

https://codereview.appspot.com/6484051/diff/4001/cmd/juju/cmd_test.go#newcode24
cmd/juju/cmd_test.go:24: // JujuConnSuite.
On 2012/08/27 13:29:10, niemeyer wrote:
> // because ...

because ... i forgot to delete the comment, which no longer applies.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go
File juju/testing/conn.go (right):

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode25
juju/testing/conn.go:25: // The name of the dummy environment is "jujutest".
On 2012/08/27 13:29:10, niemeyer wrote:
> Can we use "dummyenv" here, so one can tell it's a dummy environment being
> played with? (everything is "jujutest", in a sense)

Done.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode51
juju/testing/conn.go:51: environs.VarDir = c.MkDir()
On 2012/08/27 13:29:10, niemeyer wrote:
> How about having a temporary root instead, so we can have 
> 
> s.rootDir = c.MkDir()
> $HOME = filepath.Join(s.rootDir, "/home/ubuntu")
> VarDir = filepath.Join(s.rootDir, environs.VarDir)
> 
> Also, let's please use s.oldVarDir and s.oldHome for those names. Right now
they
> look like things one should be using in a test.

Done.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode56
juju/testing/conn.go:56: err = ioutil.WriteFile(filepath.Join(home, ".juju",
"environments.yaml"), config, 0666)
On 2012/08/27 13:29:10, niemeyer wrote:
> 0600

i'm not sure. 0666 is the usual default for file creation. it's what the shell
and os.Create use. if the person running the tests cares, they can set umask.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode64
juju/testing/conn.go:64: c.Assert(conn.Bootstrap(false), IsNil)
On 2012/08/27 13:29:10, niemeyer wrote:
> How long does it take to run just JujuConnSutie.SetUpTest by itself today? I'm
> curious because what's being done here sounds slightly heavy for a permanent
> base.

The first time it's called, it takes around 5s (waiting for zk to come up). Each
subsequent time, it takes approx 40ms.

Are you concerned about the cost of creating the temporary directory and writing
the environments.yaml file?

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode88
juju/testing/conn.go:88: err := ioutil.WriteFile(path, []byte(config), 0666)
On 2012/08/27 13:29:10, niemeyer wrote:
> 0600

as above.

[rog, 2012-08-29 13:16:20]

Please take a look.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go
File juju/testing/conn.go (right):

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode56
juju/testing/conn.go:56: err = ioutil.WriteFile(filepath.Join(home, ".juju",
"environments.yaml"), config, 0666)
On 2012/08/27 13:29:10, niemeyer wrote:
> 0600

Done.

https://codereview.appspot.com/6484051/diff/4001/juju/testing/conn.go#newcode88
juju/testing/conn.go:88: err := ioutil.WriteFile(path, []byte(config), 0666)
On 2012/08/27 13:29:10, niemeyer wrote:
> 0600

Done.

[niemeyer, 2012-08-29 13:29:30]

LGTM

[rog, 2012-08-29 13:32:12]

*** Submitted:

cmd/juju: simplify testing infrastructure.

The deploy tests were running 4 times each!

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