Refactor environs/openstack/config_test.go

Refactor environs/openstack/config_test.go

Clean up OpenStack config_test.go to define all the tests using configTests array, including those that override environment variables.

https://code.launchpad.net/~danilo/juju-core/refactor-openstack-config-tests/+merge/157923

(do not edit description out of merge proposal)

[Danilo, 2013-04-09 17:22:42]

Please take a look.

[dimitern, 2013-04-09 17:41:55]

Nice, LGTM modulo the trivials below.
(Since you're new, just a short explanation, if you need it: when someone says
"LGTM with/modulo/assuming xyz" it *usually* means - I trust you to make the
suggested changes and not ask me again; unless of course you don't agree on some
of them and there's a discussion after that).

As discussed, please add:
// TODO(danilos) Fix these tests to save/restore env vars, like in openstack
config_test
so we won't forget it.

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test.go
File environs/openstack/config_test.go (right):

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test....
environs/openstack/config_test.go:161: 
d (=delete this line)

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test....
environs/openstack/config_test.go:269: "auth-mode": "invalid-mode",
see below - here as well (const, not literal)

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test....
environs/openstack/config_test.go:303: authMode:   "legacy",
see below; also make this use the actual constant from goose, not a literal,
please.

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test....
environs/openstack/config_test.go:320: authMode: string(AuthUserPass),
make this typed the same as AuthUserPass, so you won't need to cast it.

[niemeyer, 2013-04-09 18:18:16]

LGTM assuming the following changes:

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test.go
File environs/openstack/config_test.go (right):

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test....
environs/openstack/config_test.go:93: }
defer restoreEnvVars(savedVars)

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test....
environs/openstack/config_test.go:114: restoreEnvVars(savedVars)
d

https://codereview.appspot.com/8584043/diff/1/environs/openstack/config_test....
environs/openstack/config_test.go:116: } else {
Drop the whole branch too.

[Danilo, 2013-04-09 18:19:06]

As we discussed on IRC, it's not as simple as using the right type. It compiles,
but tests do not pass since config.go returns a string in the config object, not
an actual AuthMode:

config_test.go:130:
    c.Assert(ecfg.authMode(), Equals, t.authMode)
... obtained string = "userpass"
... expected openstack.AuthMode = "userpass"

[Danilo, 2013-04-09 18:37:34]

*** Submitted:

Refactor environs/openstack/config_test.go

Clean up OpenStack config_test.go to define all the tests using configTests
array, including those that override environment variables.

R=dimitern, niemeyer
CC=
https://codereview.appspot.com/8584043