Improve Session and Background tests

background.go

@@ -24,7 +24,7 @@ type ErrJobExists struct {
 
 // Error implements error interface.
 func (a ErrJobExists) Error() string {
-	return fmt.Sprintf("Job '%s' already running with PID %d", a.Name, a.Pid)
+	return fmt.Sprintf(`job "%s" already running with PID %d`, a.Name, a.Pid)
 }
 
 // IsJobExists returns true if error is of type ErrJobExists.
@@ -60,13 +60,9 @@ func (wf *Workflow) Kill(jobName string) error {
 		return err
 	}
 	p := wf.pidFile(jobName)
-	if err = syscall.Kill(pid, syscall.SIGTERM); err != nil {
-		// Delete stale PID file
-		os.Remove(p)
-		return err
-	}
+	err = syscall.Kill(pid, syscall.SIGTERM)
 	os.Remove(p)
-	return nil
+	return err
 }
 
 // IsRunning returns true if a job with name jobName is currently running.

background_test.go

@@ -4,57 +4,99 @@
 package aw
 
 import (
+	"fmt"
+	"io/ioutil"
 	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/deanishe/awgo/util"
 )
 
-// TestRunInBackground ensures background jobs work.
-func TestRunInBackground(t *testing.T) {
+// Background jobs.
+func TestWorkflow_RunInBackground(t *testing.T) {
 	t.Parallel()
 
-	wf := New()
-
-	cmd := exec.Command("sleep", "5")
-	if wf.IsRunning("sleep") {
-		t.Fatalf("Job 'sleep' is already running")
-	}
-	if err := wf.RunInBackground("sleep", cmd); err != nil {
-		t.Fatalf("Error starting job 'sleep': %s", err)
-	}
-	if !wf.IsRunning("sleep") {
-		t.Fatalf("Job 'sleep' is not running")
-	}
-	p := wf.pidFile("sleep")
-	if !util.PathExists(p) {
-		t.Fatalf("No PID file for 'sleep'")
-	}
-	// Duplicate jobs fail
-	cmd = exec.Command("sleep", "5")
-	err := wf.RunInBackground("sleep", cmd)
-	if err == nil {
-		t.Fatal("Starting duplicate 'sleep' job didn't error")
-	}
-	if _, ok := err.(ErrJobExists); !ok {
-		t.Fatal("RunInBackground didn't return ErrAlreadyRunning")
-	}
-	if !IsJobExists(err) {
-		t.Errorf("IsAlreadyRunning didn't identify ErrAlreadyRunning")
-	}
-	// Job killed OK
-	if err := wf.Kill("sleep"); err != nil {
-		t.Fatalf("Error killing 'sleep' job: %s", err)
-	}
-	// Job kill fails
-	if err := wf.Kill("sleep"); err == nil {
-		t.Fatal("No error killing 'sleep' job")
-	}
-
-	if wf.IsRunning("sleep") {
-		t.Fatal("'sleep' job still running")
-	}
-	if util.PathExists(p) {
-		t.Fatal("'sleep' PID file not deleted")
-	}
+	withTestWf(func(wf *Workflow) {
+
+		jobName := "sleep"
+
+		cmd := exec.Command("sleep", "5")
+		// Sanity check
+		if wf.IsRunning(jobName) {
+			t.Fatalf("Job %q is already running", jobName)
+		}
+
+		// Start job
+		if err := wf.RunInBackground(jobName, cmd); err != nil {
+			t.Fatalf("Error starting job %q: %s", jobName, err)
+		}
+
+		// Job running?
+		if !wf.IsRunning(jobName) {
+			t.Fatalf("Job %q is not running", jobName)
+		}
+		pid, err := wf.getPid(jobName)
+		if err != nil {
+			t.Fatalf("get PID for job: %v", err)
+		}
+		p := wf.pidFile(jobName)
+		if !util.PathExists(p) {
+			t.Fatalf("No PID file for %q", jobName)
+		}
+
+		// Duplicate job fails?
+		cmd = exec.Command("sleep", "5")
+		err = wf.RunInBackground("sleep", cmd)
+		if err == nil {
+			t.Fatal("Starting duplicate 'sleep' job didn't error")
+		}
+		if _, ok := err.(ErrJobExists); !ok {
+			t.Fatal("RunInBackground didn't return ErrAlreadyRunning")
+		}
+		if !IsJobExists(err) {
+			t.Errorf("IsAlreadyRunning didn't identify ErrAlreadyRunning")
+		}
+		if strings.Index(err.Error(), fmt.Sprintf("%d", pid)) == -1 {
+			t.Errorf(`PID not found in error`)
+		}
+
+		// Job killed OK?
+		if err := wf.Kill("sleep"); err != nil {
+			t.Fatalf("Error killing job %q: %s", jobName, err)
+		}
+		// Killing dead job fails?
+		if err := wf.Kill("sleep"); err == nil {
+			t.Fatalf("No error killing dead job %q", jobName)
+		}
+
+		// Job has exited and tidied up?
+		if wf.IsRunning("sleep") {
+			t.Fatalf("%q job still running", jobName)
+		}
+		if util.PathExists(p) {
+			t.Fatalf("PID file for %q not deleted", jobName)
+		}
+
+		// Invalid PID returns error?
+		if err := ioutil.WriteFile(p, []byte("bad PID"), 0600); err != nil {
+			t.Fatalf("failed to write PID file %q: %v", p, err)
+		}
+
+		if err := wf.Kill(jobName); err == nil {
+			t.Fatal("invalid PID did not cause error")
+		}
+	})
+}
+
+// invalid command fails
+func TestWorkflow_RunInBackground_badJob(t *testing.T) {
+	t.Parallel()
+
+	withTestWf(func(wf *Workflow) {
+		cmd := exec.Command("/does/not/exist")
+		if err := wf.RunInBackground("badJob", cmd); err == nil {
+			t.Fatal(`run "/does/not/exist" succeeded`)
+		}
+	})
 }