Merge pull request #7 from SimonBaeumer/bug-fixes

Bug fixes

Author: SimonBaeumer

README.md

@@ -26,25 +26,32 @@ fmt.Println(c.Stdout())
 fmt.Println(c.Stderr())
 ```
 
-### Stream output to stderr and stdout
+### Configure the command
+
+To configure the command a option function will be passed which receives the command object as an argument passed by reference.
+
+Default option functions:
+
+ - `cmd.WithStandardStreams`
+ - `cmd.WithTimeout(time.Duration)`
+ - `cmd.WithoutTimeout`
+ - `cmd.WithWorkingDir(string)`
+
+#### Example
 
 ```go
 c := cmd.NewCommand("echo hello", cmd.WithStandardStreams)
 c.Execute()
 ```
 
-### Set custom options
+#### Set custom options
 
 ```go
-func SetTimeout(c *Command) {
-    c.Timeout = 1 * time.Hour
-}
-
-func SetWorkingDir(c *Command) {
+setWorkingDir := func (c *Command) {
     c.WorkingDir = "/tmp/test"
 }
 
-c := cmd.NewCommand("pwd", SetTimeout, SetWorkingDir)
+c := cmd.NewCommand("pwd", setWorkingDir)
 c.Execute()
 ```
 

command.go

@@ -6,8 +6,6 @@ import (
 	"io"
 	"os"
 	"os/exec"
-	"regexp"
-	"strings"
 	"syscall"
 	"time"
 )
@@ -30,6 +28,7 @@ type Command struct {
 
 // NewCommand creates a new command
 // You can add option with variadic option argument
+// Default timeout is set to 30 minutes
 //
 // Example:
 //      c := cmd.NewCommand("echo hello", function (c *Command) {
@@ -45,7 +44,7 @@ type Command struct {
 func NewCommand(cmd string, options ...func(*Command)) *Command {
 	c := &Command{
 		Command:  cmd,
-		Timeout:  1 * time.Minute,
+		Timeout:  30 * time.Minute,
 		executed: false,
 		Env:      []string{},
 	}
@@ -65,57 +64,42 @@ func NewCommand(cmd string, options ...func(*Command)) *Command {
 //
 // Example:
 //
-//      c := cmd.NewCommand("echo hello", cmd.WithStandardStreams)
-//      c.Execute()
+//     c := cmd.NewCommand("echo hello", cmd.WithStandardStreams)
+//     c.Execute()
 //
 func WithStandardStreams(c *Command) {
 	c.StdoutWriter = os.Stdout
 	c.StderrWriter = os.Stderr
 }
 
-// AddEnv adds an environment variable to the command
-// If a variable gets passed like ${VAR_NAME} the env variable will be read out by the current shell
-func (c *Command) AddEnv(key string, value string) {
-	vars := parseEnvVariableFromShell(value)
-	for _, v := range vars {
-		value = strings.Replace(value, v, os.Getenv(removeEnvVarSyntax(v)), -1)
+// WithTimeout sets the timeout of the command
+//
+// Example:
+//     cmd.NewCommand("sleep 10;", cmd.WithTimeout(500))
+//
+func WithTimeout(t time.Duration) func(c *Command) {
+	return func(c *Command) {
+		c.Timeout = t
 	}
-
-	c.Env = append(c.Env, fmt.Sprintf("%s=%s", key, value))
 }
 
-// Removes the ${...} characters at the beginng and end of the given string
-func removeEnvVarSyntax(v string) string {
-	return v[2:(len(v) - 1)]
+// WithoutTimeout disables the timeout for the command
+func WithoutTimeout(c *Command) {
+	c.Timeout = 0
 }
 
-//Read all environment variables from the given value
-//with the syntax ${VAR_NAME}
-func parseEnvVariableFromShell(val string) []string {
-	reg := regexp.MustCompile(`\$\{.*?\}`)
-	matches := reg.FindAllString(val, -1)
-	return matches
-}
-
-//SetTimeoutMS sets the timeout in milliseconds
-func (c *Command) SetTimeoutMS(ms int) {
-	if ms == 0 {
-		c.Timeout = 1 * time.Minute
-		return
+// WithWorkingDir sets the current working directory
+func WithWorkingDir(dir string) func(c *Command) {
+	return func(c *Command) {
+		c.WorkingDir = dir
 	}
-	c.Timeout = time.Duration(ms) * time.Millisecond
 }
 
-// SetTimeout sets the timeout given a time unit
-// Example: SetTimeout("100s") sets the timeout to 100 seconds
-func (c *Command) SetTimeout(timeout string) error {
-	d, err := time.ParseDuration(timeout)
-	if err != nil {
-		return err
-	}
-
-	c.Timeout = d
-	return nil
+// AddEnv adds an environment variable to the command
+// If a variable gets passed like ${VAR_NAME} the env variable will be read out by the current shell
+func (c *Command) AddEnv(key string, value string) {
+	value = os.ExpandEnv(value)
+	c.Env = append(c.Env, fmt.Sprintf("%s=%s", key, value))
 }
 
 //Stdout returns the output to stdout
@@ -157,14 +141,28 @@ func (c *Command) Execute() error {
 	cmd.Stderr = c.StderrWriter
 	cmd.Dir = c.WorkingDir
 
+	// Create timer only if timeout was set > 0
+	var timeoutChan = make(<-chan time.Time, 1)
+	if c.Timeout != 0 {
+		timeoutChan = time.After(c.Timeout)
+	}
+
 	err := cmd.Start()
 	if err != nil {
 		return err
 	}
 
-	done := make(chan error)
+	done := make(chan error, 1)
+	quit := make(chan bool, 1)
+	defer close(quit)
+
 	go func() {
-		done <- cmd.Wait()
+		select {
+		case <-quit:
+			return
+		case done <- cmd.Wait():
+			return
+		}
 	}()
 
 	select {
@@ -174,14 +172,14 @@ func (c *Command) Execute() error {
 			break
 		}
 		c.exitCode = 0
-	case <-time.After(c.Timeout):
+	case <-timeoutChan:
+		quit <- true
 		if err := cmd.Process.Kill(); err != nil {
 			return fmt.Errorf("Timeout occurred and can not kill process with pid %v", cmd.Process.Pid)
 		}
 		return fmt.Errorf("Command timed out after %v", c.Timeout)
 	}
 
-	//Remove leading and trailing whitespaces
 	c.executed = true
 
 	return nil

command_darwin.go

@@ -2,14 +2,9 @@ package cmd
 
 import (
 	"os/exec"
-	"strings"
 )
 
 func createBaseCommand(c *Command) *exec.Cmd {
 	cmd := exec.Command("/bin/sh", "-c", c.Command)
 	return cmd
 }
-
-func (c *Command) removeLineBreaks(text string) string {
-	return strings.Trim(text, "\n")
-}

command_darwin_test.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"github.com/stretchr/testify/assert"
 	"testing"
+	"time"
 )
 
 func TestCommand_ExecuteStderr(t *testing.T) {
@@ -15,8 +16,7 @@ func TestCommand_ExecuteStderr(t *testing.T) {
 }
 
 func TestCommand_WithTimeout(t *testing.T) {
-	cmd := NewCommand("sleep 0.5;")
-	cmd.SetTimeoutMS(5)
+	cmd := NewCommand("sleep 0.5;", WithTimeout(5*time.Millisecond))
 
 	err := cmd.Execute()
 
@@ -25,8 +25,7 @@ func TestCommand_WithTimeout(t *testing.T) {
 }
 
 func TestCommand_WithValidTimeout(t *testing.T) {
-	cmd := NewCommand("sleep 0.01;")
-	cmd.SetTimeoutMS(500)
+	cmd := NewCommand("sleep 0.01;", WithTimeout(500*time.Millisecond))
 
 	err := cmd.Execute()
 

command_linux.go

@@ -2,14 +2,9 @@ package cmd
 
 import (
 	"os/exec"
-	"strings"
 )
 
 func createBaseCommand(c *Command) *exec.Cmd {
 	cmd := exec.Command("/bin/sh", "-c", c.Command)
 	return cmd
 }
-
-func (c *Command) removeLineBreaks(text string) string {
-	return strings.Trim(text, "\n")
-}

command_linux_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestCommand_ExecuteStderr(t *testing.T) {
@@ -18,8 +19,7 @@ func TestCommand_ExecuteStderr(t *testing.T) {
 }
 
 func TestCommand_WithTimeout(t *testing.T) {
-	cmd := NewCommand("sleep 0.01;")
-	cmd.SetTimeoutMS(1)
+	cmd := NewCommand("sleep 0.1;", WithTimeout(1*time.Millisecond))
 
 	err := cmd.Execute()
 
@@ -30,8 +30,7 @@ func TestCommand_WithTimeout(t *testing.T) {
 }
 
 func TestCommand_WithValidTimeout(t *testing.T) {
-	cmd := NewCommand("sleep 0.01;")
-	cmd.SetTimeoutMS(500)
+	cmd := NewCommand("sleep 0.01;", WithTimeout(500*time.Millisecond))
 
 	err := cmd.Execute()
 
@@ -66,3 +65,19 @@ func TestCommand_WithStandardStreams(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, "hey\n", string(r))
 }
+
+func TestCommand_WithoutTimeout(t *testing.T) {
+	cmd := NewCommand("sleep 0.001; echo hello", WithoutTimeout)
+
+	err := cmd.Execute()
+
+	assert.Nil(t, err)
+	assert.Equal(t, "hello\n", cmd.Stdout())
+}
+
+func TestCommand_WithInvalidDir(t *testing.T) {
+	cmd := NewCommand("echo hello", WithWorkingDir("/invalid"))
+	err := cmd.Execute()
+	assert.NotNil(t, err)
+	assert.Equal(t, "chdir /invalid: no such file or directory", err.Error())
+}

command_test.go

@@ -81,8 +81,8 @@ func TestCommand_AddEnvWithShellVariable(t *testing.T) {
 }
 
 func TestCommand_AddMultipleEnvWithShellVariable(t *testing.T) {
-	const TestEnvKeyPlanet = "COMMANDER_TEST_PLANET"
-	const TestEnvKeyName = "COMMANDER_TEST_NAME"
+	const TestEnvKeyPlanet = "CMD_TEST_PLANET"
+	const TestEnvKeyName = "CMD_TEST_NAME"
 	os.Setenv(TestEnvKeyPlanet, "world")
 	os.Setenv(TestEnvKeyName, "Simon")
 	defer func() {
@@ -108,31 +108,6 @@ func getCommand() string {
 	return command
 }
 
-func TestCommand_SetTimeoutMS_DefaultTimeout(t *testing.T) {
-	c := NewCommand("echo test")
-	c.SetTimeoutMS(0)
-	assert.Equal(t, (1 * time.Minute), c.Timeout)
-}
-
-func TestCommand_SetTimeoutMS(t *testing.T) {
-	c := NewCommand("echo test")
-	c.SetTimeoutMS(100)
-	assert.Equal(t, 100*time.Millisecond, c.Timeout)
-}
-
-func TestCommand_SetTimeout(t *testing.T) {
-	c := NewCommand("echo test")
-	_ = c.SetTimeout("100s")
-	duration, _ := time.ParseDuration("100s")
-	assert.Equal(t, duration, c.Timeout)
-}
-
-func TestCommand_SetInvalidTimeout(t *testing.T) {
-	c := NewCommand("echo test")
-	err := c.SetTimeout("1")
-	assert.Equal(t, "time: missing unit in duration 1", err.Error())
-}
-
 func TestCommand_SetOptions(t *testing.T) {
 	writer := &bytes.Buffer{}
 

command_windows.go

@@ -2,14 +2,9 @@ package cmd
 
 import (
 	"os/exec"
-	"strings"
 )
 
 func createBaseCommand(c *Command) *exec.Cmd {
 	cmd := exec.Command(`C:\windows\system32\cmd.exe`, "/C", c.Command)
 	return cmd
 }
-
-func (c *Command) removeLineBreaks(text string) string {
-	return strings.Trim(text, "\r\n")
-}

command_windows_test.go

@@ -4,6 +4,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestCommand_ExecuteStderr(t *testing.T) {
@@ -16,8 +17,7 @@ func TestCommand_ExecuteStderr(t *testing.T) {
 }
 
 func TestCommand_WithTimeout(t *testing.T) {
-	cmd := NewCommand("timeout 0.005;")
-	cmd.SetTimeoutMS(5)
+	cmd := NewCommand("timeout 0.005;", WithTimeout(5*time.Millisecond))
 
 	err := cmd.Execute()
 
@@ -28,8 +28,7 @@ func TestCommand_WithTimeout(t *testing.T) {
 }
 
 func TestCommand_WithValidTimeout(t *testing.T) {
-	cmd := NewCommand("timeout 0.01;")
-	cmd.SetTimeoutMS(1000)
+	cmd := NewCommand("timeout 0.01;", WithTimeout(1000*time.Millisecond))
 
 	err := cmd.Execute()