Merge pull request #2409 from dgageot/more-commands-tests

Add more command tests

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.NoError(t, err)
-
+	assert.Equal(t, "\n", stdoutGetter.Output())
-	assert.Nil(t, printIP(host)())
-	w.Close()
-
-	assert.Equal(t, "", strings.TrimSpace(<-out))
 }
 
 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.NoError(t, err)
-
+	assert.Equal(t, "1.2.3.4\n", stdoutGetter.Output())
-	assert.Nil(t, printIP(host)())
-
-	w.Close()
-
-	assert.Equal(t, "1.2.3.4", strings.TrimSpace(<-out))
 }
 
 func TestConsolidateError(t *testing.T) {

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)
 }
 

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
+}

commands/commandstest/fake_machine_cli_client.go

@@ -1 +0,0 @@
-package commandstest

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
+}

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",
+							Name: "quux",
-							Driver: &fakedriver.Driver{},
+							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",
+							Name: "quux",
-							Driver: &fakedriver.Driver{},
+							Driver: &fakedriver.Driver{
+								MockState: state.Running,
+								MockIP:    "1.2.3.4",
+							},
 						},
 					},
 				},

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())
+}

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"))
+}

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{

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")

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"))
+}

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"))
+}

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())
+}

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 {
+	ip, err := d.GetIP()
-		return "", fmt.Errorf("Unable to get url")
+	if err != nil {
+		return "", err
 	}
-	if d.MockState != state.Running {
+	if ip == "" {
-		return "", drivers.ErrHostIsNotRunning
+		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
+	if d.MockState == state.Error {
-	// of hardcoded.
+		return "", fmt.Errorf("Unable to get ip")
-	return "1.2.3.4", nil
+	}
+	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
 }
 

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
 }

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 ∞'" {