Merge pull request #2754 from dgageot/shell-bugsnag

Improve shell detection in bugsnag reports

commands/env.go

@@ -9,10 +9,10 @@ import (
 	"text/template"
 
 	"github.com/docker/machine/commands/mcndirs"
-	"github.com/docker/machine/commands/shell"
 	"github.com/docker/machine/libmachine"
 	"github.com/docker/machine/libmachine/check"
 	"github.com/docker/machine/libmachine/log"
+	"github.com/docker/machine/libmachine/shell"
 )
 
 const (

libmachine/crashreport/crash_report.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/bugsnag/bugsnag-go"
 	"github.com/docker/machine/libmachine/log"
+	"github.com/docker/machine/libmachine/shell"
 	"github.com/docker/machine/version"
 )
 
@@ -125,12 +126,8 @@ func addFile(path string, metaData *bugsnag.MetaData) {
 }
 
 func detectRunningShell(metaData *bugsnag.MetaData) {
-	shell := os.Getenv("SHELL")
-	if shell != "" {
-		metaData.Add("device", "shell", shell)
-	}
-	shell = os.Getenv("__fish_bin_dir")
-	if shell != "" {
+	shell, err := shell.Detect()
+	if err == nil {
 		metaData.Add("device", "shell", shell)
 	}
 }

libmachine/shell/shell_test.go

@@ -8,22 +8,24 @@ import (
 )
 
 func TestDetectBash(t *testing.T) {
-	originalShell := os.Getenv("SHELL")
+	defer func(shell string) { os.Setenv("SHELL", shell) }(os.Getenv("SHELL"))
 	os.Setenv("SHELL", "/bin/bash")
-	defer os.Setenv("SHELL", originalShell)
+
 	shell, err := Detect()
-	assert.Nil(t, err)
+
 	assert.Equal(t, "bash", shell)
+	assert.NoError(t, err)
 }
 
 func TestDetectFish(t *testing.T) {
-	originalShell := os.Getenv("SHELL")
+	defer func(shell string) { os.Setenv("SHELL", shell) }(os.Getenv("SHELL"))
 	os.Setenv("SHELL", "/bin/bash")
-	defer os.Setenv("SHELL", originalShell)
-	originalFishdir := os.Getenv("__fish_bin_dir")
+
+	defer func(fishDir string) { os.Setenv("__fish_bin_dir", fishDir) }(os.Getenv("__fish_bin_dir"))
 	os.Setenv("__fish_bin_dir", "/usr/local/Cellar/fish/2.2.0/bin")
-	defer os.Setenv("__fish_bin_dir", originalFishdir)
+
 	shell, err := Detect()
-	assert.Nil(t, err)
+
 	assert.Equal(t, "fish", shell)
+	assert.NoError(t, err)
 }

libmachine/shell/shell_unix_test.go

@@ -3,19 +3,18 @@
 package shell
 
 import (
-	"fmt"
 	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestUnknowShell(t *testing.T) {
-	originalShell := os.Getenv("SHELL")
+func TestUnknownShell(t *testing.T) {
+	defer func(shell string) { os.Setenv("SHELL", shell) }(os.Getenv("SHELL"))
 	os.Setenv("SHELL", "")
-	defer os.Setenv("SHELL", originalShell)
+
 	shell, err := Detect()
-	fmt.Println(shell)
+
 	assert.Equal(t, err, ErrUnknownShell)
-	assert.Equal(t, "", shell)
+	assert.Empty(t, shell)
 }

libmachine/shell/shell_windows_test.go

@@ -8,17 +8,18 @@ import (
 )
 
 func TestDetect(t *testing.T) {
-	originalShell := os.Getenv("SHELL")
+	defer func(shell string) { os.Setenv("SHELL", shell) }(os.Getenv("SHELL"))
 	os.Setenv("SHELL", "")
-	defer os.Setenv("SHELL", originalShell)
+
 	shell, err := Detect()
-	assert.Nil(t, err)
+
 	assert.Equal(t, "cmd", shell)
+	assert.NoError(t, err)
 }
 
 func TestStartedBy(t *testing.T) {
 	shell, err := startedBy()
-	assert.Nil(t, err)
-	assert.NotNil(t, shell)
+
 	assert.Equal(t, "go.exe", shell)
+	assert.NoError(t, err)
 }