cmd/juju: status: add testing skeleton

cmd/juju: status: add testing skeleton

Nothing here to test (yet), presented for comment
and review in the spirit of producing smaller diffs.

https://code.launchpad.net/~dave-cheney/juju-core/go-cmd-juju-status-machines/+merge/115649

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-07-19 05:52:20]

Please take a look.

[dave_cheney.net, 2012-07-19 06:20:41]

Please take a look.

[niemeyer, 2012-07-19 21:58:07]

LGTM

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status.go#newcode42
cmd/juju/status.go:42: Machines map[int]interface{}
We'll eventually need tags in that struct, to match the original names. May be
worth putting it in a named type outside of the function too.

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status_test.go
File cmd/juju/status_test.go (right):

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:46: f        func(*state.State) error
s/f/mutate/ (or prepare, or something else more telling)

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:51: // machine/0 bootstrapped.
Didn't we just introduce that race in that other branch? ;-)

[dave_cheney.net, 2012-07-20 01:01:16]

Thanks for your comments, i'm putting this back in WIP for the moment.

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status.go#newcode42
cmd/juju/status.go:42: Machines map[int]interface{}
On 2012/07/19 21:58:07, niemeyer wrote:
> We'll eventually need tags in that struct, to match the original names. May be
> worth putting it in a named type outside of the function too.

Yup - especially if it has to support json and yaml (and other formats)

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status.go#newcode42
cmd/juju/status.go:42: Machines map[int]interface{}
On 2012/07/19 21:58:07, niemeyer wrote:
> We'll eventually need tags in that struct, to match the original names. May be
> worth putting it in a named type outside of the function too.

Yup, as it will need to support the tags for json and yaml

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status_test.go
File cmd/juju/status_test.go (right):

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:46: f        func(*state.State) error
On 2012/07/19 21:58:07, niemeyer wrote:
> s/f/mutate/ (or prepare, or something else more telling)

Done.

https://codereview.appspot.com/6432045/diff/3004/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:51: // machine/0 bootstrapped.
On 2012/07/19 21:58:07, niemeyer wrote:
> Didn't we just introduce that race in that other branch? ;-)

I wouldn't call this a race,  just a test of running juju status on an empty
state. In real life it is difficult to achieve this as the state will be hosted
on machine/0, so there will be one machine in the state. This assertion may not
hold true for the local provider.

[dave_cheney.net, 2012-07-20 02:02:52]

Please take a look.

[niemeyer, 2012-07-20 02:28:19]

LGTM, but please consider the following suggestions:

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status.go#newcode38
cmd/juju/status.go:38: result := struct {
This would be more readable as a properly named type out of Run.

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status_test.go
File cmd/juju/status_test.go (right):

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status_test.go#new...
cmd/juju/status_test.go:49: output  map[string]string
It's easier to maintain and to read this as a single map[string]interface{}
value that reflects the actual output. Maintaining those by-hand yaml and json
strings will be completely unmanageable once these maps depart from the trivial.

[dave_cheney.net, 2012-07-20 05:39:04]

*** Submitted:

cmd/juju: status: add testing skeleton

Nothing here to test (yet), presented for comment
and review in the spirit of producing smaller diffs.

R=niemeyer
CC=
https://codereview.appspot.com/6432045

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status.go#newcode38
cmd/juju/status.go:38: result := struct {
On 2012/07/20 02:28:19, niemeyer wrote:
> This would be more readable as a properly named type out of Run.

Done.

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status_test.go
File cmd/juju/status_test.go (right):

https://codereview.appspot.com/6432045/diff/10001/cmd/juju/status_test.go#new...
cmd/juju/status_test.go:49: output  map[string]string
On 2012/07/20 02:28:19, niemeyer wrote:
> It's easier to maintain and to read this as a single map[string]interface{}
> value that reflects the actual output. Maintaining those by-hand yaml and json
> strings will be completely unmanageable once these maps depart from the
trivial.

Unfortunately this is not possible. goayml and json decoders produce different
generic map types, json produces map[string]interface{}, yaml produces
map[interface{}]interface{}.