Add interfaces for CLI unit testing and env test

Signed-off-by: Nathan LeClaire <nathan.leclaire@gmail.com>

commands/commandstest/fake_command_line.go

@@ -0,0 +1,92 @@
+package commandstest
+
+import (
+	"github.com/codegangsta/cli"
+)
+
+type FakeFlagger struct {
+	Data map[string]interface{}
+}
+
+type FakeCommandLine struct {
+	LocalFlags, GlobalFlags *FakeFlagger
+	HelpShown               bool
+	CliArgs                 []string
+}
+
+func (ff FakeFlagger) String(key string) string {
+	if value, ok := ff.Data[key]; ok {
+		return value.(string)
+	}
+	return ""
+}
+
+func (ff FakeFlagger) StringSlice(key string) []string {
+	if value, ok := ff.Data[key]; ok {
+		return value.([]string)
+	}
+	return []string{}
+}
+
+func (ff FakeFlagger) Int(key string) int {
+	if value, ok := ff.Data[key]; ok {
+		return value.(int)
+	}
+	return 0
+}
+
+func (ff FakeFlagger) Bool(key string) bool {
+	if value, ok := ff.Data[key]; ok {
+		return value.(bool)
+	}
+	return false
+}
+
+func (fcli *FakeCommandLine) String(key string) string {
+	return fcli.LocalFlags.String(key)
+}
+
+func (fcli *FakeCommandLine) StringSlice(key string) []string {
+	return fcli.LocalFlags.StringSlice(key)
+}
+
+func (fcli *FakeCommandLine) Int(key string) int {
+	return fcli.LocalFlags.Int(key)
+}
+
+func (fcli *FakeCommandLine) Bool(key string) bool {
+	return fcli.LocalFlags.Bool(key)
+}
+
+func (fcli *FakeCommandLine) GlobalString(key string) string {
+	return fcli.GlobalFlags.String(key)
+}
+
+func (fcli *FakeCommandLine) Generic(name string) interface{} {
+	return fcli.LocalFlags.Data[name]
+}
+
+func (fcli *FakeCommandLine) FlagNames() []string {
+	flagNames := []string{}
+	for key := range fcli.LocalFlags.Data {
+		flagNames = append(flagNames, key)
+	}
+
+	return flagNames
+}
+
+func (fcli *FakeCommandLine) ShowHelp() {
+	fcli.HelpShown = true
+}
+
+func (fcli *FakeCommandLine) Application() *cli.App {
+	return cli.NewApp()
+}
+
+func (fcli *FakeCommandLine) Args() cli.Args {
+	return fcli.CliArgs
+}
+
+func (fcli *FakeCommandLine) ShowVersion() {
+	return
+}

commands/commandstest/fake_machine_cli_client.go

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

commands/config.go

@@ -14,6 +14,14 @@ import (
 	"github.com/docker/machine/libmachine/state"
 )
 
