catfile: Various small refactorings of tests

Refactor some of the catfile tests to match modern best practices.

3 files changed, 68 insertions, 39 deletions

diff --git a/internal/git/catfile/object_info_reader_test.go b/internal/git/catfile/object_info_reader_test.go
index 96f629eaa..6ae6006fe 100644
--- a/internal/git/catfile/object_info_reader_test.go
+++ b/internal/git/catfile/object_info_reader_test.go
@@ -6,7 +6,9 @@ import (
 	"bufio"
 	"errors"
 	"fmt"
+	"io"
 	"os"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -20,68 +22,86 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 )
 
-func TestParseObjectInfoSuccess(t *testing.T) {
-	testCases := []struct {
-		desc     string
-		input    string
-		output   *ObjectInfo
-		notFound bool
+func TestParseObjectInfo_success(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		desc               string
+		input              string
+		expectedErr        error
+		expectedObjectInfo *ObjectInfo
 	}{
 		{
 			desc:  "existing object",
 			input: "7c9373883988204e5a9f72c4a5119cbcefc83627 commit 222\n",
-			output: &ObjectInfo{
+			expectedObjectInfo: &ObjectInfo{
 				Oid:  "7c9373883988204e5a9f72c4a5119cbcefc83627",
 				Type: "commit",
 				Size: 222,
 			},
 		},
 		{
-			desc:     "non existing object",
-			input:    "bla missing\n",
-			notFound: true,
+			desc:        "non existing object",
+			input:       "bla missing\n",
+			expectedErr: NotFoundError{fmt.Errorf("object not found")},
 		},
-	}
-
-	for _, tc := range testCases {
+	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			reader := bufio.NewReader(strings.NewReader(tc.input))
-			output, err := ParseObjectInfo(gittest.DefaultObjectHash, reader)
-			if tc.notFound {
-				require.True(t, IsNotFound(err), "expect NotFoundError")
-				return
-			}
 
-			require.NoError(t, err)
-			require.Equal(t, tc.output, output)
+			objectInfo, err := ParseObjectInfo(gittest.DefaultObjectHash, reader)
+			require.Equal(t, tc.expectedErr, err)
+			require.Equal(t, tc.expectedObjectInfo, objectInfo)
 		})
 	}
 }
 
