Fix relation-list with no relations

Fix relation-list with no relations

lp:1182224 has relation-list returning null with json output if there are
no relations.  This was due to the returning of an uninitialized slice.

There was a test for this but it was using a test context that behaved
differently.  The test context has been updated, and the command fixed.

https://code.launchpad.net/~thumper/juju-core/fix-uniter-context-relation-names/+merge/165263

(do not edit description out of merge proposal)

[thumper, 2013-05-22 22:58:55]

Please take a look.

[dave_cheney.net, 2013-05-23 01:31:51]

LGTM. You can take or leave my comments.

https://codereview.appspot.com/9677044/diff/1/worker/uniter/context.go
File worker/uniter/context.go (right):

https://codereview.appspot.com/9677044/diff/1/worker/uniter/context.go#newcod...
worker/uniter/context.go:309: func (ctx *ContextRelation) UnitNames() (units
[]string) {
drop the named return param

https://codereview.appspot.com/9677044/diff/1/worker/uniter/context.go#newcod...
worker/uniter/context.go:310: units = []string{}
does this need a comment ? otherwise someone might try to rewrite it as

var units []string

[wallyworld, 2013-05-23 01:33:05]

LGTM, agree with Dave's comments

[niemeyer, 2013-05-23 02:38:35]

https://codereview.appspot.com/9677044/diff/1/worker/uniter/context.go
File worker/uniter/context.go (right):

https://codereview.appspot.com/9677044/diff/1/worker/uniter/context.go#newcod...
worker/uniter/context.go:310: units = []string{}
On 2013/05/23 01:31:51, dfc wrote:
> does this need a comment ? otherwise someone might try to rewrite it as
> 
> var units []string

This fix seems to be in the wrong spot. It's idiomatic to return nil values in
Go to mean len(slice) == 0. Rather than commenting that the common idiom is
being broken, I suggest fixing the issue where it actually makes it clear why
the given logic is used, replacing

        return c.out.Write(ctx, r.UnitNames())

by

        unitNames := r.UnitNames()
        if unitNames == nil {
            unitNames = []string{}
        }
        return c.out.Write(ctx, unitNames)

and testing the fact that relation-list itself works properly with the empty
relation names, which is the real bug.

[thumper, 2013-05-23 03:35:55]

On 2013/05/23 02:38:35, niemeyer wrote:
> https://codereview.appspot.com/9677044/diff/1/worker/uniter/context.go
> File worker/uniter/context.go (right):
> 
>
https://codereview.appspot.com/9677044/diff/1/worker/uniter/context.go#newcod...
> worker/uniter/context.go:310: units = []string{}
> On 2013/05/23 01:31:51, dfc wrote:
> > does this need a comment ? otherwise someone might try to rewrite it as
> > 
> > var units []string
> 
> This fix seems to be in the wrong spot. It's idiomatic to return nil values in
> Go to mean len(slice) == 0. Rather than commenting that the common idiom is
> being broken, I suggest fixing the issue where it actually makes it clear why
> the given logic is used, replacing
> 
>         return c.out.Write(ctx, r.UnitNames())
> 
> by
> 
>         unitNames := r.UnitNames()
>         if unitNames == nil {
>             unitNames = []string{}
>         }
>         return c.out.Write(ctx, unitNames)
> 
> and testing the fact that relation-list itself works properly with the empty
> relation names, which is the real bug.

Fair enough.  However I went hunting for the tests, and was somewhat surprised
to see a test for an empty relation, and that it was successfully returning "[]"
for json.

However it was a testing context implemented exactly how I wrote the above fix. 
I have updated the testing context to match reality, and added the fix into the
actual command.

[thumper, 2013-05-23 03:37:39]

Please take a look.

[wallyworld, 2013-05-23 03:47:30]

LGTM

[thumper, 2013-05-23 04:51:24]

*** Submitted:

Fix relation-list with no relations

lp:1182224 has relation-list returning null with json output if there are
no relations.  This was due to the returning of an uninitialized slice.

There was a test for this but it was using a test context that behaved
differently.  The test context has been updated, and the command fixed.

R=dfc, wallyworld, niemeyer
CC=
https://codereview.appspot.com/9677044