Manager: solve failing unittests by implementing some filepath functions

Both Go's standard `path` and `path/filepath` packages are too limiting to
work well for Flamenco. The former assumes Linux/POSIX paths, the latter
only works with platform-native paths. Neither can work with Windows paths
on Linux, or Linux paths on Windows.

internal/manager/job_compilers/job_compilers_test.go

@@ -84,13 +84,10 @@ func TestSimpleBlenderRenderHappy(t *testing.T) {
 	sj := exampleSubmittedJob()
 	aj, err := s.Compile(ctx, sj)
 	if err != nil {
-		t.Logf("job compiler failed: %v", err)
+		t.Fatalf("job compiler failed: %v", err)
-		t.FailNow()
 	}
-	assert.NotNil(t, aj)
 	if aj == nil {
-		// Don't bother with the rest of the test, it'll dereference a nil pointer anyway.
+		t.Fatalf("job compiler returned nil but no error")
-		return
 	}
 
 	// Properties should be copied as-is.
@@ -149,3 +146,68 @@ func TestSimpleBlenderRenderHappy(t *testing.T) {
 	}
 	assert.Equal(t, expectDeps, tVideo.Dependencies)
 }
+
+func TestSimpleBlenderRenderWindowsPaths(t *testing.T) {
+	c := mockedClock(t)
+
+	s, err := Load(c)
+	assert.NoError(t, err)
+
+	// Compiling a job should be really fast.
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
+	defer cancel()
+
+	sj := exampleSubmittedJob()
+
+	// Adjust the job to get paths in Windows notation.
+	sj.Settings.AdditionalProperties["filepath"] = "R:\\sf\\jobs\\scene123.blend"
+	sj.Settings.AdditionalProperties["render_output"] = "R:\\sprites\\farm_output\\promo\\square_ellie\\square_ellie.lighting_light_breakdown2\\######"
+
+	aj, err := s.Compile(ctx, sj)
+	if err != nil {
+		t.Fatalf("job compiler failed: %v", err)
+	}
+	if aj == nil {
+		t.Fatalf("job compiler returned nil but no error")
+	}
+
+	// Properties should be copied as-is, so also with filesystem paths as-is.
+	assert.Equal(t, sj.Name, aj.Name)
+	assert.Equal(t, sj.Type, aj.JobType)
+	assert.Equal(t, sj.Priority, aj.Priority)
+	assert.EqualValues(t, sj.Settings.AdditionalProperties, aj.Settings)
+	assert.EqualValues(t, sj.Metadata.AdditionalProperties, aj.Metadata)
+
+	settings := sj.Settings.AdditionalProperties
+
+	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding
+	assert.Equal(t, 5, len(aj.Tasks))
+	t0 := aj.Tasks[0]
+	expectCliArgs := []interface{}{ // They are strings, but Goja doesn't know that and will produce an []interface{}.
+		// The render output is constructed by the job compiler, and thus transforms to forward slashes.
+		"--render-output", "R:/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2__intermediate-2006-01-02_090405/######",
+		"--render-format", settings["format"].(string),
+		"--render-frame", "1-3",
+	}
+	assert.NotEmpty(t, t0.UUID)
+	assert.Equal(t, "render-1-3", t0.Name)
+	assert.Equal(t, 1, len(t0.Commands))
+	assert.Equal(t, "blender-render", t0.Commands[0].Name)
+	assert.EqualValues(t, AuthoredCommandParameters{
+		"exe":        "{blender}",
+		"blendfile":  "R:\\sf\\jobs\\scene123.blend", // The blendfile parameter is just copied as-is, so keeps using backslash notation.
+		"args":       expectCliArgs,
+		"argsBefore": make([]interface{}, 0),
+	}, t0.Commands[0].Parameters)
+
+	tVideo := aj.Tasks[4] // This should be a video encoding task
+	assert.NotEmpty(t, tVideo.UUID)
+	assert.Equal(t, "create-video", tVideo.Name)
+	assert.Equal(t, 1, len(tVideo.Commands))
+	assert.Equal(t, "create-video", tVideo.Commands[0].Name)
+	assert.EqualValues(t, AuthoredCommandParameters{
+		"input_files": "R:/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2__intermediate-2006-01-02_090405/*.png",
+		"output_file": "R:/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2__intermediate-2006-01-02_090405/scene123-1-10.mp4",
+		"fps":         int64(24),
+	}, tVideo.Commands[0].Parameters)
+}

internal/manager/job_compilers/path.go

@@ -21,8 +21,7 @@ package job_compilers
  * ***** END GPL LICENSE BLOCK ***** */
 
 import (
-	"path/filepath"
+	"git.blender.org/flamenco/pkg/crosspath"
-
 	"github.com/dop251/goja"
 	"github.com/rs/zerolog/log"
 )
@@ -38,14 +37,8 @@ func PathModule(r *goja.Runtime, module *goja.Object) {
 		}
 	}
 
-	mustExport("basename", filepath.Base)
+	mustExport("basename", crosspath.Base)
-	mustExport("dirname", filepath.Dir)
+	mustExport("dirname", crosspath.Dir)
-	mustExport("join", filepath.Join)
+	mustExport("join", crosspath.Join)
-	mustExport("stem", Stem)
+	mustExport("stem", crosspath.Stem)
-}
-
-func Stem(fpath string) string {
-	base := filepath.Base(fpath)
-	ext := filepath.Ext(base)
-	return base[:len(base)-len(ext)]
 }

internal/manager/job_compilers/path_test.go

