FIX docker-archive-public#2746 Improve shell detection in bugsnag reports

dgageot · dgageot · commit fc025e198415 · 2016-01-06T16:57:33.000+01:00
Signed-off-by: David Gageot &lt;david@gageot.net&gt;
diff --git a/commands/env.go b/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 (
diff --git a/libmachine/crashreport/crash_report.go b/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)
 	}
 }
diff --git a/libmachine/shell/shell.go b/libmachine/shell/shell.go
diff --git a/libmachine/shell/shell_test.go b/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)
 }
diff --git a/libmachine/shell/shell_unix_test.go b/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)
 }
diff --git a/libmachine/shell/shell_windows.go b/libmachine/shell/shell_windows.go
diff --git a/libmachine/shell/shell_windows_test.go b/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)
 }

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ import (`
`17`	`17`
`18`	`18`	`"github.com/bugsnag/bugsnag-go"`
`19`	`19`	`"github.com/docker/machine/libmachine/log"`
	`20`	`+ "github.com/docker/machine/libmachine/shell"`
`20`	`21`	`"github.com/docker/machine/version"`
`21`	`22`	`)`
`22`	`23`
`@@ -125,12 +126,8 @@ func addFile(path string, metaData *bugsnag.MetaData) {`
`125`	`126`	`}`
`126`	`127`
`127`	`128`	`func detectRunningShell(metaData *bugsnag.MetaData) {`
`128`		`- shell := os.Getenv("SHELL")`
`129`		`- if shell != "" {`
`130`		`- metaData.Add("device", "shell", shell)`
`131`		`- }`
`132`		`- shell = os.Getenv("__fish_bin_dir")`
`133`		`- if shell != "" {`
	`129`	`+ shell, err := shell.Detect()`
	`130`	`+ if err == nil {`
`134`	`131`	`metaData.Add("device", "shell", shell)`
`135`	`132`	`}`
`136`	`133`	`}`