-func TestParseObjectInfoErrors(t *testing.T) {
-	testCases := []struct {
-		desc  string
-		input string
-	}{
-		{desc: "missing newline", input: "7c9373883988204e5a9f72c4a5119cbcefc83627 commit 222"},
-		{desc: "too few words", input: "7c9373883988204e5a9f72c4a5119cbcefc83627 commit\n"},
-		{desc: "too many words", input: "7c9373883988204e5a9f72c4a5119cbcefc83627 commit 222 bla\n"},
-		{desc: "parse object size", input: "7c9373883988204e5a9f72c4a5119cbcefc83627 commit bla\n"},
-	}
+func TestParseObjectInfo_errors(t *testing.T) {
+	t.Parallel()
 
-	for _, tc := range testCases {
+	for _, tc := range []struct {
+		desc        string
+		input       string
+		expectedErr error
+	}{
+		{
+			desc:        "missing newline",
+			input:       "7c9373883988204e5a9f72c4a5119cbcefc83627 commit 222",
+			expectedErr: fmt.Errorf("read info line: %w", io.EOF),
+		},
+		{
+			desc:        "too few words",
+			input:       "7c9373883988204e5a9f72c4a5119cbcefc83627 commit\n",
+			expectedErr: fmt.Errorf("invalid info line: %q", "7c9373883988204e5a9f72c4a5119cbcefc83627 commit"),
+		},
+		{
+			desc:        "too many words",
+			input:       "7c9373883988204e5a9f72c4a5119cbcefc83627 commit 222 bla\n",
+			expectedErr: fmt.Errorf("invalid info line: %q", "7c9373883988204e5a9f72c4a5119cbcefc83627 commit 222 bla"),
+		},
+		{
+			desc:  "parse object size",
+			input: "7c9373883988204e5a9f72c4a5119cbcefc83627 commit bla\n",
+			expectedErr: fmt.Errorf("parse object size: %w", &strconv.NumError{
+				Func: "ParseInt",
+				Num:  "bla",
+				Err:  strconv.ErrSyntax,
+			}),
+		},
+	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			reader := bufio.NewReader(strings.NewReader(tc.input))
-			_, err := ParseObjectInfo(gittest.DefaultObjectHash, reader)
 
-			require.Error(t, err)
+			_, err := ParseObjectInfo(gittest.DefaultObjectHash, reader)
+			require.Equal(t, tc.expectedErr, err)
 		})
 	}
 }
 
 func TestObjectInfoReader(t *testing.T) {
-	ctx := testhelper.Context(t)
+	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 	repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
 
@@ -181,8 +201,9 @@ func TestObjectInfoReader(t *testing.T) {
 }
 
 func TestObjectInfoReader_queue(t *testing.T) {
-	ctx := testhelper.Context(t)
+	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 	repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
 
diff --git a/internal/git/catfile/parser_test.go b/internal/git/catfile/parser_test.go
index 9e8746252..e1fb34007 100644
--- a/internal/git/catfile/parser_test.go
+++ b/internal/git/catfile/parser_test.go
@@ -16,6 +16,8 @@ import (
 )
 
 func TestParser_ParseCommit(t *testing.T) {
+	t.Parallel()
+
 	info := &ObjectInfo{
 		Oid:  "a984dfa4dee018c6d5f5f57ffec0d0e22763df16",
 		Type: "commit",
@@ -28,7 +30,7 @@ func TestParser_ParseCommit(t *testing.T) {
 	// Once a repository contains a pathological object it can be hard to get
 	// rid of it. Because of this I think it's nicer to ignore such objects
 	// than to throw hard errors.
-	testCases := []struct {
+	for _, tc := range []struct {
 		desc string
 		in   string
 		out  *gitalypb.GitCommit
@@ -119,9 +121,7 @@ fF3T79iV8paT4/OfX8Ygg=
 				},
 			},
 		},
-	}
-
-	for _, tc := range testCases {
+	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			info.Size = int64(len(tc.in))
 			out, err := NewParser().ParseCommit(newStaticObject(tc.in, "commit", info.Oid))
@@ -132,6 +132,8 @@ fF3T79iV8paT4/OfX8Ygg=
 }
 
 func TestParseCommitAuthor(t *testing.T) {
+	t.Parallel()
+
 	for _, tc := range []struct {
 		desc     string
 		author   string
@@ -177,6 +179,8 @@ func TestParseCommitAuthor(t *testing.T) {
 }
 
 func TestParser_ParseTag(t *testing.T) {
+	t.Parallel()
+
 	for _, tc := range []struct {
 		desc           string
 		oid            git.ObjectID
diff --git a/internal/git/catfile/request_queue_test.go b/internal/git/catfile/request_queue_test.go
index 236e3fbdd..7818a67af 100644
--- a/internal/git/catfile/request_queue_test.go
+++ b/internal/git/catfile/request_queue_test.go
@@ -17,6 +17,8 @@ import (
 )
 
 func TestRequestQueue_ReadObject(t *testing.T) {
+	t.Parallel()
+
 	ctx := testhelper.Context(t)
 
 	t.Run("ReadInfo on ReadObject queue", func(t *testing.T) {
@@ -215,6 +217,8 @@ func TestRequestQueue_ReadObject(t *testing.T) {
 }
 
 func TestRequestQueue_RequestRevision(t *testing.T) {
+	t.Parallel()
+
 	ctx := testhelper.Context(t)
 
 	requireRevision := func(t *testing.T, queue *requestQueue, rev git.Revision) {