git: Refactor revision-validation tests

Refactor revision-validation tests to be more conformant with our
current coding style. Furthermore, extend these tests to also verify
that `ValidateRevisionAllowEmpty()` behaves as expected.

1 files changed, 47 insertions, 16 deletions

diff --git a/internal/git/revision_test.go b/internal/git/revision_test.go
index e928e69d7..0769d8ff9 100644
--- a/internal/git/revision_test.go
+++ b/internal/git/revision_test.go
@@ -1,31 +1,62 @@
 package git
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
 func TestValidateRevision(t *testing.T) {
-	testCases := []struct {
-		rev string
-		ok  bool
+	for _, tc := range []struct {
+		desc        string
+		revision    string
+		expectedErr error
 	}{
-		{rev: "foo/bar", ok: true},
-		{rev: "-foo/bar", ok: false},
-		{rev: "foo bar", ok: false},
-		{rev: "foo\x00bar", ok: false},
-		{rev: "foo/bar:baz", ok: false},
-	}
+		{
+			desc:        "empty revision",
+			revision:    "",
+			expectedErr: fmt.Errorf("empty revision"),
+		},
+		{
+			desc:     "valid revision",
+			revision: "foo/bar",
+		},
+		{
+			desc:        "leading dash",
+			revision:    "-foo/bar",
+			expectedErr: fmt.Errorf("revision can't start with '-'"),
+		},
+		{
+			desc:     "intermediate dash",
+			revision: "foo-bar",
+		},
+		{
+			desc:        "space",
+			revision:    "foo bar",
+			expectedErr: fmt.Errorf("revision can't contain whitespace"),
+		},
+		{
+			desc:        "NUL-byte",
+			revision:    "foo\x00bar",
+			expectedErr: fmt.Errorf("revision can't contain NUL"),
+		},
+		{
+			desc:        "colon",
+			revision:    "foo/bar:baz",
+			expectedErr: fmt.Errorf("revision can't contain ':'"),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			require.Equal(t, tc.expectedErr, ValidateRevision([]byte(tc.revision)))
 
-	for _, tc := range testCases {
-		t.Run(tc.rev, func(t *testing.T) {
-			err := ValidateRevision([]byte(tc.rev))
-			if tc.ok {
-				require.NoError(t, err)
-			} else {
-				require.Error(t, err)
+			// `ValidateRevision()` and `ValidateRevisionAllowEmpty()` behave the same,
+			// except in the case where the revision is empty. In that case, the latter
+			// does not return an error.
+			if tc.revision == "" {
+				tc.expectedErr = nil
 			}
+			require.Equal(t, tc.expectedErr, ValidateRevisionAllowEmpty([]byte(tc.revision)))
 		})
 	}
 }