Merge pull request #8334 from n0npax/add-tests-to-minikube-shell

Add tests to minikube shell & refactor shell.go

pkg/minikube/shell/shell.go

@@ -28,49 +28,6 @@ import (
 	"github.com/docker/machine/libmachine/shell"
 )
 
-const (
-	fishSetPfx   = "set -gx "
-	fishSetSfx   = "\";\n" // semi-colon required for fish 2.7
-	fishSetDelim = " \""
-
-	fishUnsetPfx = "set -e "
-	fishUnsetSfx = ";\n"
-
-	psSetPfx   = "$Env:"
-	psSetSfx   = "\"\n"
-	psSetDelim = " = \""
-
-	psUnsetPfx = `Remove-Item Env:\\`
-	psUnsetSfx = "\n"
-
-	cmdSetPfx   = "SET "
-	cmdSetSfx   = "\n"
-	cmdSetDelim = "="
-
-	cmdUnsetPfx   = "SET "
-	cmdUnsetSfx   = "\n"
-	cmdUnsetDelim = "="
-
-	emacsSetPfx   = "(setenv \""
-	emacsSetSfx   = "\")\n"
-	emacsSetDelim = "\" \""
-
-	emacsUnsetPfx   = "(setenv \""
-	emacsUnsetSfx   = ")\n"
-	emacsUnsetDelim = "\" nil"
-
-	bashSetPfx   = "export "
-	bashSetSfx   = "\"\n"
-	bashSetDelim = "=\""
-
-	bashUnsetPfx = "unset "
-	bashUnsetSfx = "\n"
-
-	nonePfx   = ""
-	noneSfx   = "\n"
-	noneDelim = "="
-)
-
 // Config represents the shell config
 type Config struct {
 	Prefix    string
@@ -79,6 +36,102 @@ type Config struct {
 	UsageHint string
 }
 
+type shellData struct {
+	prefix         string
+	suffix         string
+	delimiter      string
+	unsetPrefix    string
+	unsetSuffix    string
+	unsetDelimiter string
+	usageHint      func(s ...interface{}) string
+}
+
+var shellConfigMap = map[string]shellData{
+	"fish": shellData{
+		prefix:         "set -gx ",
+		suffix:         "\";\n",
+		delimiter:      " \"",
+		unsetPrefix:    "set -e ",
+		unsetSuffix:    ";\n",
+		unsetDelimiter: "",
+		usageHint: func(s ...interface{}) string {
+			return fmt.Sprintf(`
+# %s
+# %s | source
+`, s...)
+		},
+	},
+	"powershell": shellData{
+		prefix:         "$Env:",
+		suffix:         "\"\n",
+		delimiter:      " = \"",
+		unsetPrefix:    `Remove-Item Env:\\`,
+		unsetSuffix:    "\n",
+		unsetDelimiter: "",
+		usageHint: func(s ...interface{}) string {
+			return fmt.Sprintf(`# %s
+# & %s | Invoke-Expression
+`, s...)
+		},
+	},
+	"cmd": shellData{
+		prefix:         "SET ",
+		suffix:         "\n",
+		delimiter:      "=",
+		unsetPrefix:    "SET ",
+		unsetSuffix:    "\n",
+		unsetDelimiter: "=",
+		usageHint: func(s ...interface{}) string {
+			return fmt.Sprintf(`REM %s
+REM @FOR /f "tokens=*" %%i IN ('%s') DO @%%i
+`, s...)
+		},
+	},
+	"emacs": shellData{
+		prefix:         "(setenv \"",
+		suffix:         "\")\n",
+		delimiter:      "\" \"",
+		unsetPrefix:    "(setenv \"",
+		unsetSuffix:    ")\n",
+		unsetDelimiter: "\" nil",
+		usageHint: func(s ...interface{}) string {
+			return fmt.Sprintf(`;; %s
+;; (with-temp-buffer (shell-command "%s" (current-buffer)) (eval-buffer))
+`, s...)
+		},
+	},
+	"bash": shellData{
+		prefix:         "export ",
+		suffix:         "\"\n",
+		delimiter:      "=\"",
+		unsetPrefix:    "unset ",
+		unsetSuffix:    "\n",
+		unsetDelimiter: "",
+		usageHint: func(s ...interface{}) string {
+			return fmt.Sprintf(`
+# %s
+# eval $(%s)
+`, s...)
+		},
+	},
+	"none": shellData{
+		prefix:         "",
+		suffix:         "\n",
+		delimiter:      "=",
+		unsetPrefix:    "",
+		unsetSuffix:    "\n",
+		unsetDelimiter: "",
+		usageHint: func(s ...interface{}) string {
+			return fmt.Sprintf(`
+# %s
+# eval $(%s)
+`, s...)
+		},
+	},
+}
+
+var defaultShell shellData = shellConfigMap["bash"]
+
 var (
 	// ForceShell forces a shell name
 	ForceShell string
@@ -89,67 +142,26 @@ func Detect() (string, error) {
 	return shell.Detect()
 }
 
-func generateUsageHint(sh, usgPlz, usgCmd string) string {
-	var usageHintMap = map[string]string{
-		"bash": fmt.Sprintf(`
-# %s
-# eval $(%s)
-`, usgPlz, usgCmd),
-		"fish": fmt.Sprintf(`
-# %s
-# %s | source
-`, usgPlz, usgCmd),
-		"powershell": fmt.Sprintf(`# %s
-# & %s | Invoke-Expression
-`, usgPlz, usgCmd),
-		"cmd": fmt.Sprintf(`REM %s
-REM @FOR /f "tokens=*" %%i IN ('%s') DO @%%i
-`, usgPlz, usgCmd),
-		"emacs": fmt.Sprintf(`;; %s
-;; (with-temp-buffer (shell-command "%s" (current-buffer)) (eval-buffer))
-`, usgPlz, usgCmd),
-	}
-
-	hint, ok := usageHintMap[sh]
+func (c EnvConfig) getShell() shellData {
+	shell, ok := shellConfigMap[c.Shell]
 	if !ok {
-		return usageHintMap["bash"]
+		shell = defaultShell
 	}
-	return hint
+	return shell
+}
+
+func generateUsageHint(ec EnvConfig, usgPlz, usgCmd string) string {
+	shellCfg := ec.getShell()
+	return shellCfg.usageHint(usgPlz, usgCmd)
 }
 
 // CfgSet generates context variables for shell
 func CfgSet(ec EnvConfig, plz, cmd string) *Config {
-	s := &Config{
-		UsageHint: generateUsageHint(ec.Shell, plz, cmd),
-	}
+	shellCfg := ec.getShell()
+	s := &Config{}
+	s.Suffix, s.Prefix, s.Delimiter = shellCfg.suffix, shellCfg.prefix, shellCfg.delimiter
+	s.UsageHint = generateUsageHint(ec, plz, cmd)
 
-	switch ec.Shell {
-	case "fish":
-		s.Prefix = fishSetPfx
-		s.Suffix = fishSetSfx
-		s.Delimiter = fishSetDelim
-	case "powershell":
-		s.Prefix = psSetPfx
-		s.Suffix = psSetSfx
-		s.Delimiter = psSetDelim
-	case "cmd":
-		s.Prefix = cmdSetPfx
-		s.Suffix = cmdSetSfx
-		s.Delimiter = cmdSetDelim
-	case "emacs":
-		s.Prefix = emacsSetPfx
-		s.Suffix = emacsSetSfx
-		s.Delimiter = emacsSetDelim
-	case "none":
-		s.Prefix = nonePfx
-		s.Suffix = noneSfx
-		s.Delimiter = noneDelim
-		s.UsageHint = ""
-	default:
-		s.Prefix = bashSetPfx
-		s.Suffix = bashSetSfx
-		s.Delimiter = bashSetDelim
-	}
 	return s
 }
 
@@ -167,25 +179,20 @@ func SetScript(ec EnvConfig, w io.Writer, envTmpl string, data interface{}) erro
 // UnsetScript writes out a shell-compatible unset script
 func UnsetScript(ec EnvConfig, w io.Writer, vars []string) error {
 	var sb strings.Builder
+	shellCfg := ec.getShell()
+	pfx, sfx, delim := shellCfg.unsetPrefix, shellCfg.unsetSuffix, shellCfg.unsetDelimiter
 	switch ec.Shell {
-	case "fish":
-		for _, v := range vars {
-			sb.WriteString(fmt.Sprintf("%s%s%s", fishUnsetPfx, v, fishUnsetSfx))
-		}
+	case "cmd", "emacs", "fish":
+		break
 	case "powershell":
-		sb.WriteString(fmt.Sprintf("%s%s%s", psUnsetPfx, strings.Join(vars, " Env:\\\\"), psUnsetSfx))
-	case "cmd":
-		for _, v := range vars {
-			sb.WriteString(fmt.Sprintf("%s%s%s%s", cmdUnsetPfx, v, cmdUnsetDelim, cmdUnsetSfx))
-		}
-	case "emacs":
-		for _, v := range vars {
-			sb.WriteString(fmt.Sprintf("%s%s%s%s", emacsUnsetPfx, v, emacsUnsetDelim, emacsUnsetSfx))
-		}
-	case "none":
-		sb.WriteString(fmt.Sprintf("%s%s%s", nonePfx, strings.Join(vars, " "), noneSfx))
+		vars = []string{strings.Join(vars, " Env:\\\\")}
 	default:
-		sb.WriteString(fmt.Sprintf("%s%s%s", bashUnsetPfx, strings.Join(vars, " "), bashUnsetSfx))
+		vars = []string{strings.Join(vars, " ")}
+	}
+	for _, v := range vars {
+		if _, err := sb.WriteString(fmt.Sprintf("%s%s%s%s", pfx, v, delim, sfx)); err != nil {
+			return err
+		}
 	}
 	_, err := w.Write([]byte(sb.String()))
 	return err

pkg/minikube/shell/shell_test.go

+/*
+Copyright 2020 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package shell
+
+import (
+	"bytes"
+	"os"
+	"strings"
+	"testing"
+)
+
+func TestGenerateUsageHint(t *testing.T) {
+	var testCases = []struct {
+		ec       EnvConfig
+		expected string
+	}{
+		{EnvConfig{""}, `# foo
+# eval $(bar)`},
+		{EnvConfig{"powershell"}, `# foo
+# & bar | Invoke-Expression`},
+		{EnvConfig{"bash"}, `# foo
+# eval $(bar)`},
+		{EnvConfig{"powershell"}, `# foo
+# & bar | Invoke-Expression`},
+		{EnvConfig{"emacs"}, `;; foo
+;; (with-temp-buffer (shell-command "bar" (current-buffer)) (eval-buffer))`},
+		{EnvConfig{"fish"}, `# foo
+# bar | source`},
+		{EnvConfig{"none"}, `# foo
+# eval $(bar)`},
+	}
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.ec.Shell, func(t *testing.T) {
+			got := strings.TrimSpace(generateUsageHint(tc.ec, "foo", "bar"))
+			expected := strings.TrimSpace(tc.expected)
+			if got != expected {
+				t.Errorf("Expected '%v' but got '%v'", expected, got)
+			}
+		})
+	}
+}
+
+func TestCfgSet(t *testing.T) {
+	var testCases = []struct {
+		plz, cmd string
+		ec       EnvConfig
+		expected string
+	}{
+		{"", "eval", EnvConfig{""}, `"`},
+		{"", "eval", EnvConfig{"bash"}, `"`},
+		{"", "eval", EnvConfig{"powershell"}, `"`},
+		{"", "eval", EnvConfig{"cmd"}, ``},
+		{"", "eval", EnvConfig{"emacs"}, `")`},
+		{"", "eval", EnvConfig{"none"}, ``},
+		{"", "eval", EnvConfig{"fish"}, `";`},
+	}
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.ec.Shell, func(t *testing.T) {
+			conf := CfgSet(tc.ec, tc.plz, tc.cmd)
+			expected := strings.TrimSpace(tc.expected)
+			got := strings.TrimSpace(conf.Suffix)
+			if expected != got {
+				t.Errorf("Expected suffix '%v' but got '%v'", expected, got)
+			}
+		})
+	}
+}
+
+func TestUnsetScript(t *testing.T) {
+	var testCases = []struct {
+		vars     []string
+		ec       EnvConfig
+		expected string
+	}{
+		{[]string{"baz", "bar"}, EnvConfig{""}, `unset baz bar`},
+		{[]string{"baz", "bar"}, EnvConfig{"bash"}, `unset baz bar`},
+		{[]string{"baz", "bar"}, EnvConfig{"powershell"}, `Remove-Item Env:\\baz Env:\\bar`},
+		{[]string{"baz", "bar"}, EnvConfig{"cmd"}, `SET baz=
+SET bar=`},
+		{[]string{"baz", "bar"}, EnvConfig{"fish"}, `set -e baz;
+set -e bar;`},
+		{[]string{"baz", "bar"}, EnvConfig{"emacs"}, `(setenv "baz" nil)
+(setenv "bar" nil)`},
+		{[]string{"baz", "bar"}, EnvConfig{"none"}, `baz bar`},
+	}
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.ec.Shell, func(t *testing.T) {
+			var b bytes.Buffer
+
+			if err := UnsetScript(tc.ec, &b, tc.vars); err != nil {
+				t.Fatalf("Unexpected error when unseting script happen: %v", err)
+			} else {
+				writtenMessage := strings.TrimSpace(b.String())
+				expected := strings.TrimSpace(tc.expected)
+				if writtenMessage != expected {
+					t.Fatalf("Expected '%v' but got '%v' ", tc.expected, writtenMessage)
+				}
+			}
+		})
+	}
+}
+
+func TestDetect(t *testing.T) {
+	orgShellEnv := os.Getenv("SHELL")
+	defer os.Setenv("SHELL", orgShellEnv)
+
+	os.Setenv("SHELL", "bash")
+	if s, err := Detect(); err != nil {
+		t.Fatalf("unexpected error: '%v' during shell detection. Returned shell: %s", err, s)
+	} else if s == "" {
+		t.Fatalf("Detected shell expected to be non empty string")
+	}
+}
+
+func TestSetScript(t *testing.T) {
+	ec := EnvConfig{"bash"}
+	var w bytes.Buffer
+	if err := SetScript(ec, &w, "foo", nil); err != nil {
+		t.Fatalf("Unexpected error: '%v' during Setting script", err)
+	}
+	if w.String() != "foo" {
+		t.Fatalf("Expected foo writed by SetScript, but got '%v'", w.String())
+	}
+	if ec.Shell == "" {
+		t.Fatalf("Expected no empty shell")
+	}
+}