environs/configstore: memory-based store

environs/configstore: memory-based store

We'll use this for testing, and it's potentially useful
in other circumstances too.

https://code.launchpad.net/~rogpeppe/juju-core/404-mem-configstore/+merge/186643

(do not edit description out of merge proposal)

[rog, 2013-09-19 20:24:10]

Please take a look.

[natefinch, 2013-09-19 21:10:17]

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
File environs/configstore/interface_test.go (right):

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
environs/configstore/interface_test.go:22: s.NewStore(c)
What does this test exactly?  Other than the fact that NewStore isn't nil and
doesn't panic?

[natefinch, 2013-09-19 21:46:28]

LGTM.  If it were me, I'd probably make sequential asserts into checks (except
possibly the last check, if we need to stop the test).  But that's really more a
personal preference.

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
File environs/configstore/interface_test.go (right):

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
environs/configstore/interface_test.go:136: c.Assert(info1.APIEndpoint(),
gc.DeepEquals, environs.APIEndpoint{})
These four should probably all be c.Check, right?

[thumper, 2013-09-19 21:56:05]

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
File environs/configstore/interface_test.go (right):

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
environs/configstore/interface_test.go:136: c.Assert(info1.APIEndpoint(),
gc.DeepEquals, environs.APIEndpoint{})
On 2013/09/19 21:46:28, nate.finch wrote:
> These four should probably all be c.Check, right?

Why?  I don't see any issue with asserts

https://codereview.appspot.com/13489044/diff/2/environs/configstore/mem.go
File environs/configstore/mem.go (right):

https://codereview.appspot.com/13489044/diff/2/environs/configstore/mem.go#ne...
environs/configstore/mem.go:24: 
Perhaps add a line here

var _ environs.EnvironInfo = (*memInfo)(nil)

and similarly for environs.ConfigStorage.

[rog, 2013-09-19 22:00:36]

Please take a look.

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
File environs/configstore/interface_test.go (right):

https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
environs/configstore/interface_test.go:22: s.NewStore(c)
On 2013/09/19 21:10:18, nate.finch wrote:
> What does this test exactly?  Other than the fact that NewStore isn't nil and
> doesn't panic?

It was supposed to test that we could create the new
config storage, but you're right, it doesn't add anything much.
I'll delete it.

https://codereview.appspot.com/13489044/diff/2/environs/configstore/mem.go
File environs/configstore/mem.go (right):

https://codereview.appspot.com/13489044/diff/2/environs/configstore/mem.go#ne...
environs/configstore/mem.go:24: 
On 2013/09/19 21:56:05, thumper wrote:
> Perhaps add a line here
> 
> var _ environs.EnvironInfo = (*memInfo)(nil)
> 
> and similarly for environs.ConfigStorage.

Given that it's returned from CreateInfo etc, I don't
really see the point. The check is already done.

[natefinch, 2013-09-19 23:00:33]

On Sep 19, 2013 5:56 PM, <tim.penhey@canonical.com> wrote:
>
>
>
https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
> File environs/configstore/interface_test.go (right):
>
>
https://codereview.appspot.com/13489044/diff/2/environs/configstore/interface...
> environs/configstore/interface_test.go:136:
> c.Assert(info1.APIEndpoint(), gc.DeepEquals, environs.APIEndpoint{})
> On 2013/09/19 21:46:28, nate.finch wrote:
>>
>> These four should probably all be c.Check, right?
>
>
> Why?  I don't see any issue with asserts

Because then you'll see all four values if they fail, not just the first
one to fail. It just prevents you from fixing the first one, retesting,
fixing the second one, etc. Like I said, not a big deal, but that's the
nice thing about check - you get more info about the state than you would
from just the first failure stopping everything.