@@ -1,35 +0,0 @@
-package job_compilers
-
-/* ***** 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 (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestStem(t *testing.T) {
-	assert.Equal(t, "", Stem(""))
-	assert.Equal(t, "stem", Stem("stem.txt"))
-	assert.Equal(t, "stem.a", Stem("stem.a.b"))
-	assert.Equal(t, "file", Stem("/path/to/file.txt"))
-	// assert.Equal(t, "file", Stem("C:\\path\\to\\file.txt"))
-}

pkg/crosspath/crosspath.go

@@ -0,0 +1,80 @@
+// Package crosspath deals with file/directory paths in a cross-platform way.
+//
+// This package tries to understand Windows paths on UNIX and vice versa.
+// Returned paths may be using forward slashes as separators.
+package crosspath
+
+/* ***** 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 (
+	path_module "path" // import under other name so that parameters can be called 'path'
+	"strings"
+)
+
+// Base returns the last element of path. Trailing slashes are removed before
+// extracting the last element. If the path is empty, Base returns ".". If the
+// path consists entirely of slashes, Base returns "/".
+func Base(path string) string {
+	slashed := ToSlash(path)
+	return path_module.Base(slashed)
+}
+
+// Dir returns all but the last element of path, typically the path's directory.
+// If the path is empty, Dir returns ".".
+func Dir(path string) string {
+	if path == "" {
+		return "."
+	}
+
+	slashed := ToSlash(path)
+
+	// Don't use path.Dir(), as that cleans up the path and removes double
+	// slashes. However, Windows UNC paths start with double blackslashes, which
+	// will translate to double slashes and should not be removed.
+	dir, _ := path_module.Split(slashed)
+	switch {
+	case dir == "":
+		return "."
+	case len(dir) > 1:
+		// Remove trailing slash.
+		return dir[:len(dir)-1]
+	default:
+		return dir
+	}
+}
+
+func Join(elem ...string) string {
+	return ToSlash(path_module.Join(elem...))
+}
+
+// Stem returns the filename without extension.
+func Stem(path string) string {
+	base := Base(path)
+	ext := path_module.Ext(base)
+	return base[:len(base)-len(ext)]
+}
+
+// ToSlash replaces all backslashes with forward slashes.
+// Contrary to filepath.ToSlash(), this also happens on Linux; it does not
+// expect `path` to be in platform-native notation.
+func ToSlash(path string) string {
+	return strings.ReplaceAll(path, "\\", "/")
+}

pkg/crosspath/crosspath_test.go

@@ -0,0 +1,107 @@
+package crosspath
+
+/* ***** 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 (
+	"path"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestBase(t *testing.T) {
+	tests := []struct {
+		expect, input string
+	}{
+		{".", ""},
+		{"justafile.txt", "justafile.txt"},
+		{"with spaces.txt", "/Linux path/with spaces.txt"},
+		{"awésom.tar.gz", "C:\\ünicode\\is\\awésom.tar.gz"},
+	}
+	for _, test := range tests {
+		assert.Equal(t, test.expect, Base(test.input))
+	}
+}
+
+func TestDir(t *testing.T) {
+	// Just to show how path.Dir() behaves:
+	assert.Equal(t, ".", path.Dir(""))
+	assert.Equal(t, ".", path.Dir("justafile.txt"))
+
+	tests := []struct {
+		expect, input string
+	}{
+		// Follow path.Dir() when it comes to empty directories:
+		{".", ""},
+		{".", "justafile.txt"},
+
+		{"/", "/"},
+		{"/", "/file-at-root"},
+		{"C:", "C:\\file-at-root"},
+		{"/Linux path", "/Linux path/with spaces.txt"},
+		{"C:/ünicode/is", "C:\\ünicode\\is\\awésom.tar.gz"},
+		{"//SERVER/ünicode/is", "\\\\SERVER\\ünicode\\is\\awésom.tar.gz"},
+	}
+	for _, test := range tests {
+		assert.Equal(t,
+			test.expect, Dir(test.input),
+			"for input %q", test.input)
+	}
+}
+
+func TestJoin(t *testing.T) {
+	// Just to show how path.Join() behaves:
+	assert.Equal(t, "", path.Join())
+	assert.Equal(t, "", path.Join(""))
+	assert.Equal(t, "", path.Join("", ""))
+	assert.Equal(t, "a/b", path.Join("", "", "a", "", "b", ""))
+
+	tests := []struct {
+		expect string
+		input  []string
+	}{
+		// Should behave the same as path.Join():
+		{"", []string{}},
+		{"", []string{""}},
+		{"", []string{"", ""}},
+		{"a/b", []string{"", "", "a", "", "b", ""}},
+
+		{"/file-at-root", []string{"/", "file-at-root"}},
+		{"C:/file-at-root", []string{"C:", "file-at-root"}},
+
+		{"/Linux path/with spaces.txt", []string{"/Linux path", "with spaces.txt"}},
+		{"C:/ünicode/is/awésom.tar.gz", []string{"C:\\ünicode", "is\\awésom.tar.gz"}},
+		{"//SERVER/mount/dir/file.txt", []string{"\\\\SERVER", "mount", "dir", "file.txt"}},
+	}
+	for _, test := range tests {
+		assert.Equal(t,
+			test.expect, Join(test.input...),
+			"for input %q", test.input)
+	}
+}
+
+func TestStem(t *testing.T) {
+	assert.Equal(t, "", Stem(""))
+	assert.Equal(t, "stem", Stem("stem.txt"))
+	assert.Equal(t, "stem.tar", Stem("stem.tar.gz"))
+	assert.Equal(t, "file", Stem("/path/to/file.txt"))
+	assert.Equal(t, "file", Stem("C:\\path\\to\\file.txt"))
+}