Isolate ssh tests with FakeHomeSuite

Isolate ssh tests with FakeHomeSuite

Use the FakeHomeSuite to isolate
provider/manual tests which require
ssh keys.

https://code.launchpad.net/~waigani/juju-core/1299969-isolate-ssh-tests/+merge/214850

(do not edit description out of merge proposal)

[waigani, 2014-04-08 22:41:52]

Please take a look.

[thumper, 2014-04-09 05:14:31]

https://codereview.appspot.com/85710043/diff/1/environs/config/authkeys_test.go
File environs/config/authkeys_test.go (right):

https://codereview.appspot.com/85710043/diff/1/environs/config/authkeys_test....
environs/config/authkeys_test.go:28: // Use authkeys.AuthKeysSuite
wat?

https://codereview.appspot.com/85710043/diff/1/provider/manual/config_test.go
File provider/manual/config_test.go (right):

https://codereview.appspot.com/85710043/diff/1/provider/manual/config_test.go...
provider/manual/config_test.go:27: s.AuthKeysSuite.SetUpTest(c)
there is no default upcalling, if you contain two suites, you need to call setup
on both of them.

https://codereview.appspot.com/85710043/diff/1/provider/manual/environ_test.go
File provider/manual/environ_test.go (right):

https://codereview.appspot.com/85710043/diff/1/provider/manual/environ_test.g...
provider/manual/environ_test.go:36: s.AuthKeysSuite.SetUpTest(c)
This was also missing the SetUpTest for s.LoggingSuite.

https://codereview.appspot.com/85710043/diff/1/provider/manual/environ_test.g...
provider/manual/environ_test.go:43: s.AuthKeysSuite.TearDownTest(c)
need to tear down both two. tear down in the opposite order to setup.

perhaps better to have AuthKeysSuite to contain LoggingSuite?

https://codereview.appspot.com/85710043/diff/1/provider/manual/provider_test.go
File provider/manual/provider_test.go (right):

https://codereview.appspot.com/85710043/diff/1/provider/manual/provider_test....
provider/manual/provider_test.go:31: s.AuthKeysSuite.SetUpTest(c)
same problem as the other files.

https://codereview.appspot.com/85710043/diff/1/testing/authkeys/authkeys.go
File testing/authkeys/authkeys.go (right):

https://codereview.appspot.com/85710043/diff/1/testing/authkeys/authkeys.go#n...
testing/authkeys/authkeys.go:19: testbase.LoggingSuite
Oh? it is here.  In which case you should be removing it from the other suites
that include this one.

https://codereview.appspot.com/85710043/diff/1/testing/authkeys/authkeys.go#n...
testing/authkeys/authkeys.go:19: testbase.LoggingSuite
Would it make more sense to use the testing.FakeHomeSuite?

https://codereview.appspot.com/85710043/diff/1/testing/authkeys/authkeys_test.go
File testing/authkeys/authkeys_test.go (right):

https://codereview.appspot.com/85710043/diff/1/testing/authkeys/authkeys_test...
testing/authkeys/authkeys_test.go:14: type authKeysSuite struct {
You don't need this entire file.

[waigani, 2014-04-10 00:25:24]

Please take a look.

https://codereview.appspot.com/85710043/diff/1/environs/config/authkeys_test.go
File environs/config/authkeys_test.go (right):

https://codereview.appspot.com/85710043/diff/1/environs/config/authkeys_test....
environs/config/authkeys_test.go:28: // Use authkeys.AuthKeysSuite
On 2014/04/09 05:14:32, thumper wrote:
> wat?

There is a lot of duplication between this file and authkeys.AuthKeysSuite. So
why not use AuthKeysSuite and then just add/remove keys per test?

[dave_cheney.net, 2014-04-10 01:04:54]

not lgtm. Where is the code for {Fake}AuthSuite ? Does this branch had a
prerequisite ?

[waigani, 2014-04-10 01:58:33]

On 2014/04/10 01:04:54, dfc wrote:
> not lgtm. Where is the code for {Fake}AuthSuite ? Does this branch had a
> prerequisite ?

Sorry, description should read "FakeHomeSuite". Editing now...

[waigani, 2014-04-10 02:02:15]

Please take a look.

[dave_cheney.net, 2014-04-10 02:04:22]

LGTM. Thanks, that makes more sense.

[thumper, 2014-04-10 02:20:27]

NOT LGTM yet

https://codereview.appspot.com/85710043/diff/40001/provider/manual/config_tes...
File provider/manual/config_test.go (right):

https://codereview.appspot.com/85710043/diff/40001/provider/manual/config_tes...
provider/manual/config_test.go:23: func (s *configSuite) SetupTest(c *gc.C) {
delete this method, it isn't needed.

https://codereview.appspot.com/85710043/diff/40001/provider/manual/environ_te...
File provider/manual/environ_test.go (right):

https://codereview.appspot.com/85710043/diff/40001/provider/manual/environ_te...
provider/manual/environ_test.go:25: testbase.LoggingSuite
remove this logging suite

https://codereview.appspot.com/85710043/diff/40001/provider/manual/provider_t...
File provider/manual/provider_test.go (right):

https://codereview.appspot.com/85710043/diff/40001/provider/manual/provider_t...
provider/manual/provider_test.go:29: s.LoggingSuite.SetUpTest(c)
delete this line

[waigani, 2014-04-10 02:32:35]

Please take a look.

[thumper, 2014-04-10 10:45:37]

On 2014/04/10 02:32:35, waigani wrote:
> Please take a look.

LGTM