Merge branch 'smh-user-commit-files-autocrlf' into 'master'

Improve test coverage of line ending normalization

See merge request gitlab-org/gitaly!3026

2 files changed, 81 insertions, 25 deletions

diff --git a/internal/git/localrepo_objects_test.go b/internal/git/localrepo_objects_test.go
index 516e2f30a..2dae8951a 100644
--- a/internal/git/localrepo_objects_test.go
+++ b/internal/git/localrepo_objects_test.go
@@ -28,22 +28,15 @@ func TestLocalRepository_WriteBlob(t *testing.T) {
 	pbRepo, repoPath, clean := testhelper.InitBareRepo(t)
 	defer clean()
 
-	// write attributes file so we can verify WriteBlob runs the files through filters as
-	// appropriate
-	require.NoError(t, ioutil.WriteFile(filepath.Join(repoPath, "info", "attributes"), []byte(`
-crlf binary
-lf   text
-	`), os.ModePerm))
-
 	repo := NewRepository(pbRepo, config.Config)
 
 	for _, tc := range []struct {
-		desc    string
-		path    string
-		input   io.Reader
-		sha     string
-		error   error
-		content string
+		desc       string
+		attributes string
+		input      io.Reader
+		sha        string
+		error      error
+		content    string
 	}{
 		{
 			desc:  "error reading",
@@ -53,38 +46,41 @@ lf   text
 		{
 			desc:    "successful empty blob",
 			input:   strings.NewReader(""),
-			sha:     "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
 			content: "",
 		},
 		{
 			desc:    "successful blob",
 			input:   strings.NewReader("some content"),
-			sha:     "f0eec86f614944a81f87d879ebdc9a79aea0d7ea",
 			content: "some content",
 		},
 		{
-			desc:    "line endings not normalized",
-			path:    "crlf",
-			input:   strings.NewReader("\r\n"),
-			sha:     "d3f5a12faa99758192ecc4ed3fc22c9249232e86",
-			content: "\r\n",
+			desc:    "LF line endings left unmodified",
+			input:   strings.NewReader("\n"),
+			content: "\n",
 		},
 		{
-			desc:    "line endings normalized",
-			path:    "lf",
+			desc:    "CRLF converted to LF due to global git config",
 			input:   strings.NewReader("\r\n"),
-			sha:     "8b137891791fe96927ad78e64b0aad7bded08bdc",
 			content: "\n",
 		},
+		{
+			desc:       "line endings preserved in binary files",
+			input:      strings.NewReader("\r\n"),
+			attributes: "file-path binary",
+			content:    "\r\n",
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			sha, err := repo.WriteBlob(ctx, tc.path, tc.input)
+			require.NoError(t,
+				ioutil.WriteFile(filepath.Join(repoPath, "info", "attributes"), []byte(tc.attributes), os.ModePerm),
+			)
+
+			sha, err := repo.WriteBlob(ctx, "file-path", tc.input)
 			require.Equal(t, tc.error, err)
 			if tc.error != nil {
 				return
 			}
 
-			assert.Equal(t, tc.sha, sha)
 			content, err := repo.ReadObject(ctx, sha)
 			require.NoError(t, err)
 			assert.Equal(t, tc.content, string(content))
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index 5fe58b105..c6f75f612 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -298,6 +298,23 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
+			desc: "create file normalizes line endings",
+			steps: []step{
+				{
+					actions: []*gitalypb.UserCommitFilesRequest{
+						createFileHeaderRequest("file-1"),
+						actionContentRequest("content-1\r\n"),
+						actionContentRequest(" content-2\r\n"),
+					},
+					repoCreated:   true,
+					branchCreated: true,
+					treeEntries: []testhelper.TreeEntry{
+						{Mode: DefaultMode, Path: "file-1", Content: "content-1\n content-2\n"},
+					},
+				},
+			},
+		},
+		{
 			desc: "create duplicate file",
 			steps: []step{
 				{
@@ -346,6 +363,24 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
+			desc: "update file normalizes line endings",
+			steps: []step{
+				{
+					actions: []*gitalypb.UserCommitFilesRequest{
+						createFileHeaderRequest("file-1"),
+						actionContentRequest("content-1"),
+						updateFileHeaderRequest("file-1"),
+						actionContentRequest("content-2\r\n"),
+					},
+					repoCreated:   true,
+					branchCreated: true,
+					treeEntries: []testhelper.TreeEntry{
+						{Mode: DefaultMode, Path: "file-1", Content: "content-2\n"},
+					},
+				},
+			},
+		},
+		{
 			desc: "update base64 content",
 			steps: []step{
 				{
@@ -599,6 +634,31 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
+			desc: "move file normalizes line endings",
+			steps: []step{
+				{
+					actions: []*gitalypb.UserCommitFilesRequest{
+						createFileHeaderRequest("original-file"),
+						actionContentRequest("original-content"),
+					},
+					repoCreated:   true,
+					branchCreated: true,
+					treeEntries: []testhelper.TreeEntry{
+						{Mode: DefaultMode, Path: "original-file", Content: "original-content"},
+					},
+				},
+				{
+					actions: []*gitalypb.UserCommitFilesRequest{
+						moveFileHeaderRequest("original-file", "moved-file", false),
+						actionContentRequest("new-content\r\n"),
+					},
+					treeEntries: []testhelper.TreeEntry{
+						{Mode: DefaultMode, Path: "moved-file", Content: "new-content\n"},
+					},
+				},
+			},
+		},
+		{
 			desc: "mark non-existing file executable",
 			steps: []step{
 				{