environs/ec2: several live tests fixes

environs/ec2: several live tests fixes

This should get tests working, or pretty close to working.

Time's up today.

https://code.launchpad.net/~niemeyer/juju-core/live-tests-fixes/+merge/126366

(do not edit description out of merge proposal)

[niemeyer, 2012-09-26 02:24:56]

Please take a look.

[dave_cheney.net, 2012-09-26 02:45:15]

On 2012/09/26 02:24:56, niemeyer wrote:
> Please take a look.

Everything but jujutest/livetests.go looks good. livetests doesn't look good,
but that isn't your fault.

[niemeyer, 2012-09-26 02:52:36]

Okay, I'm submitting this as it's surely a step forward. I got the
BootstrapAndDeploy passing til the last bits at least once, so it should help
people tomorrow before I get it.

[dave_cheney.net, 2012-09-26 05:11:20]

Some comments that I missed.

https://codereview.appspot.com/6567048/diff/1/environs/ec2/live_test.go
File environs/ec2/live_test.go (left):

https://codereview.appspot.com/6567048/diff/1/environs/ec2/live_test.go#oldco...
environs/ec2/live_test.go:109: 
LGTM. I wonder why someone thought they should use Check here.

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go
File environs/jujutest/livetests.go (right):

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go#...
environs/jujutest/livetests.go:218: // 5. Provisioner removes dead machine
Can you add a line explaining why the above isn't/cannot be done right now.

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go#...
environs/jujutest/livetests.go:291: c.Assert(ok, Equals, true, Commentf("watcher
%d died: %v", i, w.Err()))
Should this move up two lines, if the channel is closed, you'll get machine 0

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go#...
environs/jujutest/livetests.go:379: c.Assert(err, IsNil)
err is shadowed by the inner loop, err here will never be non nil, you can drop
this test.

[niemeyer, 2012-09-26 18:01:22]

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go
File environs/jujutest/livetests.go (right):

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go#...
environs/jujutest/livetests.go:218: // 5. Provisioner removes dead machine
On 2012/09/26 05:11:20, dfc wrote:
> Can you add a line explaining why the above isn't/cannot be done right now.

I don't think there is any reason why it cannot be done. It isn't done just
because no one did it yet, but we should get there soon.

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go#...
environs/jujutest/livetests.go:291: c.Assert(ok, Equals, true, Commentf("watcher
%d died: %v", i, w.Err()))
On 2012/09/26 05:11:20, dfc wrote:
> Should this move up two lines, if the channel is closed, you'll get machine 0

There's no machine id in the message.

https://codereview.appspot.com/6567048/diff/1/environs/jujutest/livetests.go#...
environs/jujutest/livetests.go:379: c.Assert(err, IsNil)
On 2012/09/26 05:11:20, dfc wrote:
> err is shadowed by the inner loop, err here will never be non nil, you can
drop
> this test.

Will do, thanks.