Add test for Blender command and better command parameter parsing

internal/worker/command_blender.go

@@ -25,13 +25,12 @@ package worker
 import (
 	"context"
 	"fmt"
-	"os/exec"
 
 	"github.com/rs/zerolog"
 	"gitlab.com/blender/flamenco-ng-poc/pkg/api"
 )
 
-type BlenderSettings struct {
+type BlenderParameters struct {
 	exe        string   // Expansion of `{blender}`: executable path + its CLI parameters defined by the Manager.
 	argsBefore []string // Additional CLI arguments defined by the job compiler script, to go before the blend file name.
 	blendfile  string   // Path of the file to open.
@@ -40,40 +39,46 @@ type BlenderSettings struct {
 
 // cmdBlender executes the "blender-render" command.
 func (ce *CommandExecutor) cmdBlenderRender(ctx context.Context, logger zerolog.Logger, taskID string, cmd api.Command) error {
-	settings := BlenderSettings{
+	var (
-		exe:       cmd.Parameters["exe"].(string),
+		parameters BlenderParameters
-		blendfile: cmd.Parameters["blendfile"].(string),
+		ok         bool
+	)
+
+	if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" {
+		logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter")
+		return fmt.Errorf("missing 'exe' parameter: %+v", cmd.Parameters)
 	}
-	var ok bool
+	if parameters.argsBefore, ok = cmdParameterAsStrings(cmd, "argsBefore"); !ok {
-	if settings.exe == "" {
+		logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' parameter")
-		logger.Warn().Interface("command", cmd).Msg("missing 'exe' setting")
+		return fmt.Errorf("invalid 'argsBefore' parameter: %+v", cmd.Parameters)
-		return fmt.Errorf("missing 'exe' setting: %+v", cmd)
 	}
-	if settings.argsBefore, ok = cmdSettingAsStrings(cmd, "argsBefore"); !ok {
+	if parameters.blendfile, ok = cmdParameter[string](cmd, "blendfile"); !ok || parameters.blendfile == "" {
-		logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' setting")
+		logger.Warn().Interface("command", cmd).Msg("missing 'blendfile' parameter")
-		return fmt.Errorf("invalid 'argsBefore' setting: %+v", cmd)
+		return fmt.Errorf("missing 'blendfile' parameter: %+v", cmd.Parameters)
 	}
-	if settings.blendfile == "" {
+	if parameters.args, ok = cmdParameterAsStrings(cmd, "args"); !ok {
-		logger.Warn().Interface("command", cmd).Msg("missing 'blendfile' setting")
+		logger.Warn().Interface("command", cmd).Msg("invalid 'args' parameter")
-		return fmt.Errorf("missing 'blendfile' setting: %+v", cmd)
+		return fmt.Errorf("invalid 'args' parameter: %+v", cmd.Parameters)
-	}
-	if settings.args, ok = cmdSettingAsStrings(cmd, "args"); !ok {
-		logger.Warn().Interface("command", cmd).Msg("invalid 'args' setting")
-		return fmt.Errorf("invalid 'args' setting: %+v", cmd)
 	}
 
 	cliArgs := make([]string, 0)
-	cliArgs = append(cliArgs, settings.argsBefore...)
+	cliArgs = append(cliArgs, parameters.argsBefore...)
-	cliArgs = append(cliArgs, settings.blendfile)
+	cliArgs = append(cliArgs, parameters.blendfile)
-	cliArgs = append(cliArgs, settings.args...)
+	cliArgs = append(cliArgs, parameters.args...)
-	execCmd := exec.CommandContext(ctx, settings.exe, cliArgs...)
+	execCmd := ce.cli.CommandContext(ctx, parameters.exe, cliArgs...)
+	if execCmd == nil {
+		logger.Error().
+			Str("cmdName", cmd.Name).
+			Msg("unable to create command executor")
+		return ErrNoExecCmd
+	}
 	logger.Info().
 		Str("cmdName", cmd.Name).
 		Str("execCmd", execCmd.String()).
 		Msg("going to execute Blender")
 
-	// if err := ce.listener.LogProduced(ctx, taskID, fmt.Sprintf("echo: %q", messageStr)); err != nil {
+	if err := ce.listener.LogProduced(ctx, taskID, fmt.Sprintf("going to run: %s", cliArgs)); err != nil {
-	// 	return err
+		return err
-	// }
+	}
 	return nil
 }

internal/worker/command_blender_test.go

@@ -0,0 +1,56 @@
+package worker
+
+import (
+	"context"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/rs/zerolog"
+	"github.com/stretchr/testify/assert"
+	"gitlab.com/blender/flamenco-ng-poc/pkg/api"
+)
+
+/* ***** 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 ***** */
+
+func TestCmdBlenderSimpleCliArgs(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	ce, mocks := testCommandExecutor(t, mockCtrl)
+
+	ctx := context.Background()
+	taskID := "1d54c6fe-1242-4c8f-bd63-5a09e358d7b6"
+	cmd := api.Command{
+		Name: "blender",
+		Parameters: map[string]interface{}{
+			"exe":        "/path/to/blender",
+			"argsBefore": []string{"--background"},
+			"blendfile":  "file.blend",
+			"args":       []string{"--render-output", "/frames"},
+		},
+	}
+
+	cliArgs := []string{"--background", "file.blend", "--render-output", "/frames"}
+	mocks.cli.EXPECT().CommandContext(ctx, "/path/to/blender", cliArgs).Return(nil)
+
+	err := ce.cmdBlenderRender(ctx, zerolog.Nop(), taskID, cmd)
+	assert.Equal(t, ErrNoExecCmd, err, "nil *exec.Cmd should result in ErrNoExecCmd")
+}

internal/worker/command_exe.go

@@ -22,6 +22,7 @@ package worker
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os/exec"
 	"time"
@@ -37,6 +38,7 @@ import (
 // CommandListener sends the result of commands (log, output files) to the Manager.
 type CommandListener interface {
 	// LogProduced sends any logging to whatever service for storing logging.
+	// logLines are concatenated.
 	LogProduced(ctx context.Context, taskID string, logLines ...string) error
 	// OutputProduced tells the Manager there has been some output (most commonly a rendered frame or video).
 	OutputProduced(ctx context.Context, taskID string, outputLocation string) error
@@ -66,6 +68,12 @@ type CommandLineRunner interface {
 	CommandContext(ctx context.Context, name string, arg ...string) *exec.Cmd
 }
 
+// ErrNoExecCmd means CommandLineRunner.CommandContext() returned nil.
+// This shouldn't happen in production, but can happen in unit tests when the
+// test just wants to check the CLI arguments that are supposed to be executed,
+// without actually executing anything.
+var ErrNoExecCmd = errors.New("no exec.Cmd could be created")
+
 func NewCommandExecutor(cli CommandLineRunner, listener CommandListener, timeService TimeService) *CommandExecutor {
 	ce := &CommandExecutor{
 		cli:         cli,
@@ -97,18 +105,19 @@ func (ce *CommandExecutor) Run(ctx context.Context, taskID string, cmd api.Comma
 	return runner(ctx, logger, taskID, cmd)
 }
 
-// cmdSettingAsStrings converts an array setting ([]interface{}) to a []string slice.
+// cmdParameterAsStrings converts an array parameter ([]interface{}) to a []string slice.
-func cmdSettingAsStrings(cmd api.Command, key string) ([]string, bool) {
+// A missing parameter is ok and returned as empty slice.
-	setting, found := cmd.Parameters[key]
+func cmdParameterAsStrings(cmd api.Command, key string) ([]string, bool) {
+	parameter, found := cmd.Parameters[key]
 	if !found {
-		return []string{}, false
+		return []string{}, true
 	}
 
-	if asStrSlice, ok := setting.([]string); ok {
+	if asStrSlice, ok := parameter.([]string); ok {
 		return asStrSlice, true
 	}
 
-	interfSlice, ok := setting.([]interface{})
+	interfSlice, ok := parameter.([]interface{})
 	if !ok {
 		return []string{}, false
 	}
@@ -126,3 +135,15 @@ func cmdSettingAsStrings(cmd api.Command, key string) ([]string, bool) {
 	}
 	return strSlice, true
 }
+
+// cmdParameter retrieves a single parameter of a certain type.
+func cmdParameter[T any](cmd api.Command, key string) (T, bool) {
+	setting, found := cmd.Parameters[key]
+	if !found {
+		var zeroValue T
+		return zeroValue, false
+	}
+
+	value, ok := setting.(T)
+	return value, ok
+}