Merge pull request #2522 from dgageot/simplify-ls-tests

Simplify ls command tests

commands/create.go

@@ -293,7 +293,7 @@ func cmdCreateOuter(c CommandLine, api libmachine.API) error {
 	}
 
 	if _, ok := driver.(*errdriver.Driver); ok {
-		return errdriver.NotLoadable{driverName}
+		return errdriver.NotLoadable{Name: driverName}
 	}
 
 	// TODO: So much flag manipulation and voodoo here, it seems to be

commands/ls.go

@@ -160,9 +160,11 @@ func filterHosts(hosts []*host.Host, filters FilterOptions) []*host.Host {
 func getSwarmMasters(hosts []*host.Host) map[string]string {
 	swarmMasters := make(map[string]string)
 	for _, h := range hosts {
-		swarmOptions := h.HostOptions.SwarmOptions
-		if swarmOptions != nil && swarmOptions.Master {
-			swarmMasters[swarmOptions.Discovery] = h.Name
+		if h.HostOptions != nil {
+			swarmOptions := h.HostOptions.SwarmOptions
+			if swarmOptions != nil && swarmOptions.Master {
+				swarmMasters[swarmOptions.Discovery] = h.Name
+			}
 		}
 	}
 	return swarmMasters
@@ -183,7 +185,7 @@ func matchesSwarmName(host *host.Host, swarmNames []string, swarmMasters map[str
 		return true
 	}
 	for _, n := range swarmNames {
-		if host.HostOptions.SwarmOptions != nil {
+		if host.HostOptions != nil && host.HostOptions.SwarmOptions != nil {
 			if strings.EqualFold(n, swarmMasters[host.HostOptions.SwarmOptions.Discovery]) {
 				return true
 			}

commands/ls_test.go

@@ -84,9 +84,8 @@ func TestFilterHostsReturnsSameGivenNoFilters(t *testing.T) {
 	opts := FilterOptions{}
 	hosts := []*host.Host{
 		{
-			Name:        "testhost",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
+			Name:       "testhost",
+			DriverName: "fakedriver",
 		},
 	}
 	actual := filterHosts(hosts, opts)
@@ -126,9 +125,8 @@ func TestFilterHostsReturnsEmptyGivenNonMatchingFilters(t *testing.T) {
 	}
 	hosts := []*host.Host{
 		{
-			Name:        "testhost",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
+			Name:       "testhost",
+			DriverName: "fakedriver",
 		},
 	}
 	assert.Empty(t, filterHosts(hosts, opts))
@@ -171,21 +169,18 @@ func TestFilterHostsByDriverName(t *testing.T) {
 	}
 	node1 :=
 		&host.Host{
-			Name:        "node1",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
+			Name:       "node1",
+			DriverName: "fakedriver",
 		}
 	node2 :=
 		&host.Host{
-			Name:        "node2",
-			DriverName:  "virtualbox",
-			HostOptions: &host.Options{},
+			Name:       "node2",
+			DriverName: "virtualbox",
 		}
 	node3 :=
 		&host.Host{
-			Name:        "node3",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
+			Name:       "node3",
+			DriverName: "fakedriver",
 		}
 	hosts := []*host.Host{node1, node2, node3}
 	expected := []*host.Host{node1, node3}
@@ -199,24 +194,21 @@ func TestFilterHostsByState(t *testing.T) {
 	}
 	node1 :=
 		&host.Host{
-			Name:        "node1",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Paused},
+			Name:       "node1",
+			DriverName: "fakedriver",
+			Driver:     &fakedriver.Driver{MockState: state.Paused},
 		}
 	node2 :=
 		&host.Host{
-			Name:        "node2",
-			DriverName:  "virtualbox",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Stopped},
+			Name:       "node2",
+			DriverName: "virtualbox",
+			Driver:     &fakedriver.Driver{MockState: state.Stopped},
 		}
 	node3 :=
 		&host.Host{
-			Name:        "node3",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Running},
+			Name:       "node3",
+			DriverName: "fakedriver",
+			Driver:     &fakedriver.Driver{MockState: state.Running},
 		}
 	hosts := []*host.Host{node1, node2, node3}
 	expected := []*host.Host{node1, node2}
@@ -230,31 +222,27 @@ func TestFilterHostsByName(t *testing.T) {
 	}
 	node1 :=
 		&host.Host{
-			Name:        "fire",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Paused, MockName: "fire"},
+			Name:       "fire",
+			DriverName: "fakedriver",
+			Driver:     &fakedriver.Driver{MockState: state.Paused, MockName: "fire"},
 		}
 	node2 :=
 		&host.Host{
-			Name:        "ice",
-			DriverName:  "adriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Paused, MockName: "ice"},
+			Name:       "ice",
+			DriverName: "adriver",
+			Driver:     &fakedriver.Driver{MockState: state.Paused, MockName: "ice"},
 		}
 	node3 :=
 		&host.Host{
-			Name:        "air",
-			DriverName:  "nodriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Paused, MockName: "air"},
+			Name:       "air",
+			DriverName: "nodriver",
+			Driver:     &fakedriver.Driver{MockState: state.Paused, MockName: "air"},
 		}
 	node4 :=
 		&host.Host{
-			Name:        "water",
-			DriverName:  "falsedriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Paused, MockName: "water"},
+			Name:       "water",
+			DriverName: "falsedriver",
+			Driver:     &fakedriver.Driver{MockState: state.Paused, MockName: "water"},
 		}
 	hosts := []*host.Host{node1, node2, node3, node4}
 	expected := []*host.Host{node1, node2, node3}
@@ -269,21 +257,18 @@ func TestFilterHostsMultiFlags(t *testing.T) {
 	}
 	node1 :=
 		&host.Host{
-			Name:        "node1",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
+			Name:       "node1",
+			DriverName: "fakedriver",
 		}
 	node2 :=
 		&host.Host{
-			Name:        "node2",
-			DriverName:  "virtualbox",
-			HostOptions: &host.Options{},
+			Name:       "node2",
+			DriverName: "virtualbox",
 		}
 	node3 :=
 		&host.Host{
-			Name:        "node3",
-			DriverName:  "softlayer",
-			HostOptions: &host.Options{},
+			Name:       "node3",
+			DriverName: "softlayer",
 		}
 	hosts := []*host.Host{node1, node2, node3}
 	expected := []*host.Host{node1, node2}
@@ -296,38 +281,34 @@ func TestFilterHostsDifferentFlagsProduceAND(t *testing.T) {
 		DriverName: []string{"virtualbox"},
 		State:      []string{"Running"},
 	}
-	node1 :=
-		&host.Host{
-			Name:        "node1",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Paused},
-		}
-	node2 :=
-		&host.Host{
-			Name:        "node2",
-			DriverName:  "virtualbox",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Stopped},
-		}
-	node3 :=
-		&host.Host{
-			Name:        "node3",
-			DriverName:  "fakedriver",
-			HostOptions: &host.Options{},
-			Driver:      &fakedriver.Driver{MockState: state.Running},
-		}
-	hosts := []*host.Host{node1, node2, node3}
-	expected := []*host.Host{}
 
-	assert.EqualValues(t, filterHosts(hosts, opts), expected)
+	hosts := []*host.Host{
+		{
+			Name:       "node1",
+			DriverName: "fakedriver",
+			Driver:     &fakedriver.Driver{MockState: state.Paused},
+		},
+		{
+			Name:       "node2",
+			DriverName: "virtualbox",
+			Driver:     &fakedriver.Driver{MockState: state.Stopped},
+		},
+		{
+			Name:       "node3",
+			DriverName: "fakedriver",
+			Driver:     &fakedriver.Driver{MockState: state.Running},
+		},
+	}
+
+	assert.Empty(t, filterHosts(hosts, opts))
 }
 
 func TestGetHostListItems(t *testing.T) {
+	defer func(versioner mcndockerclient.DockerVersioner) { mcndockerclient.CurrentDockerVersioner = versioner }(mcndockerclient.CurrentDockerVersioner)
 	mcndockerclient.CurrentDockerVersioner = &mcndockerclient.FakeDockerVersioner{Version: "1.9"}
-	defer mcndockerclient.CleanupDockerVersioner()
 
 	// TODO: Ideally this would mockable via interface instead.
+	defer func(host string) { os.Setenv("DOCKER_HOST", host) }(os.Getenv("DOCKER_HOST"))
 	os.Setenv("DOCKER_HOST", "tcp://active.host.com:2376")
 
 	hosts := []*host.Host{
@@ -337,27 +318,18 @@ func TestGetHostListItems(t *testing.T) {
 				MockState: state.Running,
 				MockIP:    "active.host.com",
 			},
-			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{},
-			},
 		},
 		{
 			Name: "bar100",
 			Driver: &fakedriver.Driver{
 				MockState: state.Stopped,
 			},
-			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{},
-			},
 		},
 		{
 			Name: "bar10",
 			Driver: &fakedriver.Driver{
 				MockState: state.Error,
 			},
-			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{},
-			},
 		},
 	}
 
