From c421bb456ee58717f93c31272553a5a76ac91209 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 8 Dec 2015 09:28:42 +0100 Subject: [PATCH] Simplify ls command tests Signed-off-by: David Gageot --- commands/create.go | 2 +- commands/ls.go | 10 +- commands/ls_test.go | 211 ++++++------------ commands/version_test.go | 4 +- .../mcndockerclient/fake_docker_versioner.go | 6 - 5 files changed, 82 insertions(+), 151 deletions(-) diff --git a/commands/create.go b/commands/create.go index a9eb1badd9..8a2cf65430 100644 --- a/commands/create.go +++ b/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 diff --git a/commands/ls.go b/commands/ls.go index 1a30b68f51..e95af6f954 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -154,9 +154,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 @@ -176,7 +178,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 } diff --git a/commands/ls_test.go b/commands/ls_test.go index 2b3ffdd5fa..d10a52045c 100644 --- a/commands/ls_test.go +++ b/commands/ls_test.go @@ -76,9 +76,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) @@ -99,9 +98,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)) @@ -144,21 +142,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} @@ -172,24 +167,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} @@ -203,31 +195,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} @@ -242,21 +230,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} @@ -269,38 +254,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{ @@ -310,27 +291,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{}, - }, }, } @@ -355,22 +327,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{ @@ -380,52 +343,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 { @@ -433,7 +375,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) } } @@ -458,12 +399,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{ { @@ -474,15 +416,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) { @@ -495,8 +433,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) @@ -506,9 +443,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{ { @@ -526,7 +463,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) @@ -535,7 +471,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) } diff --git a/commands/version_test.go b/commands/version_test.go index 9675745d3a..7d593f09ec 100644 --- a/commands/version_test.go +++ b/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"}, diff --git a/libmachine/mcndockerclient/fake_docker_versioner.go b/libmachine/mcndockerclient/fake_docker_versioner.go index 8d30360008..32c4bb4cdf 100644 --- a/libmachine/mcndockerclient/fake_docker_versioner.go +++ b/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