fix bindetector and tests

Makefile

@@ -30,11 +30,9 @@ lint:
 	docker build --tag cavoritelint -f _ci/lint/Dockerfile .
 	docker run cavoritelint
 
-test: gazelle
+tests: gazelle
 	bazel test //...
 
-tests: gazelle test
-
 minio:
 	docker run -p 9000:9000 -p 9001:9001 quay.io/minio/minio server /data --console-address ":9001"
 

bindetector/BUILD.bazel

@@ -9,12 +9,16 @@ go_library(
     ],
     importpath = "github.com/discentem/cavorite/bindetector",
     visibility = ["//:__subpackages__"],
-    deps = ["@com_github_google_logger//:logger"],
+    deps = ["//exec"],
 )
 
 go_test(
     name = "bindetector_test",
     srcs = ["bindetector_unix_test.go"],
     embed = [":bindetector"],
-    deps = ["@com_github_stretchr_testify//require"],
+    deps = [
+        "//exec",
+        "@com_github_hashicorp_go_multierror//:go-multierror",
+        "@com_github_stretchr_testify//assert",
+    ],
 )

bindetector/bindetector.go

@@ -1,23 +1,17 @@
 package bindetector
 
 import (
-	"regexp"
-
-	"github.com/google/logger"
+	shell "github.com/discentem/cavorite/exec"
 )
 
