Add test cases on toolchain command runner

Author: mengdaming

src/engine/tcr.go

@@ -524,7 +524,7 @@ func (tcr *TCREngine) RunTCRCycle() {
 
 // AbortCommand triggers interruption of an ongoing TCR cycle operation
 func (tcr *TCREngine) AbortCommand() {
-	tcr.toolchain.AbortExecution()
+	_ = tcr.toolchain.AbortExecution()
 }
 
 func (tcr *TCREngine) createTCREvent(testResult toolchain.TestCommandResult) (event events.TCREvent) {

src/toolchain/command_runner.go

@@ -27,12 +27,15 @@ import (
 	"github.com/murex/tcr/report"
 	"io"
 	"os/exec"
+	"sync"
 )
 
 type (
 	// CommandRunner is in charge of managing the lifecycle of a command
 	CommandRunner struct {
 		command *exec.Cmd
+		// commandMutex is here to enforce that commands run in sequence
+		commandMutex sync.Mutex
 	}
 
 	// CommandStatus is the result status of a Command execution
@@ -64,16 +67,17 @@ const (
 )
 
 // commandRunner singleton instance
-var commandRunner = getCommandRunner()
+var commandRunner = &CommandRunner{
+	command: nil,
+}
 
 func getCommandRunner() *CommandRunner {
-	return &CommandRunner{
-		command: nil,
-	}
+	return commandRunner
 }
 
 // Run launches the execution of the provided command
 func (r *CommandRunner) Run(cmd *Command) (result CommandResult) {
+	commandRunner.commandMutex.Lock()
 	result = CommandResult{Status: CommandStatusUnknown, Output: ""}
 	report.PostText(cmd.asCommandLine())
 
@@ -98,6 +102,7 @@ func (r *CommandRunner) Run(cmd *Command) (result CommandResult) {
 		// but we need to ensure first that TCR engine can handle it correctly.
 		result.Status = CommandStatusFail
 		r.command = nil
+		commandRunner.commandMutex.Unlock()
 		return result
 	}
 
@@ -110,6 +115,7 @@ func (r *CommandRunner) Run(cmd *Command) (result CommandResult) {
 	}
 
 	r.command = nil
+	commandRunner.commandMutex.Unlock()
 	return result
 }
 
@@ -123,13 +129,14 @@ func (*CommandRunner) reportCommandTrace(readCloser io.ReadCloser) {
 }
 
 // AbortRunningCommand triggers aborting of any command that is currently running
-func (r *CommandRunner) AbortRunningCommand() {
+func (r *CommandRunner) AbortRunningCommand() bool {
 	if r.command == nil || r.command.Process == nil {
 		report.PostWarning("There is no command running at this time")
-		return
+		return false
 	}
 	report.PostWarning("Aborting command: \"", r.command.String(), "\"")
 	_ = r.command.Process.Kill()
 	// Calling Kill() may be a bit too brutal (may leave children process alive)
 	//_ = r.command.Process.Signal(os.Kill)
+	return true
 }

src/toolchain/command_runner_test.go

@@ -24,13 +24,14 @@ package toolchain
 
 import (
 	"fmt"
+	"github.com/murex/tcr/utils"
 	"github.com/stretchr/testify/assert"
 	"testing"
+	"time"
 )
 
 func Test_command_result_outcome(t *testing.T) {
-
-	testFlags := []struct {
+	testCases := []struct {
 		status         CommandStatus
 		expectedPassed bool
 		expectedFailed bool
@@ -39,11 +40,81 @@ func Test_command_result_outcome(t *testing.T) {
 		{"fail", false, true},
 		{"unknown", false, false},
 	}
-	for _, tt := range testFlags {
+	for _, tt := range testCases {
 		t.Run(fmt.Sprint(tt.status, "_status"), func(t *testing.T) {
 			result := CommandResult{Status: tt.status}
 			assert.Equal(t, tt.expectedPassed, result.Passed())
 			assert.Equal(t, tt.expectedFailed, result.Failed())
 		})
 	}
 }
+
+func Test_run_command(t *testing.T) {
+	testCases := []struct {
+		desc           string
+		command        Command
+		expectedStatus CommandStatus
+	}{
+		{
+			"unknown command",
+			Command{Path: "unknown-command"},
+			CommandStatusFail,
+		},
+		{
+			"passing command",
+			Command{Path: "true"},
+			CommandStatusPass,
+		},
+		{
+			"failing command",
+			Command{Path: "false"},
+			CommandStatusFail,
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.desc, func(t *testing.T) {
+			result := getCommandRunner().Run(&tt.command)
+			assert.Equal(t, tt.expectedStatus, result.Status)
+		})
+	}
+}
+
+func Test_abort_command(t *testing.T) {
+	// this test fails on Windows CI for an unexplained reason.
+	// This seems to be related to command.Process never being set when running a command,
+	// but no explanation why this happens on Windows CI while this works as expected
+	// on local Windows as well as on Unix CIs.
+	utils.SkipOnWindowsCI(t)
+
+	testCases := []struct {
+		desc     string
+		command  *Command
+		expected bool
+	}{
+		{
+			"with no running command",
+			nil,
+			false,
+		},
+		{
+			"with running command",
+			&Command{Path: "sleep", Arguments: []string{"5"}},
+			true,
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.desc, func(t *testing.T) {
+			if tt.command != nil {
+				go getCommandRunner().Run(tt.command)
+				// wait for the new command process to start
+				for getCommandRunner().command == nil {
+					time.Sleep(10 * time.Millisecond)
+				}
+			}
+			result := getCommandRunner().AbortRunningCommand()
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

src/toolchain/toolchain.go

@@ -70,7 +70,7 @@ type (
 		checkTestCommand() error
 		runsOnPlatform(osName OsName, archName ArchName) bool
 		CheckCommandAccess(cmdPath string) (string, error)
-		AbortExecution()
+		AbortExecution() bool
 	}
 )
 
@@ -164,8 +164,8 @@ func (tchn Toolchain) RunTests() TestCommandResult {
 }
 
 // AbortExecution asks the toolchain to abort any command currently executing
-func (Toolchain) AbortExecution() {
-	commandRunner.AbortRunningCommand()
+func (Toolchain) AbortExecution() bool {
+	return commandRunner.AbortRunningCommand()
 }
 
 // BuildCommandPath returns the build command path for this toolchain

src/utils/test_helper.go

@@ -27,6 +27,8 @@ package utils
 import (
 	"bytes"
 	"github.com/stretchr/testify/assert"
+	"os"
+	"runtime"
 	"testing"
 )
 
@@ -50,3 +52,10 @@ func AssertSimpleTrace(t *testing.T, expected []string, operation func()) {
 	}
 	assert.Equal(t, expectedWithWrapping, output.String())
 }
+
+// SkipOnWindowsCI allows to prevent running a test when on Windows CI when called at the beginning of the test
+func SkipOnWindowsCI(t *testing.T) {
+	if os.Getenv("GITHUB_ACTIONS") == "true" && runtime.GOOS == "windows" {
+		t.Skip("test skipped on windows CI")
+	}
+}