Worker: add CLI runner interface and move test/mock code around a bit

cmd/flamenco-worker-poc/main.go

@@ -83,9 +83,10 @@ func main() {
 	shutdownComplete = make(chan struct{})
 	workerCtx, workerCtxCancel := context.WithCancel(context.Background())
 
+	cliRunner := worker.NewCLIRunner()
 	listener = worker.NewListener(client)
 	timeService := clock.New()
-	cmdRunner := worker.NewCommandExecutor(listener, timeService)
+	cmdRunner := worker.NewCommandExecutor(cliRunner, listener, timeService)
 	taskRunner := worker.NewTaskExecutor(cmdRunner, listener)
 	w = worker.NewWorker(client, taskRunner)
 

internal/worker/cli_runner.go

@@ -0,0 +1,38 @@
+package worker
+
+/* ***** BEGIN GPL LICENSE BLOCK *****
+ *
+ * Original Code Copyright (C) 2022 Blender Foundation.
+ *
+ * This file is part of Flamenco.
+ *
+ * Flamenco is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later
+ * version.
+ *
+ * Flamenco is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+ * A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * Flamenco.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * ***** END GPL LICENSE BLOCK ***** */
+
+import (
+	"context"
+	"os/exec"
+)
+
+// CLIRunner is a wrapper around exec.CommandContext() to allow mocking.
+type CLIRunner struct {
+}
+
+func NewCLIRunner() *CLIRunner {
+	return &CLIRunner{}
+}
+
+func (cli *CLIRunner) CommandContext(ctx context.Context, name string, arg ...string) *exec.Cmd {
+	return nil
+}

internal/worker/command_exe_test.go

@@ -23,10 +23,34 @@ package worker
 import (
 	"testing"
 
+	"github.com/benbjohnson/clock"
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"gitlab.com/blender/flamenco-ng-poc/internal/worker/mocks"
 	"gitlab.com/blender/flamenco-ng-poc/pkg/api"
 )
 
+type CommandExecutorMocks struct {
+	cli      *mocks.MockCommandLineRunner
+	listener *mocks.MockCommandListener
+	clock    *clock.Mock
+}
+
+func testCommandExecutor(t *testing.T, mockCtrl *gomock.Controller) (*CommandExecutor, *CommandExecutorMocks) {
+	cli := mocks.NewMockCommandLineRunner(mockCtrl)
+	listener := mocks.NewMockCommandListener(mockCtrl)
+	clock := mockedClock(t)
+
+	ce := NewCommandExecutor(cli, listener, clock)
+	mocks := CommandExecutorMocks{
+		cli:      cli,
+		listener: listener,
+		clock:    clock,
+	}
+
+	return ce, &mocks
+}
+
 func TestCmdSettingAsStrings(t *testing.T) {
 	cmd := api.Command{
 		Name: "test",
@@ -39,7 +63,7 @@ func TestCmdSettingAsStrings(t *testing.T) {
 	}
 
 	{
-		slice, ok := cmdSettingAsStrings(cmd, "strings")
+		slice, ok := cmdParameterAsStrings(cmd, "strings")
 		if ok {
 			assert.Equal(t, []string{"a", "b"}, slice)
 		} else {
@@ -47,15 +71,15 @@ func TestCmdSettingAsStrings(t *testing.T) {
 		}
 	}
 	{
-		_, ok := cmdSettingAsStrings(cmd, "ints")
+		_, ok := cmdParameterAsStrings(cmd, "ints")
 		assert.False(t, ok, "only []string or []interface{} are expected to work")
 	}
 	{
-		_, ok := cmdSettingAsStrings(cmd, "floats")
+		_, ok := cmdParameterAsStrings(cmd, "floats")
 		assert.False(t, ok, "only []string or []interface{} are expected to work")
 	}
 	{
-		slice, ok := cmdSettingAsStrings(cmd, "mixed")
+		slice, ok := cmdParameterAsStrings(cmd, "mixed")
 		if ok {
 			assert.Equal(t, []string{"a", "47", "0.327"}, slice)
 		} else {

internal/worker/command_misc_test.go

@@ -25,38 +25,15 @@ import (
 	"testing"
 	"time"
 
-	"github.com/benbjohnson/clock"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
-	"gitlab.com/blender/flamenco-ng-poc/internal/worker/mocks"
 	"gitlab.com/blender/flamenco-ng-poc/pkg/api"
 )
 
-type MockCommandExecutor struct {
-	ce       *CommandExecutor
-	cli      *mocks.MockCommandLineRunner
-	listener *mocks.MockCommandListener
-	clock    *clock.Mock
-}
-
-func testCommandExecutor(t *testing.T, mockCtrl *gomock.Controller) *MockCommandExecutor {
-	cli := mocks.NewMockCommandLineRunner(mockCtrl)
-	listener := mocks.NewMockCommandListener(mockCtrl)
-	clock := mockedClock(t)
-	ce := NewCommandExecutor(cli, listener, clock)
-
-	return &MockCommandExecutor{
-		ce:       ce,
-		cli:      cli,
-		listener: listener,
-		clock:    clock,
-	}
-}
-
 func TestCommandEcho(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
-	tce := testCommandExecutor(t, mockCtrl)
+	ce, mocks := testCommandExecutor(t, mockCtrl)
 
 	ctx := context.Background()
 	message := "понављај за мном"
@@ -66,16 +43,16 @@ func TestCommandEcho(t *testing.T) {
 		Parameters: map[string]interface{}{"message": message},
 	}
 
-	tce.listener.EXPECT().LogProduced(gomock.Any(), taskID, "echo: \"понављај за мном\"")
+	mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID, "echo: \"понављај за мном\"")
 
-	err := tce.ce.Run(ctx, taskID, cmd)
+	err := ce.Run(ctx, taskID, cmd)
 	assert.NoError(t, err)
 }
 
 func TestCommandSleep(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
-	tce := testCommandExecutor(t, mockCtrl)
+	ce, mocks := testCommandExecutor(t, mockCtrl)
 
 	ctx := context.Background()
 	taskID := "90e9d656-e201-4ef0-b6b0-c80684fafa27"
@@ -84,9 +61,9 @@ func TestCommandSleep(t *testing.T) {
 		Parameters: map[string]interface{}{"duration_in_seconds": 47},
 	}
 
-	tce.listener.EXPECT().LogProduced(gomock.Any(), taskID, "slept 47s")
+	mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID, "slept 47s")
 
-	timeBefore := tce.clock.Now()
+	timeBefore := mocks.clock.Now()
 
 	// Run the test in a goroutine, as we also need to actually increase the
 	// mocked clock at the same time; without that, the command will sleep
@@ -94,7 +71,7 @@ func TestCommandSleep(t *testing.T) {
 	runDone := make(chan struct{})
 	var err error
 	go func() {
-		err = tce.ce.Run(ctx, taskID, cmd)
+		err = ce.Run(ctx, taskID, cmd)
 		close(runDone)
 	}()
 
@@ -105,12 +82,12 @@ loop:
 		case <-runDone:
 			break loop
 		default:
-			tce.clock.Add(timeStepSize)
+			mocks.clock.Add(timeStepSize)
 		}
 	}
 
 	assert.NoError(t, err)
-	timeAfter := tce.clock.Now()
+	timeAfter := mocks.clock.Now()
 	// Within the step size is precise enough. We're testing our implementation, not the precision of `time.After()`.
 	assert.WithinDuration(t, timeBefore.Add(47*time.Second), timeAfter, timeStepSize)
 }