[v2,2/4] t6038: remove problematic test

Commit Message

Koji Nakamaru via GitGitGadget Aug. 3, 2020, 6:41 p.m. UTC

From: Elijah Newren <[e-mail]>

t6038.11, 'cherry-pick patch from after text=auto' was a test of
undefined behavior.  To make matters worse, while there are a couple
possible correct answers, this test was coded to only check for an
obviously incorrect answer.  And the final cherry on top is that the
test is marked test_expect_failure, meaning it can't provide much value,
other than possibly confusing future folks who come along and try to
work on attributes and look at existing tests.  Because of all these
problems, just remove the test.

But for any future code spelunkers, here's my understanding of the two
possible correct answers:

This test was set up so that on a branch with no .gitattributes file,
you cherry-picked a patch from a branch that had a .gitattributes file
(containing '* text=auto').  Further, the two branches had a file which
differed only in line endings.  In this situation, correct behavior is
not well defined: should the .gitattributes file affect the merge or
not?

If the .gitattributes file on the other branch should not affect the
merge, then we would have a content conflict with all three stages
different (the merge base didn't match either side).

If the .gitattributes file from the other branch should affect the
merge, then we would expect the line endings to be normalized to LF for
the version to be recorded in the repository.  This would mean that when
doing a three-way content merge on the file that differed in line
endings, that the three-way content merge would see that the versions on
both sides matched and so the cherry-pick has no conflicts and can
succeed.  The line endings in the file as recorded in the repository
will change from CRLF to LF.  The version checked out in the working
copy will depend on the platform (since there's no eol attribute defined
for the file).

Also, as a final side note, this test expected an error message that was
built assuming cherry-pick was the old scripted version, because
cherry-pick no longer uses the error message that was encoded in this
test.  So it was wrong for yet another reason.

Given that the handling of .gitattributes is not well defined and this
test was obviously broken and could do nothing but confuse future
readers, just remove it.

Signed-off-by: Elijah Newren <[e-mail]>
---
 t/t6038-merge-text-auto.sh | 14 --------------
 1 file changed, 14 deletions(-)

Patch

diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index 27cea15533..9337745793 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -188,20 +188,6 @@  test_expect_failure 'checkout -m addition of text=auto' '
 	git diff --no-index --ignore-cr-at-eol expected file
 '
 
-test_expect_failure 'cherry-pick patch from after text=auto was added' '
-	append_cr <<-\EOF >expected &&
-	first line
-	same line
-	EOF
-
-	git config merge.renormalize true &&
-	git rm -fr . &&
-	git reset --hard b &&
-	test_must_fail git cherry-pick a >err 2>&1 &&
-	grep "[Nn]othing added" err &&
-	compare_files expected file
-'
-
 test_expect_success 'Test delete/normalize conflict' '
 	git checkout -f side &&
 	git rm -fr . &&