From 742cdb958b6bf32b48f812b74f8866d24850b0d9 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 25 Nov 2015 13:41:04 +0100 Subject: [PATCH] Add more command tests Signed-off-by: David Gageot --- commands/commands_test.go | 47 ++++------- commands/commandstest/fake_command_line.go | 3 + commands/commandstest/fake_lib_machine_api.go | 80 +++++++++++++++++++ .../commandstest/fake_machine_cli_client.go | 1 - commands/commandstest/stdout_capture.go | 54 +++++++++++++ commands/env_test.go | 15 +++- commands/ip_test.go | 45 +++++++++++ commands/kill_test.go | 55 +++++++++++++ commands/ls_test.go | 23 +----- commands/rm.go | 3 +- commands/rm_test.go | 49 ++++++++++++ commands/stop_test.go | 55 +++++++++++++ commands/url_test.go | 56 +++++++++++++ drivers/fakedriver/fakedriver.go | 25 +++--- libmachine/libmachine.go | 2 +- test/integration/cli/create-rm.bats | 2 +- 16 files changed, 447 insertions(+), 68 deletions(-) create mode 100644 commands/commandstest/fake_lib_machine_api.go delete mode 100644 commands/commandstest/fake_machine_cli_client.go create mode 100644 commands/commandstest/stdout_capture.go create mode 100644 commands/ip_test.go create mode 100644 commands/kill_test.go create mode 100644 commands/rm_test.go create mode 100644 commands/stop_test.go create mode 100644 commands/url_test.go diff --git a/commands/commands_test.go b/commands/commands_test.go index 63017b203b..f29de808c6 100644 --- a/commands/commands_test.go +++ b/commands/commands_test.go @@ -2,10 +2,9 @@ package commands import ( "errors" - "os" - "strings" "testing" + "github.com/docker/machine/commands/commandstest" "github.com/docker/machine/drivers/fakedriver" "github.com/docker/machine/libmachine/host" "github.com/docker/machine/libmachine/hosttest" @@ -13,18 +12,6 @@ import ( "github.com/stretchr/testify/assert" ) -var ( - stdout *os.File -) - -func init() { - stdout = os.Stdout -} - -func cleanup() { - os.Stdout = stdout -} - func TestRunActionForeachMachine(t *testing.T) { // Assume a bunch of machines in randomly started or // stopped states. @@ -95,29 +82,29 @@ func TestRunActionForeachMachine(t *testing.T) { } func TestPrintIPEmptyGivenLocalEngine(t *testing.T) { - defer cleanup() + stdoutGetter := commandstest.NewStdoutGetter() + defer stdoutGetter.Stop() + host, _ := hosttest.GetDefaultTestHost() + err := printIP(host)() - out, w := captureStdout() - - assert.Nil(t, printIP(host)()) - w.Close() - - assert.Equal(t, "", strings.TrimSpace(<-out)) + assert.NoError(t, err) + assert.Equal(t, "\n", stdoutGetter.Output()) } func TestPrintIPPrintsGivenRemoteEngine(t *testing.T) { - defer cleanup() + stdoutGetter := commandstest.NewStdoutGetter() + defer stdoutGetter.Stop() + host, _ := hosttest.GetDefaultTestHost() - host.Driver = &fakedriver.Driver{} + host.Driver = &fakedriver.Driver{ + MockState: state.Running, + MockIP: "1.2.3.4", + } + err := printIP(host)() - out, w := captureStdout() - - assert.Nil(t, printIP(host)()) - - w.Close() - - assert.Equal(t, "1.2.3.4", strings.TrimSpace(<-out)) + assert.NoError(t, err) + assert.Equal(t, "1.2.3.4\n", stdoutGetter.Output()) } func TestConsolidateError(t *testing.T) { diff --git a/commands/commandstest/fake_command_line.go b/commands/commandstest/fake_command_line.go index ffa060ce92..df11c488d6 100644 --- a/commands/commandstest/fake_command_line.go +++ b/commands/commandstest/fake_command_line.go @@ -55,6 +55,9 @@ func (fcli *FakeCommandLine) Int(key string) int { } func (fcli *FakeCommandLine) Bool(key string) bool { + if fcli.LocalFlags == nil { + return false + } return fcli.LocalFlags.Bool(key) } diff --git a/commands/commandstest/fake_lib_machine_api.go b/commands/commandstest/fake_lib_machine_api.go new file mode 100644 index 0000000000..e28903ab2a --- /dev/null +++ b/commands/commandstest/fake_lib_machine_api.go @@ -0,0 +1,80 @@ +package commandstest + +import ( + "github.com/docker/machine/libmachine" + "github.com/docker/machine/libmachine/drivers" + "github.com/docker/machine/libmachine/host" + "github.com/docker/machine/libmachine/mcnerror" + "github.com/docker/machine/libmachine/state" +) + +type FakeLibmachineAPI struct { + Hosts []*host.Host +} + +func (api *FakeLibmachineAPI) NewPluginDriver(string, []byte) (drivers.Driver, error) { + return nil, nil +} + +func (api *FakeLibmachineAPI) NewHost(driver drivers.Driver) (*host.Host, error) { + return nil, nil +} + +func (api *FakeLibmachineAPI) Create(h *host.Host) error { + return nil +} + +func (api *FakeLibmachineAPI) Exists(name string) (bool, error) { + for _, host := range api.Hosts { + if name == host.Name { + return true, nil + } + } + + return false, nil +} + +func (api *FakeLibmachineAPI) List() ([]string, error) { + return []string{}, nil +} + +func (api *FakeLibmachineAPI) Load(name string) (*host.Host, error) { + for _, host := range api.Hosts { + if name == host.Name { + return host, nil + } + } + + return nil, mcnerror.ErrHostDoesNotExist{ + Name: name, + } +} + +func (api *FakeLibmachineAPI) Remove(name string) error { + newHosts := []*host.Host{} + + for _, host := range api.Hosts { + if name != host.Name { + newHosts = append(newHosts, host) + } + } + + api.Hosts = newHosts + + return nil +} + +func (api *FakeLibmachineAPI) Save(host *host.Host) error { + return nil +} + +func State(api libmachine.API, name string) state.State { + host, _ := api.Load(name) + machineState, _ := host.Driver.GetState() + return machineState +} + +func Exists(api libmachine.API, name string) bool { + exists, _ := api.Exists(name) + return exists +} diff --git a/commands/commandstest/fake_machine_cli_client.go b/commands/commandstest/fake_machine_cli_client.go deleted file mode 100644 index 31cfb43e57..0000000000 --- a/commands/commandstest/fake_machine_cli_client.go +++ /dev/null @@ -1 +0,0 @@ -package commandstest diff --git a/commands/commandstest/stdout_capture.go b/commands/commandstest/stdout_capture.go new file mode 100644 index 0000000000..926aed3d8a --- /dev/null +++ b/commands/commandstest/stdout_capture.go @@ -0,0 +1,54 @@ +package commandstest + +import ( + "bytes" + "io" + + "os" +) + +var ( + stdout *os.File +) + +func init() { + stdout = os.Stdout +} + +type StdoutGetter interface { + Output() string + Stop() +} + +type stdoutCapturer struct { + stdout *os.File + output chan string +} + +func NewStdoutGetter() StdoutGetter { + r, w, _ := os.Pipe() + os.Stdout = w + + output := make(chan string) + go func() { + var testOutput bytes.Buffer + io.Copy(&testOutput, r) + output <- testOutput.String() + }() + + return &stdoutCapturer{ + stdout: w, + output: output, + } +} + +func (c *stdoutCapturer) Output() string { + c.stdout.Close() + text := <-c.output + close(c.output) + return text +} + +func (c *stdoutCapturer) Stop() { + os.Stdout = stdout +} diff --git a/commands/env_test.go b/commands/env_test.go index 950dd60c46..561976d675 100644 --- a/commands/env_test.go +++ b/commands/env_test.go @@ -14,6 +14,7 @@ import ( "github.com/docker/machine/libmachine/host" "github.com/docker/machine/libmachine/libmachinetest" "github.com/docker/machine/libmachine/persist/persisttest" + "github.com/docker/machine/libmachine/state" "github.com/stretchr/testify/assert" ) @@ -272,8 +273,11 @@ func TestShellCfgSet(t *testing.T) { FakeStore: &persisttest.FakeStore{ Hosts: []*host.Host{ { - Name: "quux", - Driver: &fakedriver.Driver{}, + Name: "quux", + Driver: &fakedriver.Driver{ + MockState: state.Running, + MockIP: "1.2.3.4", + }, }, }, }, @@ -315,8 +319,11 @@ func TestShellCfgSet(t *testing.T) { FakeStore: &persisttest.FakeStore{ Hosts: []*host.Host{ { - Name: "quux", - Driver: &fakedriver.Driver{}, + Name: "quux", + Driver: &fakedriver.Driver{ + MockState: state.Running, + MockIP: "1.2.3.4", + }, }, }, }, diff --git a/commands/ip_test.go b/commands/ip_test.go new file mode 100644 index 0000000000..187a953cb0 --- /dev/null +++ b/commands/ip_test.go @@ -0,0 +1,45 @@ +package commands + +import ( + "testing" + + "github.com/docker/machine/commands/commandstest" + "github.com/docker/machine/drivers/fakedriver" + "github.com/docker/machine/libmachine/host" + "github.com/docker/machine/libmachine/state" + "github.com/stretchr/testify/assert" +) + +func TestCmdIPMissingMachineName(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{} + api := &commandstest.FakeLibmachineAPI{} + + err := cmdURL(commandLine, api) + + assert.EqualError(t, err, "Error: Expected one machine name as an argument") +} + +func TestCmdIP(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{ + CliArgs: []string{"machine"}, + } + api := &commandstest.FakeLibmachineAPI{ + Hosts: []*host.Host{ + { + Name: "machine", + Driver: &fakedriver.Driver{ + MockState: state.Running, + MockIP: "1.2.3.4", + }, + }, + }, + } + + stdoutGetter := commandstest.NewStdoutGetter() + defer stdoutGetter.Stop() + + err := cmdIP(commandLine, api) + + assert.NoError(t, err) + assert.Equal(t, "1.2.3.4\n", stdoutGetter.Output()) +} diff --git a/commands/kill_test.go b/commands/kill_test.go new file mode 100644 index 0000000000..4b32870022 --- /dev/null +++ b/commands/kill_test.go @@ -0,0 +1,55 @@ +package commands + +import ( + "testing" + + "github.com/docker/machine/commands/commandstest" + "github.com/docker/machine/drivers/fakedriver" + "github.com/docker/machine/libmachine/host" + "github.com/docker/machine/libmachine/state" + "github.com/stretchr/testify/assert" +) + +func TestCmdKillMissingMachineName(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{} + api := &commandstest.FakeLibmachineAPI{} + + err := cmdKill(commandLine, api) + + assert.EqualError(t, err, "Error: Expected to get one or more machine names as arguments") +} + +func TestCmdKill(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{ + CliArgs: []string{"machineToKill1", "machineToKill2"}, + } + api := &commandstest.FakeLibmachineAPI{ + Hosts: []*host.Host{ + { + Name: "machineToKill1", + Driver: &fakedriver.Driver{ + MockState: state.Running, + }, + }, + { + Name: "machineToKill2", + Driver: &fakedriver.Driver{ + MockState: state.Running, + }, + }, + { + Name: "machine", + Driver: &fakedriver.Driver{ + MockState: state.Running, + }, + }, + }, + } + + err := cmdKill(commandLine, api) + assert.NoError(t, err) + + assert.Equal(t, state.Stopped, commandstest.State(api, "machineToKill1")) + assert.Equal(t, state.Stopped, commandstest.State(api, "machineToKill2")) + assert.Equal(t, state.Running, commandstest.State(api, "machine")) +} diff --git a/commands/ls_test.go b/commands/ls_test.go index 9aa411cee4..b77bd42217 100644 --- a/commands/ls_test.go +++ b/commands/ls_test.go @@ -1,8 +1,6 @@ package commands import ( - "bytes" - "io" "os" "testing" @@ -277,21 +275,6 @@ func TestFilterHostsDifferentFlagsProduceAND(t *testing.T) { assert.EqualValues(t, filterHosts(hosts, opts), expected) } -func captureStdout() (chan string, *os.File) { - r, w, _ := os.Pipe() - os.Stdout = w - - out := make(chan string) - - go func() { - var testOutput bytes.Buffer - io.Copy(&testOutput, r) - out <- testOutput.String() - }() - - return out, w -} - func TestGetHostListItems(t *testing.T) { hostListItemsChan := make(chan HostListItem) activeHostURL := "tcp://active.host.com:2376" @@ -301,7 +284,7 @@ func TestGetHostListItems(t *testing.T) { Name: "foo", Driver: &fakedriver.Driver{ MockState: state.Running, - MockURL: activeHostURL, + MockIP: "active.host.com", }, HostOptions: &host.Options{ SwarmOptions: &swarm.Options{}, @@ -334,7 +317,7 @@ func TestGetHostListItems(t *testing.T) { }{ "foo": {state.Running, true, ""}, "bar": {state.Stopped, false, ""}, - "baz": {state.Error, false, "Unable to get url"}, + "baz": {state.Error, false, "Unable to get ip"}, } // TODO: Ideally this would mockable via interface instead. @@ -377,7 +360,7 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) { Name: "foo", Driver: &fakedriver.Driver{ MockState: state.Running, - MockURL: "tcp://120.0.0.1:2376", + MockIP: "120.0.0.1", }, HostOptions: &host.Options{ SwarmOptions: &swarm.Options{ diff --git a/commands/rm.go b/commands/rm.go index 726ec0aaa8..9e67434f83 100644 --- a/commands/rm.go +++ b/commands/rm.go @@ -1,7 +1,6 @@ package commands import ( - "errors" "fmt" "github.com/docker/machine/libmachine" @@ -11,7 +10,7 @@ import ( func cmdRm(c CommandLine, api libmachine.API) error { if len(c.Args()) == 0 { c.ShowHelp() - return errors.New("You must specify a machine name") + return ErrNoMachineSpecified } force := c.Bool("force") diff --git a/commands/rm_test.go b/commands/rm_test.go new file mode 100644 index 0000000000..d71be6f75e --- /dev/null +++ b/commands/rm_test.go @@ -0,0 +1,49 @@ +package commands + +import ( + "testing" + + "github.com/docker/machine/commands/commandstest" + "github.com/docker/machine/drivers/fakedriver" + "github.com/docker/machine/libmachine/host" + "github.com/stretchr/testify/assert" +) + +func TestCmdRmMissingMachineName(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{} + api := &commandstest.FakeLibmachineAPI{} + + err := cmdRm(commandLine, api) + + assert.EqualError(t, err, "Error: Expected to get one or more machine names as arguments") + assert.True(t, commandLine.HelpShown) +} + +func TestCmdRm(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{ + CliArgs: []string{"machineToRemove1", "machineToRemove2"}, + } + api := &commandstest.FakeLibmachineAPI{ + Hosts: []*host.Host{ + { + Name: "machineToRemove1", + Driver: &fakedriver.Driver{}, + }, + { + Name: "machineToRemove2", + Driver: &fakedriver.Driver{}, + }, + { + Name: "machine", + Driver: &fakedriver.Driver{}, + }, + }, + } + + err := cmdRm(commandLine, api) + assert.NoError(t, err) + + assert.False(t, commandstest.Exists(api, "machineToRemove1")) + assert.False(t, commandstest.Exists(api, "machineToRemove2")) + assert.True(t, commandstest.Exists(api, "machine")) +} diff --git a/commands/stop_test.go b/commands/stop_test.go new file mode 100644 index 0000000000..8adb6c4e90 --- /dev/null +++ b/commands/stop_test.go @@ -0,0 +1,55 @@ +package commands + +import ( + "testing" + + "github.com/docker/machine/commands/commandstest" + "github.com/docker/machine/drivers/fakedriver" + "github.com/docker/machine/libmachine/host" + "github.com/docker/machine/libmachine/state" + "github.com/stretchr/testify/assert" +) + +func TestCmdStopMissingMachineName(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{} + api := &commandstest.FakeLibmachineAPI{} + + err := cmdStop(commandLine, api) + + assert.EqualError(t, err, "Error: Expected to get one or more machine names as arguments") +} + +func TestCmdStop(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{ + CliArgs: []string{"machineToStop1", "machineToStop2"}, + } + api := &commandstest.FakeLibmachineAPI{ + Hosts: []*host.Host{ + { + Name: "machineToStop1", + Driver: &fakedriver.Driver{ + MockState: state.Running, + }, + }, + { + Name: "machineToStop2", + Driver: &fakedriver.Driver{ + MockState: state.Running, + }, + }, + { + Name: "machine", + Driver: &fakedriver.Driver{ + MockState: state.Running, + }, + }, + }, + } + + err := cmdStop(commandLine, api) + assert.NoError(t, err) + + assert.Equal(t, state.Stopped, commandstest.State(api, "machineToStop1")) + assert.Equal(t, state.Stopped, commandstest.State(api, "machineToStop2")) + assert.Equal(t, state.Running, commandstest.State(api, "machine")) +} diff --git a/commands/url_test.go b/commands/url_test.go new file mode 100644 index 0000000000..773fbe07df --- /dev/null +++ b/commands/url_test.go @@ -0,0 +1,56 @@ +package commands + +import ( + "testing" + + "github.com/docker/machine/commands/commandstest" + "github.com/docker/machine/drivers/fakedriver" + "github.com/docker/machine/libmachine/host" + "github.com/docker/machine/libmachine/state" + "github.com/stretchr/testify/assert" +) + +func TestCmdURLMissingMachineName(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{} + api := &commandstest.FakeLibmachineAPI{} + + err := cmdURL(commandLine, api) + + assert.EqualError(t, err, "Error: Expected one machine name as an argument") +} + +func TestCmdURLTooManyNames(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{ + CliArgs: []string{"machineToRemove1", "machineToRemove2"}, + } + api := &commandstest.FakeLibmachineAPI{} + + err := cmdURL(commandLine, api) + + assert.EqualError(t, err, "Error: Expected one machine name as an argument") +} + +func TestCmdURL(t *testing.T) { + commandLine := &commandstest.FakeCommandLine{ + CliArgs: []string{"machine"}, + } + api := &commandstest.FakeLibmachineAPI{ + Hosts: []*host.Host{ + { + Name: "machine", + Driver: &fakedriver.Driver{ + MockState: state.Running, + MockIP: "120.0.0.1", + }, + }, + }, + } + + stdoutGetter := commandstest.NewStdoutGetter() + defer stdoutGetter.Stop() + + err := cmdURL(commandLine, api) + + assert.NoError(t, err) + assert.Equal(t, "tcp://120.0.0.1:2376\n", stdoutGetter.Output()) +} diff --git a/drivers/fakedriver/fakedriver.go b/drivers/fakedriver/fakedriver.go index b038032847..96af416852 100644 --- a/drivers/fakedriver/fakedriver.go +++ b/drivers/fakedriver/fakedriver.go @@ -11,7 +11,7 @@ import ( type Driver struct { *drivers.BaseDriver MockState state.State - MockURL string + MockIP string MockName string } @@ -29,13 +29,14 @@ func (d *Driver) SetConfigFromFlags(flags drivers.DriverOptions) error { } func (d *Driver) GetURL() (string, error) { - if d.MockState == state.Error { - return "", fmt.Errorf("Unable to get url") + ip, err := d.GetIP() + if err != nil { + return "", err } - if d.MockState != state.Running { - return "", drivers.ErrHostIsNotRunning + if ip == "" { + return "", nil } - return d.MockURL, nil + return fmt.Sprintf("tcp://%s:2376", ip), nil } func (d *Driver) GetMachineName() string { @@ -43,9 +44,13 @@ func (d *Driver) GetMachineName() string { } func (d *Driver) GetIP() (string, error) { - // TODO: Expose this and other similar values as configurable instead - // of hardcoded. - return "1.2.3.4", nil + if d.MockState == state.Error { + return "", fmt.Errorf("Unable to get ip") + } + if d.MockState != state.Running { + return "", drivers.ErrHostIsNotRunning + } + return d.MockIP, nil } func (d *Driver) GetSSHHostname() (string, error) { @@ -87,10 +92,12 @@ func (d *Driver) Stop() error { } func (d *Driver) Restart() error { + d.MockState = state.Running return nil } func (d *Driver) Kill() error { + d.MockState = state.Stopped return nil } diff --git a/libmachine/libmachine.go b/libmachine/libmachine.go index 63fe37da7a..a23c0327be 100644 --- a/libmachine/libmachine.go +++ b/libmachine/libmachine.go @@ -21,7 +21,7 @@ import ( type API interface { persist.Store - NewPluginDriver(string, []byte) (drivers.Driver, error) + persist.PluginDriverFactory NewHost(drivers.Driver) (*host.Host, error) Create(h *host.Host) error } diff --git a/test/integration/cli/create-rm.bats b/test/integration/cli/create-rm.bats index 49a9e38d19..8bd8e12525 100644 --- a/test/integration/cli/create-rm.bats +++ b/test/integration/cli/create-rm.bats @@ -67,7 +67,7 @@ load ${BASE_TEST_DIR}/helpers.bash run machine rm last=$(expr ${#lines[@]} - 1) [ "$status" -eq 1 ] - [[ ${lines[$last]} == "You must specify a machine name" ]] + [[ ${lines[$last]} == "Error: Expected to get one or more machine names as arguments" ]] } @test "none: rm non existent machine fails 'machine rm ∞'" {