Fix the warning which found by lgtm.com (#293)

* Fix the warning which found by lgtm.com

* Add test cases for shell command

* Fix the issues found by hound

* Remove duplicate methods

* Reuse the fake exec cmd function

app/cmd/plugin_search.go

@@ -3,10 +3,11 @@ package cmd
 import (
 	"bytes"
 	"fmt"
-	"github.com/jenkins-zh/jenkins-cli/app/i18n"
 	"net/http"
 	"strings"
 
+	"github.com/jenkins-zh/jenkins-cli/app/i18n"
+
 	"github.com/jenkins-zh/jenkins-cli/app/helper"
 
 	"github.com/jenkins-zh/jenkins-cli/client"
@@ -152,7 +153,6 @@ func buildInstalledPlugins(installedPlugins *client.InstalledPluginList, plugin
 }
 
 func buildNoMatchPlugins(plugin client.AvailablePlugin, result []client.CenterPlugin) (resultData []client.CenterPlugin) {
-	resultData = result
 	resultPlugin := client.CenterPlugin{}
 	resultPlugin.CompatibleWithInstalledVersion = false
 	resultPlugin.Name = plugin.Name

app/cmd/shell.go

@@ -2,10 +2,10 @@ package cmd
 
 import (
 	"fmt"
+	"github.com/jenkins-zh/jenkins-cli/util"
 	"gopkg.in/yaml.v2"
 	"io/ioutil"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"runtime"
 
@@ -16,6 +16,16 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// ShellOptions is the option of shell command
+type ShellOptions struct {
+	CommonOption
+
+	TmpDir            string
+	TmpConfigFileName string
+}
+
+var shellOptions ShellOptions
+
 func init() {
 	rootCmd.AddCommand(shellCmd)
 }
@@ -49,27 +59,24 @@ var shellCmd = &cobra.Command{
 	Short:   i18n.T("Create a sub shell so that changes to a specific Jenkins remain local to the shell."),
 	Long:    i18n.T("Create a sub shell so that changes to a specific Jenkins remain local to the shell."),
 	Aliases: []string{"sh"},
-	PreRun: func(cmd *cobra.Command, args []string) {
+	PreRunE: func(cmd *cobra.Command, args []string) (err error) {
 		if len(args) > 0 {
 			jenkinsName := args[0]
 			setCurrentJenkins(jenkinsName)
 		}
-	},
-	RunE: func(cmd *cobra.Command, _ []string) error {
-		tmpDirName, err := ioutil.TempDir("", ".jcli-shell-")
-		if err != nil {
-			return err
-		}
-		tmpConfigFileName := filepath.Join(tmpDirName, "/config")
 
-		var data []byte
-		config := getConfig()
-		if data, err = yaml.Marshal(&config); err == nil {
-			err = ioutil.WriteFile(tmpConfigFileName, data, 0644)
-		} else {
-			return err
-		}
+		if shellOptions.TmpDir, err = ioutil.TempDir("", ".jcli-shell-"); err == nil {
+			shellOptions.TmpConfigFileName = filepath.Join(shellOptions.TmpDir, "/config")
 
+			var data []byte
+			config := getConfig()
+			if data, err = yaml.Marshal(&config); err == nil {
+				err = ioutil.WriteFile(shellOptions.TmpConfigFileName, data, 0644)
+			}
+		}
+		return
+	},
+	RunE: func(cmd *cobra.Command, _ []string) (err error) {
 		fullShell := os.Getenv("SHELL")
 		shell := filepath.Base(fullShell)
 		if fullShell == "" && runtime.GOOS == "windows" {
@@ -78,8 +85,8 @@ var shellCmd = &cobra.Command{
 		}
 
 		prompt := createNewBashPrompt(os.Getenv("PS1"))
-		rcFile := defaultRcFile + "\nexport PS1=" + prompt + "\nexport JCLI_CONFIG=\"" + tmpConfigFileName + "\"\n"
-		tmpRCFileName := tmpDirName + "/.bashrc"
+		rcFile := defaultRcFile + "\nexport PS1=" + prompt + "\nexport JCLI_CONFIG=\"" + shellOptions.TmpConfigFileName + "\"\n"
+		tmpRCFileName := shellOptions.TmpDir + "/.bashrc"
 
 		err = ioutil.WriteFile(tmpRCFileName, []byte(rcFile), 0760)
 		if err != nil {
@@ -87,16 +94,16 @@ var shellCmd = &cobra.Command{
 		}
 
 		logger.Debug("temporary shell profile loaded", zap.String("path", tmpRCFileName))
-		e := exec.Command(shell, "-rcfile", tmpRCFileName, "-i")
+		e := util.ExecCommand(shellOptions.ExecContext, shell, "-rcfile", tmpRCFileName, "-i")
 		if shell == "zsh" {
 			env := os.Environ()
-			env = append(env, fmt.Sprintf("ZDOTDIR=%s", tmpDirName))
-			e = exec.Command(shell, "-i")
+			env = append(env, fmt.Sprintf("ZDOTDIR=%s", shellOptions.TmpDir))
+			e = util.ExecCommand(shellOptions.ExecContext, shell, "-i")
 			e.Env = env
 		} else if shell == "cmd.exe" {
 			env := os.Environ()
-			env = append(env, fmt.Sprintf("JCLI_CONFIG=%s", tmpConfigFileName))
-			e = exec.Command(shell)
+			env = append(env, fmt.Sprintf("JCLI_CONFIG=%s", shellOptions.TmpConfigFileName))
+			e = util.ExecCommand(shellOptions.ExecContext, shell)
 			e.Env = env
 		}
 
@@ -104,10 +111,11 @@ var shellCmd = &cobra.Command{
 		e.Stderr = cmd.OutOrStderr()
 		e.Stdin = os.Stdin
 		err = e.Run()
-		if deleteError := os.RemoveAll(tmpDirName); deleteError != nil {
-			panic(err)
-		}
-		return err
+		return
+	},
+	PostRunE: func(cmd *cobra.Command, args []string) (err error) {
+		err = os.RemoveAll(shellOptions.TmpDir)
+		return
 	},
 }
 

app/cmd/shell_test.go

+package cmd
+
+import (
+	"bytes"
+	"github.com/jenkins-zh/jenkins-cli/util"
+	"io/ioutil"
+	"os"
+
+	"github.com/golang/mock/gomock"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("shell command", func() {
+	var (
+		ctrl *gomock.Controller
+	)
+
+	BeforeEach(func() {
+		ctrl = gomock.NewController(GinkgoT())
+		rootCmd.SetArgs([]string{})
+		rootOptions.Jenkins = ""
+		rootOptions.ConfigFile = "test.yaml"
+		shellOptions.ExecContext = util.FakeExecCommandSuccess
+	})
+
+	AfterEach(func() {
+		rootCmd.SetArgs([]string{})
+		os.Remove(rootOptions.ConfigFile)
+		rootOptions.ConfigFile = ""
+		ctrl.Finish()
+	})
+
+	Context("basic test", func() {
+		It("should success", func() {
+			data, err := generateSampleConfig()
+			Expect(err).To(BeNil())
+			err = ioutil.WriteFile(rootOptions.ConfigFile, data, 0664)
+			Expect(err).To(BeNil())
+
+			rootCmd.SetArgs([]string{"shell", "yourServer"})
+
+			buf := new(bytes.Buffer)
+			rootCmd.SetOutput(buf)
+			_, err = rootCmd.ExecuteC()
+			Expect(err).To(BeNil())
+
+			Expect(buf.String()).To(ContainSubstring("testing: warning: no tests to run\nPASS\n"))
+		})
+	})
+})

util/cmd.go

@@ -22,11 +22,7 @@ func Open(url string, cmdContext ExecContext) error {
 		cmd = "xdg-Open"
 	}
 	args = append(args, url)
-
-	if cmdContext == nil {
-		cmdContext = exec.Command
-	}
-	return cmdContext(cmd, args...).Start()
+	return ExecCommand(cmdContext, cmd, args...).Start()
 }
 
 // Exec is the wrapper of syscall.Exec
@@ -75,3 +71,11 @@ func FakeSystemCallExecSuccess(argv0 string, argv []string, envv []string) (err
 func FakeLookPath(path string) (string, error) {
 	return path, nil
 }
+
+// ExecCommand is a warp of exec.Command
+func ExecCommand(context ExecContext, name string, arg ...string) *exec.Cmd {
+	if context == nil {
+		context = exec.Command
+	}
+	return context(name, arg...)
+}

util/cmd_test.go

@@ -3,6 +3,8 @@ package util
 import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"os"
+	"testing"
 )
 
 var _ = Describe("Test open browser", func() {
@@ -11,3 +13,11 @@ var _ = Describe("Test open browser", func() {
 		Expect(err).To(BeNil())
 	})
 })
+
+// TestShellProcessSuccess only for test
+func TestShellProcessSuccess(t *testing.T) {
+	if os.Getenv("GO_TEST_PROCESS") != "1" {
+		return
+	}
+	//os.Exit(0)
+}