@@ -382,22 +354,13 @@ func TestGetHostListItems(t *testing.T) {
 		assert.Equal(t, expected[i].version, items[i].DockerVersion)
 		assert.Equal(t, expected[i].error, items[i].Error)
 	}
-
-	os.Unsetenv("DOCKER_HOST")
 }
 
-// issue #1908
 func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
+	defer func(versioner mcndockerclient.DockerVersioner) { mcndockerclient.CurrentDockerVersioner = versioner }(mcndockerclient.CurrentDockerVersioner)
 	mcndockerclient.CurrentDockerVersioner = &mcndockerclient.FakeDockerVersioner{Version: "1.9"}
-	defer mcndockerclient.CleanupDockerVersioner()
 
-	orgDockerHost := os.Getenv("DOCKER_HOST")
-	defer func() {
-		// revert DOCKER_HOST
-		os.Setenv("DOCKER_HOST", orgDockerHost)
-	}()
-
-	// unset DOCKER_HOST
+	defer func(host string) { os.Setenv("DOCKER_HOST", host) }(os.Getenv("DOCKER_HOST"))
 	os.Unsetenv("DOCKER_HOST")
 
 	hosts := []*host.Host{
@@ -407,52 +370,31 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
 				MockState: state.Running,
 				MockIP:    "120.0.0.1",
 			},