-func IsBinary(filepath string) bool {
+func IsBinary(filepath string) (bool, error) {
 	// for now, we must shell out to /usr/bin/file or respective windows exe called file.exe provided by git
 	// someday, binary determination could be implemented in-house without the need for external tools but until then
 	// shelling out is a necessary evil
-	return isBinary(execFile(filepath))
-}
-
-func isBinary(bytes []byte) bool {
-	matched, err := regexp.Match(`binary`, bytes)
+	e := shell.NewRealExecutor()
+	binary, err := fileIsABinary(e, filepath)
 	if err != nil {
-		logger.Errorf("isBinary regex matching error: %v", err)
+		return false, err
 	}
-
-	return matched
+	return binary, nil
 }

bindetector/bindetector_unix.go

@@ -5,28 +5,35 @@ package bindetector
 
 import (
 	"bytes"
-	"os/exec"
+	"errors"
 
-	"github.com/google/logger"
+	shell "github.com/discentem/cavorite/exec"
 )
 
-func execFile(filepath string) []byte {
-	var stdoutBuf, stderrBuf bytes.Buffer
-	cmd := exec.Command(
+func fileIsABinary(executor shell.Executor, filepath string) (bool, error) {
+	buf := new(bytes.Buffer)
+
+	executor.Command(
 		"/usr/bin/file",
 		"--mime-encoding",
 		"-b",
 		filepath,
 	)
 
-	cmd.Stdout = &stdoutBuf
-	cmd.Stderr = &stderrBuf
+	if err := executor.Stream(&nopWriteCloser{buf}); err != nil {
+		return false, err
+	}
 
-	err := cmd.Run()
-	if err != nil {
-		logger.Errorf("isBinary cmd run error: %v", err)
+	output := bytes.TrimSpace(buf.Bytes())
+	if len(output) == 0 {
+		return false, errors.New("fileIsABinary: no output from file command")
 	}
 
-	return stdoutBuf.Bytes()
+	return bytes.Contains(output, []byte("binary")), nil
+}
 
+type nopWriteCloser struct {
+	*bytes.Buffer
 }
+
+func (n *nopWriteCloser) Close() error { return nil }

bindetector/bindetector_unix_test.go

@@ -1,13 +1,80 @@
 package bindetector
 
 import (
+	"errors"
+	"io"
 	"testing"
 
-	"github.com/stretchr/testify/require"
+	shell "github.com/discentem/cavorite/exec"
+	"github.com/hashicorp/go-multierror"
+	multierr "github.com/hashicorp/go-multierror"
+	"github.com/stretchr/testify/assert"
 )
 
-func TestIsBinary(t *testing.T) {
-	require.True(t, isBinary([]byte(`binary`)))
-	require.False(t, isBinary([]byte(`blah`)))
+type fakeExecutor struct {
+	output string
+}
+
+func (f fakeExecutor) Command(name string, arg ...string) {}
+
+func (f fakeExecutor) Stream(posters ...io.WriteCloser) error {
+	var result *multierror.Error
+	for _, p := range posters {
+		_, err := p.Write([]byte(f.output))
+		result = multierr.Append(result, err)
+	}
+	return result.ErrorOrNil()
+}
 
+type brokenExecutor struct{}
+
+func (b brokenExecutor) Command(name string, arg ...string) {}
+
+func (b brokenExecutor) Stream(posters ...io.WriteCloser) error {
+	return multierr.Append(nil, errors.New("broken executor"))
+}
+func TestFileIsABinary(t *testing.T) {
+	tests := []struct {
+		name        string
+		executor    shell.Executor
+		filepath    string
+		expected    bool
+		expectedErr bool
+	}{
+		{
+			name:        "file is a binary",
+			executor:    fakeExecutor{output: "application/x-executable; charset=binary\n"},
+			filepath:    "whatever",
+			expected:    true,
+			expectedErr: false,
+		},
+		{
+			name:        "file is not a binary",
+			executor:    fakeExecutor{output: "text/plain; charset=us-ascii\n"},
+			filepath:    "whatever",
+			expected:    false,
+			expectedErr: false,
+		},
+		{
+			name:        "no output from binary check command",
+			executor:    fakeExecutor{output: ""},
+			filepath:    "whatever",
+			expected:    false, // no output means we can't determine if the file is a binary
+			expectedErr: true,  // we expect an error because the output is empty
+		},
+		{
+			name:        "broken executor",
+			executor:    brokenExecutor{},
+			filepath:    "whatever",
+			expected:    false, // we can't determine if the file is a binary because the executor
+			expectedErr: true,  // we expect an error because the executor is broken
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual, err := fileIsABinary(test.executor, test.filepath)
+			assert.Equal(t, test.expected, actual, "expected %v, got %v", test.expected, actual)
+			assert.Equal(t, test.expectedErr, err != nil, "expected error: %v", test.expectedErr)
+		})
+	}
 }

bindetector/bindetector_windows.go

@@ -3,25 +3,9 @@
 
 package bindetector
 
-func execFile(filepath string) []byte {
-	var stdoutBuf bytes.Buffer
-	// cmd := exec.Command(
-	// 	"/usr/bin/file",
-	// 	"--mime-encoding",
-	// 	"-b",
-	// 	filepath,
-	// )
+import "errors"
 
-	// cmd.Stdout = &stdoutBuf
-	// cmd.Stderr = &stderrBuf
-
-	// err := cmd.Run()
-	// if err != nil {
-	// 	logger.Errorf("isBinary cmd run error: %v", err)
-	// }
-
-	// return stdoutBuf.Bytes()
-
-	return stdoutBuf.Bytes()
+func fileIsABinary(filepath string) (*bool, error) {
+	return nil, errors.New("fileIsABinary not implemented on Windows")
 
 }

exec/exec.go

@@ -18,6 +18,23 @@ var (
 
 type RealExecutor struct {
 	*exec.Cmd
+	posters []io.WriteCloser
+}
+
+type RealExecutorOption func(e *RealExecutor)
+
+func WithPosters(posters ...io.WriteCloser) RealExecutorOption {
+	return func(e *RealExecutor) {
+		e.posters = posters
+	}
+}
+
+func NewRealExecutor(opts ...RealExecutorOption) *RealExecutor {
+	re := &RealExecutor{}
+	for _, opt := range opts {
+		opt(re)
+	}
+	return re
 }
 
 func (e *RealExecutor) Command(bin string, args ...string) {
@@ -45,7 +62,12 @@ func (e *RealExecutor) Stream(posters ...io.WriteCloser) error {
 	}
 
 	if posters == nil {
-		posters = []io.WriteCloser{os.Stdout}
+		if e.posters == nil {
+			posters = []io.WriteCloser{os.Stdout}
+		} else {
+			posters = e.posters
+		}
+
 	}
 
 	for _, pipe := range inputPipes {

exec/exec_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type NopBufferCloser struct {
@@ -115,3 +116,30 @@ func TestWriteOutput(t *testing.T) {
 		assert.Equal(t, test.expected, out.String())
 	}
 }
+
+type mockWriteCloser struct {
+	io.Writer
+}
+
+func (m *mockWriteCloser) Close() error {
+	return nil
+}
+
+func TestWithPosters_AssignsExactInstances(t *testing.T) {
+	var buf1, buf2 bytes.Buffer
+	w1 := &mockWriteCloser{Writer: &buf1}
+	w2 := &mockWriteCloser{Writer: &buf2}
+
+	exec := RealExecutor{}
+	WithPosters(w1, w2)(&exec)
+
+	require.Len(t, exec.posters, 2)
+
+	// Confirm the WriteClosers themselves are the same (not just equal)
+	require.Same(t, w1, exec.posters[0])
+	require.Same(t, w2, exec.posters[1])
+
+	// Also confirm that the internal buffers are the same instances
+	require.Same(t, &buf1, exec.posters[0].(*mockWriteCloser).Writer)
+	require.Same(t, &buf2, exec.posters[1].(*mockWriteCloser).Writer)
+}