+var (
+	defaultConnChecker ConnChecker
+)
+
+func init() {
+	defaultConnChecker = &MachineConnChecker{}
+}
+
 // ErrCertInvalid for when the cert is computed to be invalid.
 type ErrCertInvalid struct {
 	wrappedErr error
@@ -41,7 +49,7 @@ func cmdConfig(c CommandLine, api libmachine.API) error {
 		return err
 	}
 
-	dockerHost, authOptions, err := runConnectionBoilerplate(host, c)
+	dockerHost, authOptions, err := defaultConnChecker.Check(host, c.Bool("swarm"))
 	if err != nil {
 		return fmt.Errorf("Error running connection boilerplate: %s", err)
 	}
@@ -54,7 +62,13 @@ func cmdConfig(c CommandLine, api libmachine.API) error {
 	return nil
 }
 
-func runConnectionBoilerplate(h *host.Host, c CommandLine) (string, *auth.Options, error) {
+type ConnChecker interface {
+	Check(*host.Host, bool) (string, *auth.Options, error)
+}
+
+type MachineConnChecker struct{}
+
+func (mcc *MachineConnChecker) Check(h *host.Host, swarm bool) (string, *auth.Options, error) {
 	hostState, err := h.Driver.GetState()
 	if err != nil {
 		// TODO: This is a common operation and should have a commonly
@@ -70,7 +84,7 @@ func runConnectionBoilerplate(h *host.Host, c CommandLine) (string, *auth.Option
 		return "", &auth.Options{}, fmt.Errorf("Error getting driver URL: %s", err)
 	}
 
-	if c.Bool("swarm") {
+	if swarm {
 		var err error
 		dockerHost, err = parseSwarm(dockerHost, h)
 		if err != nil {

commands/env.go

@@ -21,8 +21,13 @@ const (
 var (
 	errImproperEnvArgs      = errors.New("Error: Expected one machine name")
 	errImproperUnsetEnvArgs = errors.New("Error: Expected no machine name when the -u flag is present")
+	defaultUsageHinter      UsageHintGenerator
 )
 
+func init() {
+	defaultUsageHinter = &EnvUsageHintGenerator{}
+}
+
 type ShellConfig struct {
 	Prefix          string
 	Delimiter       string
@@ -37,48 +42,62 @@ type ShellConfig struct {
 }
 
 func cmdEnv(c CommandLine, api libmachine.API) error {
+	var (
+		err      error
+		shellCfg *ShellConfig
+	)
+
 	// Ensure that log messages always go to stderr when this command is
 	// being run (it is intended to be run in a subshell)
 	log.SetOutWriter(os.Stderr)
 
 	if c.Bool("unset") {
-		return unset(c, api)
+		shellCfg, err = shellCfgUnset(c, api)
+		if err != nil {
+			return err
+		}
+	} else {
+		shellCfg, err = shellCfgSet(c, api)
+		if err != nil {
+			return err
+		}
 	}
-	return set(c, api)
+
+	return executeTemplateStdout(shellCfg)
 }
 
-func set(c CommandLine, api libmachine.API) error {
+func shellCfgSet(c CommandLine, api libmachine.API) (*ShellConfig, error) {
 	if len(c.Args()) != 1 {
-		return errImproperEnvArgs
+		return nil, errImproperEnvArgs
 	}
 
 	host, err := api.Load(c.Args().First())
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	dockerHost, _, err := runConnectionBoilerplate(host, c)
+	dockerHost, _, err := defaultConnChecker.Check(host, c.Bool("swarm"))
 	if err != nil {
-		return fmt.Errorf("Error running connection boilerplate: %s", err)
+		return nil, fmt.Errorf("Error checking TLS connection: %s", err)
 	}
 
-	userShell, err := getShell(c)
+	userShell, err := getShell(c.String("shell"))
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	shellCfg := &ShellConfig{
 		DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), host.Name),
 		DockerHost:      dockerHost,
 		DockerTLSVerify: "1",
-		UsageHint:       generateUsageHint(userShell, os.Args),
+		UsageHint:       defaultUsageHinter.GenerateUsageHint(userShell, os.Args),
 		MachineName:     host.Name,
 	}
 
 	if c.Bool("no-proxy") {
 		ip, err := host.Driver.GetIP()
 		if err != nil {
-			return fmt.Errorf("Error getting host IP: %s", err)
+			return nil, fmt.Errorf("Error getting host IP: %s", err)
 		}
 
 		noProxyVar, noProxyValue := findNoProxyFromEnv()
@@ -116,21 +135,21 @@ func set(c CommandLine, api libmachine.API) error {
 		shellCfg.Delimiter = "=\""
 	}
 
-	return executeTemplateStdout(shellCfg)
+	return shellCfg, nil
 }
 
-func unset(c CommandLine, api libmachine.API) error {
+func shellCfgUnset(c CommandLine, api libmachine.API) (*ShellConfig, error) {
 	if len(c.Args()) != 0 {
-		return errImproperUnsetEnvArgs
+		return nil, errImproperUnsetEnvArgs
 	}
 
-	userShell, err := getShell(c)
+	userShell, err := getShell(c.String("shell"))
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	shellCfg := &ShellConfig{
-		UsageHint: generateUsageHint(userShell, os.Args),
+		UsageHint: defaultUsageHinter.GenerateUsageHint(userShell, os.Args),
 	}
 
 	if c.Bool("no-proxy") {
@@ -156,7 +175,7 @@ func unset(c CommandLine, api libmachine.API) error {
 		shellCfg.Delimiter = ""
 	}
 
-	return executeTemplateStdout(shellCfg)
+	return shellCfg, nil
 }
 
 func executeTemplateStdout(shellCfg *ShellConfig) error {
@@ -169,8 +188,7 @@ func executeTemplateStdout(shellCfg *ShellConfig) error {
 	return tmpl.Execute(os.Stdout, shellCfg)
 }
 
-func getShell(c CommandLine) (string, error) {
+func getShell(userShell string) (string, error) {
-	userShell := c.String("shell")
 	if userShell != "" {
 		return userShell, nil
 	}
@@ -190,7 +208,13 @@ func findNoProxyFromEnv() (string, string) {
 	return noProxyVar, noProxyValue
 }
 
-func generateUsageHint(userShell string, args []string) string {
+type UsageHintGenerator interface {
+	GenerateUsageHint(string, []string) string
+}
+
+type EnvUsageHintGenerator struct{}
+
+func (g *EnvUsageHintGenerator) GenerateUsageHint(userShell string, args []string) string {
 	cmd := ""
 	comment := "#"
 

commands/env_test.go

@@ -1,13 +1,40 @@
 package commands
 
 import (
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
-	"strings"
+	"github.com/docker/machine/commands/commandstest"
-
+	"github.com/docker/machine/commands/mcndirs"
+	"github.com/docker/machine/drivers/fakedriver"
+	"github.com/docker/machine/libmachine"
+	"github.com/docker/machine/libmachine/auth"
+	"github.com/docker/machine/libmachine/host"
+	"github.com/docker/machine/libmachine/libmachinetest"
+	"github.com/docker/machine/libmachine/persist/persisttest"
 	"github.com/stretchr/testify/assert"
 )
 
+type FakeConnChecker struct {
+	DockerHost  string
+	AuthOptions *auth.Options
+	Err         error
+}
+
+func (fcc *FakeConnChecker) Check(_ *host.Host, _ bool) (string, *auth.Options, error) {
+	return fcc.DockerHost, fcc.AuthOptions, fcc.Err
+}
+
+type SimpleUsageHintGenerator struct {
+	Hint string
+}
+
+func (suhg *SimpleUsageHintGenerator) GenerateUsageHint(_ string, _ []string) string {
+	return suhg.Hint
+}
+
 func TestHints(t *testing.T) {
 	var tests = []struct {
 		userShell     string
@@ -40,8 +67,450 @@ func TestHints(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		hints := generateUsageHint(test.userShell, strings.Split(test.commandLine, " "))
+		hints := defaultUsageHinter.GenerateUsageHint(test.userShell, strings.Split(test.commandLine, " "))
-
 		assert.Equal(t, test.expectedHints, hints)
 	}
 }
+
+func revertUsageHinter(uhg UsageHintGenerator) {
+	defaultUsageHinter = uhg
+}
+
+func TestShellCfgSet(t *testing.T) {
+	const (
+		usageHint = "This is a usage hint"
+	)
+
+	// TODO: This should be embedded in some kind of wrapper struct for all
+	// these `env` operations.
+	defer revertUsageHinter(defaultUsageHinter)
+	defaultUsageHinter = &SimpleUsageHintGenerator{usageHint}
+
+	var tests = []struct {
+		description      string
+		commandLine      CommandLine
+		api              libmachine.API
+		connChecker      ConnChecker
+		noProxyVar       string
+		noProxyValue     string
+		expectedShellCfg *ShellConfig
+		expectedErr      error
+	}{
+		{
+			description: "no host name specified",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: nil,
+			},
+			expectedShellCfg: nil,
+			expectedErr:      errImproperEnvArgs,
+		},
+		{
+			description: "bash shell set happy path without any flags set",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: []string{"quux"},
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "bash",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{
+					Hosts: []*host.Host{
+						{
+							Name: "quux",
+						},
+					},
+				},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:          "export ",
+				Delimiter:       "=\"",
+				Suffix:          "\"\n",
+				DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), "quux"),
+				DockerHost:      "tcp://1.2.3.4:2376",
+				DockerTLSVerify: "1",
+				UsageHint:       usageHint,
+				MachineName:     "quux",
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "fish shell set happy path",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: []string{"quux"},
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "fish",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{
+					Hosts: []*host.Host{
+						{
+							Name: "quux",
+						},
+					},
+				},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:          "set -gx ",
+				Suffix:          "\";\n",
+				Delimiter:       " \"",
+				DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), "quux"),
+				DockerHost:      "tcp://1.2.3.4:2376",
+				DockerTLSVerify: "1",
+				UsageHint:       usageHint,
+				MachineName:     "quux",
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "powershell set happy path",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: []string{"quux"},
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "powershell",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{
+					Hosts: []*host.Host{
+						{
+							Name: "quux",
+						},
+					},
+				},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:          "$Env:",
+				Suffix:          "\"\n",
+				Delimiter:       " = \"",
+				DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), "quux"),
+				DockerHost:      "tcp://1.2.3.4:2376",
+				DockerTLSVerify: "1",
+				UsageHint:       usageHint,
+				MachineName:     "quux",
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "cmd.exe happy path",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: []string{"quux"},
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "cmd",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{
+					Hosts: []*host.Host{
+						{
+							Name: "quux",
+						},
+					},
+				},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:          "SET ",
+				Suffix:          "\n",
+				Delimiter:       "=",
+				DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), "quux"),
+				DockerHost:      "tcp://1.2.3.4:2376",
+				DockerTLSVerify: "1",
+				UsageHint:       usageHint,
+				MachineName:     "quux",
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "bash shell set happy path with --no-proxy flag; no existing environment variable set",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: []string{"quux"},
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "bash",
+						"swarm":    false,
+						"no-proxy": true,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{
+					Hosts: []*host.Host{
+						{
+							Name:   "quux",
+							Driver: &fakedriver.Driver{},
+						},
+					},
+				},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:          "export ",
+				Delimiter:       "=\"",
+				Suffix:          "\"\n",
+				DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), "quux"),
+				DockerHost:      "tcp://1.2.3.4:2376",
+				DockerTLSVerify: "1",
+				UsageHint:       usageHint,
+				NoProxyVar:      "NO_PROXY",
+				NoProxyValue:    "1.2.3.4", // From FakeDriver
+				MachineName:     "quux",
+			},
+			noProxyVar:   "NO_PROXY",
+			noProxyValue: "",
+			expectedErr:  nil,
+		},
+		{
+			description: "bash shell set happy path with --no-proxy flag; existing environment variable _is_ set",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: []string{"quux"},
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "bash",
+						"swarm":    false,
+						"no-proxy": true,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{
+					Hosts: []*host.Host{
+						{
+							Name:   "quux",
+							Driver: &fakedriver.Driver{},
+						},
+					},
+				},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:          "export ",
+				Delimiter:       "=\"",
+				Suffix:          "\"\n",
+				DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), "quux"),
+				DockerHost:      "tcp://1.2.3.4:2376",
+				DockerTLSVerify: "1",
+				UsageHint:       usageHint,
+				NoProxyVar:      "no_proxy",
+				NoProxyValue:    "192.168.59.1,1.2.3.4", // From FakeDriver
+				MachineName:     "quux",
+			},
+			noProxyVar:   "no_proxy",
+			noProxyValue: "192.168.59.1",
+			expectedErr:  nil,
+		},
+	}
+
+	for _, test := range tests {
+		// TODO: Ideally this should not hit the environment at all but
+		// rather should go through an interface.
+		os.Setenv(test.noProxyVar, test.noProxyValue)
+
+		t.Log(test.description)
+
+		defaultConnChecker = test.connChecker
+		shellCfg, err := shellCfgSet(test.commandLine, test.api)
+		assert.Equal(t, test.expectedShellCfg, shellCfg)
+		assert.Equal(t, test.expectedErr, err)
+
+		os.Unsetenv(test.noProxyVar)
+	}
+}
+
+func TestShellCfgUnset(t *testing.T) {
+	const (
+		usageHint = "This is the unset usage hint"
+	)
+
+	defer revertUsageHinter(defaultUsageHinter)
+	defaultUsageHinter = &SimpleUsageHintGenerator{usageHint}
+
+	var tests = []struct {
+		description      string
+		commandLine      CommandLine
+		api              libmachine.API
+		connChecker      ConnChecker
+		noProxyVar       string
+		noProxyValue     string
+		expectedShellCfg *ShellConfig
+		expectedErr      error
+	}{
+		{
+			description: "more than expected args passed in",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: []string{"foo", "bar"},
+			},
+			expectedShellCfg: nil,
+			expectedErr:      errImproperUnsetEnvArgs,
+		},
+		{
+			description: "bash shell unset happy path without any flags set",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: nil,
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "bash",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:    "unset ",
+				Suffix:    "\n",
+				Delimiter: "",
+				UsageHint: usageHint,
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "fish shell unset happy path",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: nil,
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "fish",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:    "set -e ",
+				Suffix:    ";\n",
+				Delimiter: "",
+				UsageHint: usageHint,
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "powershell unset happy path",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: nil,
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "powershell",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:    `Remove-Item Env:\\`,
+				Suffix:    "\n",
+				Delimiter: "",
+				UsageHint: usageHint,
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "cmd.exe unset happy path",
+			commandLine: &commandstest.FakeCommandLine{
+				CliArgs: nil,
+				LocalFlags: &commandstest.FakeFlagger{
+					Data: map[string]interface{}{
+						"shell":    "cmd",
+						"swarm":    false,
+						"no-proxy": false,
+					},
+				},
+			},
+			api: &libmachinetest.FakeAPI{
+				FakeStore: &persisttest.FakeStore{},
+			},
+			connChecker: &FakeConnChecker{
+				DockerHost:  "tcp://1.2.3.4:2376",
+				AuthOptions: nil,
+				Err:         nil,
+			},
+			expectedShellCfg: &ShellConfig{
+				Prefix:    "SET ",
+				Suffix:    "\n",
+				Delimiter: "=",
+				UsageHint: usageHint,
+			},
+			expectedErr: nil,
+		},
+		// TODO: There is kind of a funny bug (feature?) I discovered
+		// reasoning about unset() where if there was a NO_PROXY value
+		// set _before_ the original docker-machine env, it won't be
+		// restored (NO_PROXY won't be unset at all, it will stay the
+		// same).  We should define expected behavior in this case.
+	}
+
+	for _, test := range tests {
+		os.Setenv(test.noProxyVar, test.noProxyValue)
+
+		t.Log(test.description)
+
+		defaultConnChecker = test.connChecker
+		shellCfg, err := shellCfgUnset(test.commandLine, test.api)
+		assert.Equal(t, test.expectedShellCfg, shellCfg)
+		assert.Equal(t, test.expectedErr, err)
+
+		os.Setenv(test.noProxyVar, "")
+	}
+}

commands/ls_test.go

@@ -294,12 +294,14 @@ func captureStdout() (chan string, *os.File) {
 
 func TestGetHostListItems(t *testing.T) {
 	hostListItemsChan := make(chan HostListItem)
+	activeHostURL := "tcp://active.host.com:2376"
 
 	hosts := []*host.Host{
 		{
 			Name: "foo",
 			Driver: &fakedriver.Driver{
 				MockState: state.Running,
+				MockURL:   activeHostURL,
 			},
 			HostOptions: &host.Options{
 				SwarmOptions: &swarm.Options{},
@@ -335,6 +337,9 @@ func TestGetHostListItems(t *testing.T) {
 		"baz": {state.Error, false, "Unable to get url"},
 	}
 
+	// TODO: Ideally this would mockable via interface instead.
+	os.Setenv("DOCKER_HOST", activeHostURL)
+
 	items := []HostListItem{}
 	for _, host := range hosts {
 		go getHostState(host, hostListItemsChan)
@@ -350,6 +355,8 @@ func TestGetHostListItems(t *testing.T) {
 		assert.Equal(t, expected.active, item.Active)
 		assert.Equal(t, expected.error, item.Error)
 	}
+
+	os.Unsetenv("DOCKER_HOST")
 }
 
 // issue #1908

drivers/amazonec2/amazonec2_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/docker/machine/commands/commandstest"
 	"github.com/docker/machine/commands/mcndirs"
 	"github.com/docker/machine/drivers/amazonec2/amz"
 	"github.com/docker/machine/libmachine/drivers"
@@ -30,26 +31,6 @@ var (
 	}
 )
 
-type DriverOptionsMock struct {
-	Data map[string]interface{}
-}
-
-func (d DriverOptionsMock) String(key string) string {
-	return d.Data[key].(string)
-}
-
-func (d DriverOptionsMock) StringSlice(key string) []string {
-	return d.Data[key].([]string)
-}
-
-func (d DriverOptionsMock) Int(key string) int {
-	return d.Data[key].(int)
-}
-
-func (d DriverOptionsMock) Bool(key string) bool {
-	return d.Data[key].(bool)
-}
-
 func cleanup() error {
 	return os.RemoveAll(testStoreDir)
 }
@@ -63,8 +44,8 @@ func getTestStorePath() (string, error) {
 	return tmpDir, nil
 }
 
-func getDefaultTestDriverFlags() *DriverOptionsMock {
+func getDefaultTestDriverFlags() *commandstest.FakeFlagger {
-	return &DriverOptionsMock{
+	return &commandstest.FakeFlagger{
 		Data: map[string]interface{}{
 			"name":                            "test",
 			"url":                             "unix:///var/run/docker.sock",

drivers/fakedriver/fakedriver.go

@@ -43,6 +43,8 @@ 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
 }
 

drivers/softlayer/driver_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/docker/machine/commands/commandstest"
 	"github.com/docker/machine/commands/mcndirs"
 	"github.com/docker/machine/libmachine/drivers"
 	"github.com/stretchr/testify/assert"
@@ -17,38 +18,6 @@ const (
 	machineTestPrivateKey = "test-key"
 )
 
-type DriverOptionsMock struct {
-	Data map[string]interface{}
-}
-
-func (d DriverOptionsMock) String(key string) string {
-	if value, ok := d.Data[key]; ok {
-		return value.(string)
-	}
-	return ""
-}
-
-func (d DriverOptionsMock) StringSlice(key string) []string {
-	if value, ok := d.Data[key]; ok {
-		return value.([]string)
-	}
-	return []string{}
-}
-
-func (d DriverOptionsMock) Int(key string) int {
-	if value, ok := d.Data[key]; ok {
-		return value.(int)
-	}
-	return 0
-}
-
-func (d DriverOptionsMock) Bool(key string) bool {
-	if value, ok := d.Data[key]; ok {
-		return value.(bool)
-	}
-	return false
-}
-
 func cleanup() error {
 	return os.RemoveAll(testStoreDir)
 }
@@ -62,8 +31,8 @@ func getTestStorePath() (string, error) {
 	return tmpDir, nil
 }
 
-func getDefaultTestDriverFlags() *DriverOptionsMock {
+func getDefaultTestDriverFlags() *commandstest.FakeFlagger {
-	return &DriverOptionsMock{
+	return &commandstest.FakeFlagger{
 		Data: map[string]interface{}{
 			"name":                   "test",
 			"url":                    "unix:///var/run/docker.sock",

libmachine/libmachinetest/fake_api.go

@@ -0,0 +1,26 @@
+package libmachinetest
+
+import (
+	"github.com/docker/machine/libmachine/drivers"
+	"github.com/docker/machine/libmachine/host"
+	"github.com/docker/machine/libmachine/persist/persisttest"
+)
+
+type FakeAPI struct {
+	*persisttest.FakeStore
+	FakeNewDriver                             drivers.Driver
+	FakeNewHost                               *host.Host
+	NewPluginDriverErr, NewHostErr, CreateErr error
+}
+
+func (fapi *FakeAPI) NewPluginDriver(string, []byte) (drivers.Driver, error) {
+	return fapi.FakeNewDriver, fapi.NewPluginDriverErr
+}
+
+func (fapi *FakeAPI) NewHost(drivers.Driver) (*host.Host, error) {
+	return fapi.FakeNewHost, fapi.NewHostErr
+}
+
+func (fapi *FakeAPI) Create(h *host.Host) error {
+	return fapi.CreateErr
+}

libmachine/persist/filestore_test.go

@@ -3,29 +3,16 @@ package persist
 import (
 	"fmt"
 	"io/ioutil"
+	"log"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/docker/machine/commands/mcndirs"
-	"github.com/docker/machine/drivers/none"
+	_ "github.com/docker/machine/drivers/none"
-	"github.com/docker/machine/libmachine/drivers"
 	"github.com/docker/machine/libmachine/hosttest"
-	"github.com/stretchr/testify/assert"
 )
 
-const (
-	storedDriverURL = "1.2.3.4"
-)
-
-type FakePluginDriverFactory struct {
-	drivers.Driver
-}
-
-func (fpdf *FakePluginDriverFactory) NewPluginDriver(string, []byte) (drivers.Driver, error) {
-	return fpdf.Driver, nil
-}
-
 func cleanup() {
 	os.RemoveAll(os.Getenv("MACHINE_STORAGE_PATH"))
 }
@@ -43,11 +30,6 @@ func getTestStore() Filestore {
 		Path:             tmpDir,
 		CaCertPath:       filepath.Join(tmpDir, "certs", "ca-cert.pem"),
 		CaPrivateKeyPath: filepath.Join(tmpDir, "certs", "ca-key.pem"),
-		PluginDriverFactory: &FakePluginDriverFactory{
-			&none.Driver{
-				URL: storedDriverURL,
-			},
-		},
 	}
 }
 
@@ -165,9 +147,13 @@ func TestStoreExists(t *testing.T) {
 	}
 }
 
-func TestStoreSaveLoad(t *testing.T) {
+func TestStoreLoad(t *testing.T) {
 	defer cleanup()
 
+	expectedURL := "unix:///foo/baz"
+	flags := hosttest.GetTestDriverFlags()
+	flags.Data["url"] = expectedURL
+
 	store := getTestStore()
 
 	h, err := hosttest.GetDefaultTestHost()
@@ -175,13 +161,17 @@ func TestStoreSaveLoad(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	if err := h.Driver.SetConfigFromFlags(flags); err != nil {
+		t.Fatal(err)
+	}
+
 	if err := store.Save(h); err != nil {
 		t.Fatal(err)
 	}
 
 	h, err = store.Load(h.Name)
 	if err != nil {
-		t.Fatal(err)
+		log.Fatal(err)
 	}
 
 	actualURL, err := h.GetURL()
@@ -189,5 +179,7 @@ func TestStoreSaveLoad(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	assert.Equal(t, storedDriverURL, actualURL)
+	if actualURL != expectedURL {
+		t.Fatalf("GetURL is not %q, got %q", expectedURL, actualURL)
+	}
 }

libmachine/persist/persisttest/fake_store.go

@@ -0,0 +1,50 @@
+package persisttest
+
+import "github.com/docker/machine/libmachine/host"
+
+type FakeStore struct {
+	Hosts                                           []*host.Host
+	ExistsErr, ListErr, LoadErr, RemoveErr, SaveErr error
+}
+
+func (fs *FakeStore) Exists(name string) (bool, error) {
+	if fs.ExistsErr != nil {
+		return false, fs.ExistsErr
+	}
+	for _, h := range fs.Hosts {
+		if h.Name == name {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}
+
+func (fs *FakeStore) List() ([]string, error) {
+	names := []string{}
+	for _, h := range fs.Hosts {
+		names = append(names, h.Name)
+	}
+	return names, fs.ListErr
+}
+
+func (fs *FakeStore) Load(name string) (*host.Host, error) {
+	if fs.LoadErr != nil {
+		return nil, fs.LoadErr
+	}
+	for _, h := range fs.Hosts {
+		if h.Name == name {
+			return h, nil
+		}
+	}
+
+	return nil, nil
+}
+
+func (fs *FakeStore) Remove(name string) error {
+	return fs.RemoveErr
+}
+
+func (fs *FakeStore) Save(host *host.Host) error {
+	return fs.SaveErr
+}