-			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{
-					Master:    false,
-					Address:   "",
-					Discovery: "",
-				},
-			},
 		},
 		{
 			Name: "bar",
 			Driver: &fakedriver.Driver{
 				MockState: state.Stopped,
 			},
-			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{
-					Master:    false,
-					Address:   "",
-					Discovery: "",
-				},
-			},
 		},
 		{
 			Name: "baz",
 			Driver: &fakedriver.Driver{
 				MockState: state.Saved,
 			},
-			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{
-					Master:    false,
-					Address:   "",
-					Discovery: "",
-				},
-			},
 		},
 	}
 
 	expected := map[string]struct {
-		state   state.State
-		active  bool
-		version string
+		state  state.State
+		active bool
 	}{
-		"foo": {state.Running, false, "v1.9"},
-		"bar": {state.Stopped, false, "Unknown"},
-		"baz": {state.Saved, false, "Unknown"},
+		"foo": {state.Running, false},
+		"bar": {state.Stopped, false},
+		"baz": {state.Saved, false},
 	}
 
+	// TEST
 	items := getHostListItems(hosts, map[string]error{})
 
 	for _, item := range items {
@@ -460,7 +402,6 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
 
 		assert.Equal(t, expected.state, item.State)
 		assert.Equal(t, expected.active, item.Active)
-		assert.Equal(t, expected.version, item.DockerVersion)
 	}
 }
 
