Skip to content

Use nil instead of len for arg check.#224

Closed
jonreyna wants to merge 1 commit into
spf13:masterfrom
jonreyna:nilArgsCheck
Closed

Use nil instead of len for arg check.#224
jonreyna wants to merge 1 commit into
spf13:masterfrom
jonreyna:nilArgsCheck

Conversation

@jonreyna
Copy link
Copy Markdown
Contributor

The fix in #155 for go test -v breaks SetArgs().

package main

import (
    "fmt"
    "strings"

    "github.com/spf13/cobra"
)

func main() {

    var cmdPrint = &cobra.Command{
        Use:   "print [string to print]",
        Short: "Print anything to the screen",
        Long: `print is for printing anything back to the screen.
        For many years people have printed back to the screen.
        `,
        Run: func(cmd *cobra.Command, args []string) {
            fmt.Println("Print: " + strings.Join(args, " "))
        },
    }

    cmdPrint.SetArgs([]string{})

    cmdPrint.Execute()
}
meddling_monk at BorgCube in /media/meddling_monk/Bear/dev/golang/src/github.com/JReyLBC/testCobra 
$ go run test_cobra1.go abc
Print: abc

Checking if args is nil, instead of having a zero length, honors the use of SetArgs(), and doesn't break go test -v.

This honors the SetArgs() call
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
All committers have accepted the CLA.

squaremo added a commit to weaveworks-experiments/flux-classic that referenced this pull request Jan 19, 2016
 - ability to tap (and thereby test) the stdout and stderr, at the
   mild cost of having to replace uses of fmt.Print{ln,f}. Use it to
   test the output of `fluxctl list`

 - convert all subcommands to use the `RunE` style of command, which
   makes them easier to test (since they return an error rather than
   panicking or worse, exiting altogether)

 - add a test to the helpers, so that it imports "testing" (apparently
   important for the coverage tool), and so there's a test

 - work around a bug in cobra.Command, whereby supplying an empty
   slice to `cmd.SetArgs(...)` makes it fall back to `os.Args[1:]`,
   breaking the coverage tooling. (See
   spf13/cobra#224)
@spf13
Copy link
Copy Markdown
Owner

spf13 commented Feb 8, 2016

Thanks for the fix. Merged as 1ef0913

@spf13 spf13 closed this Feb 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants