Run tests against service doubles

Run tests against service doubles

This branch adds support run running the Openstack tests against service doubles instead of a live instance.
We want the tests against the service doubles to always run, and the tests against a live instance to run if specified using -live.

Tests which do not pass yet (some code is still WIP), are skipped.

https://code.launchpad.net/~wallyworld/juju-core/goose-test-doubles/+merge/143442

Requires: https://code.launchpad.net/~wallyworld/juju-core/upload-fake-tools/+merge/143228

(do not edit description out of merge proposal)

[wallyworld, 2013-01-16 06:53:58]

Please take a look.

[wallyworld, 2013-01-17 06:07:51]

Please take a look.

[dimitern, 2013-01-17 17:02:58]

LGTM, we should go ahead and land this, before my bootstrap tests get even more
complicated :)

[rog, 2013-01-17 17:22:14]

looks reasonable. a few comments below.

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/local_tes...
File environs/openstack/local_test.go (right):

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/local_tes...
environs/openstack/local_test.go:54: s.identityDouble =
identityservice.NewUserPass()
rather than assigning the UserPass to identityDouble and then dynamically
converting back in the next line, it would probably be best to do something
like:

userSvc := identityservice.NewUserPass()
token := userSvc.AddUser(s.cred.User, s.cred.Secrets)
s.Mux.Handle(baseIdentityURL, userSvc)
s.identityDouble = userSvc

then you can continue using userSvc below, instead of converting from
identityDouble each time.

alternatively, you could simply define identityDouble as
*identityservice.UserPass inside localLiveSuite, which may well be better still.

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/local_tes...
environs/openstack/local_test.go:89: if s.Server != nil {
s.Server cannot be nil here.

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/provider_...
File environs/openstack/provider_test.go (right):

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/provider_...
environs/openstack/provider_test.go:13: var local = flag.Bool("local", false,
"Include local OpenStack (service double) tests")
rather than adding a command line flag, it's probably better to skip the
relevant tests (you can use c.Skip to do that) and remove the skips when the
code starts working. that way, it's easy to reenable specific tests when
desired.

[gz, 2013-01-17 17:28:57]

Looks good overall, a couple of queries.

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/local_tes...
File environs/openstack/local_test.go (right):

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/local_tes...
environs/openstack/local_test.go:15: baseNovaURL     = "/V1/1"
Why this form rather than just "/compute" or similar to swift?

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/provider_...
File environs/openstack/provider_test.go (right):

https://codereview.appspot.com/7133043/diff/2002/environs/openstack/provider_...
environs/openstack/provider_test.go:13: var local = flag.Bool("local", false,
"Include local OpenStack (service double) tests")
Skips in the tests that don't pass yet does seem like the neatest way, when a
subset of them are ok.

[wallyworld, 2013-01-18 01:29:22]

I've addressed the identityservice issues raised.
 
> 
>
https://codereview.appspot.com/7133043/diff/2002/environs/openstack/provider_...
> environs/openstack/provider_test.go:13: var local = flag.Bool("local", false,
> "Include local OpenStack (service double) tests")
> rather than adding a command line flag, it's probably better to skip the
> relevant tests (you can use c.Skip to do that) and remove the skips when the
> code starts working. that way, it's easy to reenable specific tests when
> desired.

I considered skipping tests but there's issues with that. The tests which fail
are from aggregated from existing jujutest module and these same tests are also
run for the ec2 side of things. There's also different failures depending on
whether the tests are run using the service doubles or the live openstack
instance. The service doubles are being fixed to address this anomaly . Plus,
there's no easy correlation between further openstack provider implementation
work and test success, so it's difficult to easily know which tests to unskip
following some work being done.

So given all of the above, I think the current approach is best for now. As soon
as possible, we would look to remove the -local flag.

[wallyworld, 2013-01-18 01:56:13]

>
https://codereview.appspot.com/7133043/diff/2002/environs/openstack/local_tes...
> environs/openstack/local_test.go:15: baseNovaURL     = "/V1/1"
> Why this form rather than just "/compute" or similar to swift?
>

The "/V1/1" bit is the last part of a nova endpoint url (the bit appended to the
hostname),  like
https://nova-lcy01.canonistack.canonical.com/v2/97dca28143f744b19713bb901094a59d

The reason it needs to be specified for the nova service double and not the
swift double is that the nova service double has been written differently to the
swift double in how the http server is set up. Which way is better can be
discussed (I didn't write either so can't offer an authorative view). My
personal opinion is that the nova double is more correct since the urls it uses
match the format of the real openstack urls and if you have scripts using curl
etc, they will work with the nova double.

>
https://codereview.appspot.com/7133043/diff/2002/environs/openstack/provider_...
> File environs/openstack/provider_test.go (right):
> 
>
https://codereview.appspot.com/7133043/diff/2002/environs/openstack/provider_...
> environs/openstack/provider_test.go:13: var local = flag.Bool("local", false,
> "Include local OpenStack (service double) tests")
> Skips in the tests that don't pass yet does seem like the neatest way, when a
> subset of them are ok.

See my reply to rog

[rog, 2013-01-18 09:27:15]

On 18 January 2013 01:29,  <ian.booth@canonical.com> wrote:
> I considered skipping tests but there's issues with that. The tests
> which fail are from aggregated from existing jujutest module and these
> same tests are also run for the ec2 side of things. There's also
> different failures depending on whether the tests are run using the
> service doubles or the live openstack instance.

It should be straightforward to skip tests in all these cases.
I would not add Skip calls in jujutest at all, just implement
the test methods you want to skip on the relevant local type,
with a body that consists of a single Skip statement.

for instance if you wanted to skip TestBootstrap for the test
double and nothing else, you could write:

func (s *localLiveSuite) TestBootstrap(c *C) {
    c.Skip()
}

> Plus, there's no easy correlation
> between further openstack provider implementation work and test success,
> so it's difficult to easily know which tests to unskip following some
> work being done.

When you change something in the provider, comment out
all the skips and see what passes. Then unskip the ones that
pass.

[wallyworld, 2013-01-22 00:31:39]

*** Submitted:

Run tests against service doubles

This branch adds support run running the Openstack tests against service doubles
instead of a live instance.
We want the tests against the service doubles to always run, and the tests
against a live instance to run if specified using -live.

Tests which do not pass yet (some code is still WIP), are skipped.

R=dimitern, rog, gz
CC=
https://codereview.appspot.com/7133043