@@ -485,12 +426,13 @@ func TestIsActive(t *testing.T) {
 
 		actual := isActive(c.state, "tcp://1.2.3.4:2376")
 
-		assert.Equal(t, c.expected, actual, "IsActive(%s, \"%s\") should return %v, but didn't", c.state, c.dockerHost, c.expected)
+		assert.Equal(t, c.expected, actual)
 	}
 }
 
 func TestGetHostStateTimeout(t *testing.T) {
-	originalTimeout := stateTimeoutDuration
+	defer func(timeout time.Duration) { stateTimeoutDuration = timeout }(stateTimeoutDuration)
+	stateTimeoutDuration = 1 * time.Millisecond
 
 	hosts := []*host.Host{
 		{
@@ -501,15 +443,11 @@ func TestGetHostStateTimeout(t *testing.T) {
 		},
 	}
 
-	stateTimeoutDuration = 1 * time.Millisecond
-	hostItems := getHostListItems(hosts, map[string]error{})
-	hostItem := hostItems[0]
+	hostItem := getHostListItems(hosts, nil)[0]
 
 	assert.Equal(t, "foo", hostItem.Name)
 	assert.Equal(t, state.Timeout, hostItem.State)
 	assert.Equal(t, "Driver", hostItem.DriverName)
-
-	stateTimeoutDuration = originalTimeout
 }
 
 func TestGetHostStateError(t *testing.T) {
@@ -522,8 +460,7 @@ func TestGetHostStateError(t *testing.T) {
 		},
 	}
 
-	hostItems := getHostListItems(hosts, map[string]error{})
-	hostItem := hostItems[0]
+	hostItem := getHostListItems(hosts, nil)[0]
 
 	assert.Equal(t, "foo", hostItem.Name)
 	assert.Equal(t, state.Error, hostItem.State)
@@ -533,9 +470,9 @@ func TestGetHostStateError(t *testing.T) {
 	assert.Nil(t, hostItem.SwarmOptions)
 }
 
-func TestGetSomeHostInEror(t *testing.T) {
+func TestGetSomeHostInError(t *testing.T) {
+	defer func(versioner mcndockerclient.DockerVersioner) { mcndockerclient.CurrentDockerVersioner = versioner }(mcndockerclient.CurrentDockerVersioner)
 	mcndockerclient.CurrentDockerVersioner = &mcndockerclient.FakeDockerVersioner{Version: "1.9"}
-	defer mcndockerclient.CleanupDockerVersioner()
 
 	hosts := []*host.Host{
 		{
@@ -553,7 +490,6 @@ func TestGetSomeHostInEror(t *testing.T) {
 	assert.Equal(t, 2, len(hostItems))
 
 	hostItem := hostItems[0]
-
 	assert.Equal(t, "bar", hostItem.Name)
 	assert.Equal(t, state.Error, hostItem.State)
 	assert.Equal(t, "not found", hostItem.DriverName)
@@ -562,7 +498,6 @@ func TestGetSomeHostInEror(t *testing.T) {
 	assert.Nil(t, hostItem.SwarmOptions)
 
 	hostItem = hostItems[1]
-
 	assert.Equal(t, "foo", hostItem.Name)
 	assert.Equal(t, state.Running, hostItem.State)
 }

commands/version_test.go

@@ -46,8 +46,8 @@ func TestCmdVersionNotFound(t *testing.T) {
 }
 
 func TestCmdVersionOnHost(t *testing.T) {
+	defer func(versioner mcndockerclient.DockerVersioner) { mcndockerclient.CurrentDockerVersioner = versioner }(mcndockerclient.CurrentDockerVersioner)
 	mcndockerclient.CurrentDockerVersioner = &mcndockerclient.FakeDockerVersioner{Version: "1.9.1"}
-	defer mcndockerclient.CleanupDockerVersioner()
 
 	commandLine := &commandstest.FakeCommandLine{
 		CliArgs: []string{"machine"},
@@ -68,8 +68,8 @@ func TestCmdVersionOnHost(t *testing.T) {
 }
 
 func TestCmdVersionFailure(t *testing.T) {
+	defer func(versioner mcndockerclient.DockerVersioner) { mcndockerclient.CurrentDockerVersioner = versioner }(mcndockerclient.CurrentDockerVersioner)
 	mcndockerclient.CurrentDockerVersioner = &mcndockerclient.FakeDockerVersioner{Err: errors.New("connection failure")}
-	defer mcndockerclient.CleanupDockerVersioner()
 
 	commandLine := &commandstest.FakeCommandLine{
 		CliArgs: []string{"machine"},

libmachine/mcndockerclient/fake_docker_versioner.go

@@ -1,11 +1,5 @@
 package mcndockerclient
 
-var originalDockerVersioner = CurrentDockerVersioner
-
-func CleanupDockerVersioner() {
-	CurrentDockerVersioner = originalDockerVersioner
-}
-
 type FakeDockerVersioner struct {
 	Version string
 	Err     error