quintilesims / layer0

Build, Manage, and Deploy Your Applications
Apache License 2.0
44 stars 20 forks source link

CLI Command tests need to be refactored #536

Closed sesh-kebab closed 6 years ago

sesh-kebab commented 6 years ago

Problem

The tests for commands don't correctly use the Command to be tested. This results in defaults not being available within tests among other things.

Solution

// instead of:
command := NewEnvironmentCommand(base.Command())
...
c := testutils.NewTestContext(t, []string{"env_name"}, flags)
if err := command.create(c); err != nil {
    t.Fatal(err)
}

// do this
command := NewEnvironmentCommand(base.Command()).Command()
...
c := testutils.NewTestContext(t, []string{"environment", "create", "env_name"}, flags)
if err := command.Run(c); err != nil {
    t.Fatal(err)
}

This will require modifying NewTestContext to add arguments correctly.

zpatrick commented 6 years ago

(from discussion)

We think it would be best to approach these types of test from a higher level. For example, creating new helper function, RunTestCommand, which would create a testable *cli.App, give it the specified command cli.Command, and parse the input string and pass that into app.Run([]string{...})

func TestEnvironmentDelete(t *testing.T){
    ...
    command := NewEnvironmentCommand(mocks...)
    app := RunTestCommand(command.Command(), "l0 environment delete dev")
}

func TestEnvironmentCreateErrors {
    cases := map[string]string{
        "missing name arg": "l0 environment create",
        "invalid os": "l0 environment create --os windoows dev",
        ...
    }

    ...
}
zpatrick commented 6 years ago

possible implementation:

func NewTestApp(command cli.Command) *cli.App {
        app := cli.NewApp()
        app.Commands = []cli.Command{command}
        app.Writer = ioutil.Discard
        app.ErrWriter = ioutil.Discard

        return app
}

...tests would look something like:

func TestCreateEnvironment(t *testing.T) {
        // mocks

        app := NewTestApp(NewEnvironmentCommand(mocks...).Command())
        args := strings.Split("l0 environment create dev", " ")
        if err := app.Run(args); err != nil {
                t.Fatal(err)
        }
}

func TestCommandErrors(t *testing.T) {
        cases := map[string][]string{
                "bad os": strings.Split("l0 environment create --os pc dev", " "),
                "bad scale": strings.Split("l0 environment create --scale two dev", " "),
        }

        app := NewTestApp(Command())
        for name, args := range cases {
                t.Run(name, func(t *testing.T) {
                        if err := app.Run(args); err == nil {
                                t.Fatalf("Error was nil!")
                        }
                })
        }
}
diemonster commented 6 years ago

@zpatrick @tlake is this issue finished or not?