t0028 fix test + more tests

Commit 1/2: t0028: fix test for UTF-16-LE-BOM Commit 2/2: t0028: add more tests Please refer to individual commit messages for more information.

Alexandr Miloslavskiy (2):
t0028: fix test for UTF-16-LE-BOM
t0028: add more tests

 t/t0028-working-tree-encoding.sh | 41 +++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

base-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-347%2FSyntevoAlex%2F%230189_t0028_fixes-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-347/SyntevoAlex/#0189_t0028_fixes-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/347

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

According to its name, the test its designed for UTF-16-LE-BOM. However, possibly due to copy&paste oversight, it was using UTF-32 file.

While the test succeeds (probably interprets extra \x00\x00 as embedded zero), I myself had an unrelated problem which caused the test to fail.
When analyzing the failure I was quite puzzled by the fact that the
test is obviously bugged. And it seems that I'm not alone:
https://public-inbox.org/git/CAH8yC8kSakS807d4jc_BtcUJOrcVT4No37AXSz=jePxhw-o9Dg@mail.gmail.com/T/#u

This fix changes the test to follow its original intention.

---
 t/t0028-working-tree-encoding.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 1090e650ed..5493cf3ca9 100755
@@ -40,7 +40,7 @@ test_expect_success 'setup test files' '
 	printf "$text" | write_utf16 >test.utf16.raw &&
 	printf "$text" | write_utf32 >test.utf32.raw &&
 	printf "\377\376"                         >test.utf16lebom.raw &&
-	printf "$text" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&
+	printf "$text" | iconv -f UTF-8 -t UTF-16LE >>test.utf16lebom.raw &&
 
 	# Line ending tests
 	printf "one\ntwo\nthree\n" >lf.utf8.raw &&

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

After I discovered that UTF-16-LE-BOM test was bugged and still succeeded, I decided that better tests are required. Possibly the best option here is to compare git results against hardcoded ground truth.

The new tests also cover more interesting chars where (ANSI != UTF-8).

---
 t/t0028-working-tree-encoding.sh | 39 ++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

index 5493cf3ca9..d0dd5dd0ea 100755
@@ -280,4 +280,43 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '
 	git reset
 '
 
+# $1: checkout encoding
+# $2: test string
+# $3: binary test string in checkout encoding
+test_commit_utf8_checkout_other () {
+	encoding="$1"
+	orig_string="$2"
+	expect_bytes="$3"
+	
+	test_expect_success "Commit utf-8, checkout ${encoding}" '
+		test_when_finished "git checkout HEAD -- .gitattributes" &&
+		
+		test_ext="commit_utf8_checkout_${encoding}" &&
+		test_file="test.${test_ext}" &&
+		
+		# Commit as utf-8
+		echo "*.${test_ext} text working-tree-encoding=utf-8" >.gitattributes &&
+		printf "${orig_string}" >"${test_file}" &&
+		git add "${test_file}" &&
+		git commit -m "Test data" &&
+
+		# Checkout in tested encoding
+		rm "${test_file}" &&
+		echo "*.${test_ext} text working-tree-encoding=${encoding}" >.gitattributes &&
+		git checkout HEAD -- "${test_file}" &&
+		
+		# Test
+		printf "${expect_bytes}" > "${test_file}.raw" &&
+		test_cmp_bin "${test_file}.raw" "${test_file}"
+	'
+}
+
+test_commit_utf8_checkout_other "UTF-8"        "Test Тест" "\124\145\163\164\040\320\242\320\265\321\201\321\202"
+test_commit_utf8_checkout_other "UTF-16LE"     "Test Тест" "\124\000\145\000\163\000\164\000\040\000\042\004\065\004\101\004\102\004"
+test_commit_utf8_checkout_other "UTF-16BE"     "Test Тест" "\000\124\000\145\000\163\000\164\000\040\004\042\004\065\004\101\004\102"
+test_commit_utf8_checkout_other "UTF-16LE-BOM" "Test Тест" "\377\376\124\000\145\000\163\000\164\000\040\000\042\004\065\004\101\004\102\004"
+test_commit_utf8_checkout_other "UTF-16BE-BOM" "Test Тест" "\376\377\000\124\000\145\000\163\000\164\000\040\004\042\004\065\004\101\004\102"
+test_commit_utf8_checkout_other "UTF-32LE"     "Test Тест" "\124\000\000\000\145\000\000\000\163\000\000\000\164\000\000\000\040\000\000\000\042\004\000\000\065\004\000\000\101\004\000\000\102\004\000\000"
+test_commit_utf8_checkout_other "UTF-32BE"     "Test Тест" "\000\000\000\124\000\000\000\145\000\000\000\163\000\000\000\164\000\000\000\040\000\000\004\042\000\000\004\065\000\000\004\101\000\000\004\102"
+
 test_done

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

According to its name, the test is designed for UTF-16-LE-BOM. However, possibly due to copy&paste oversight, it was using UTF-32 file.

While the test succeeds (probably interprets extra \x00\x00 as embedded zero), I myself had an unrelated problem which caused the test to fail. When analyzing the failure I was quite puzzled by the fact that the test is obviously bugged. And it seems that I'm not alone:
https://public-inbox.org/git/CAH8yC8kSakS807d4jc_BtcUJOrcVT4No37AXSz=jePxhw-o9Dg@mail.gmail.com/T/#u

This fix changes the test to follow its original intention.

---
 t/t0028-working-tree-encoding.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 1090e650ed..5493cf3ca9 100755
@@ -40,7 +40,7 @@ test_expect_success 'setup test files' '
 	printf "$text" | write_utf16 >test.utf16.raw &&
 	printf "$text" | write_utf32 >test.utf32.raw &&
 	printf "\377\376"                         >test.utf16lebom.raw &&
-	printf "$text" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&
+	printf "$text" | iconv -f UTF-8 -t UTF-16LE >>test.utf16lebom.raw &&
 
 	# Line ending tests
 	printf "one\ntwo\nthree\n" >lf.utf8.raw &&

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

After I discovered that UTF-16-LE-BOM test was bugged and still succeeded, I decided that better tests are required. Possibly the best option here is to compare git results against hardcoded ground truth.

The new tests also cover more interesting chars where (ANSI != UTF-8).

---
 t/t0028-working-tree-encoding.sh | 39 ++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

index 5493cf3ca9..d0dd5dd0ea 100755
@@ -280,4 +280,43 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '
 	git reset
 '
 
+# $1: checkout encoding
+# $2: test string
+# $3: binary test string in checkout encoding
+test_commit_utf8_checkout_other () {
+	encoding="$1"
+	orig_string="$2"
+	expect_bytes="$3"
+	
+	test_expect_success "Commit utf-8, checkout ${encoding}" '
+		test_when_finished "git checkout HEAD -- .gitattributes" &&
+		
+		test_ext="commit_utf8_checkout_${encoding}" &&
+		test_file="test.${test_ext}" &&
+		
+		# Commit as utf-8
+		echo "*.${test_ext} text working-tree-encoding=utf-8" >.gitattributes &&
+		printf "${orig_string}" >"${test_file}" &&
+		git add "${test_file}" &&
+		git commit -m "Test data" &&
+
+		# Checkout in tested encoding
+		rm "${test_file}" &&
+		echo "*.${test_ext} text working-tree-encoding=${encoding}" >.gitattributes &&
+		git checkout HEAD -- "${test_file}" &&
+		
+		# Test
+		printf "${expect_bytes}" > "${test_file}.raw" &&
+		test_cmp_bin "${test_file}.raw" "${test_file}"
+	'
+}
+
+test_commit_utf8_checkout_other "UTF-8"        "Test Тест" "\124\145\163\164\040\320\242\320\265\321\201\321\202"
+test_commit_utf8_checkout_other "UTF-16LE"     "Test Тест" "\124\000\145\000\163\000\164\000\040\000\042\004\065\004\101\004\102\004"
+test_commit_utf8_checkout_other "UTF-16BE"     "Test Тест" "\000\124\000\145\000\163\000\164\000\040\004\042\004\065\004\101\004\102"
+test_commit_utf8_checkout_other "UTF-16LE-BOM" "Test Тест" "\377\376\124\000\145\000\163\000\164\000\040\000\042\004\065\004\101\004\102\004"
+test_commit_utf8_checkout_other "UTF-16BE-BOM" "Test Тест" "\376\377\000\124\000\145\000\163\000\164\000\040\004\042\004\065\004\101\004\102"
+test_commit_utf8_checkout_other "UTF-32LE"     "Test Тест" "\124\000\000\000\145\000\000\000\163\000\000\000\164\000\000\000\040\000\000\000\042\004\000\000\065\004\000\000\101\004\000\000\102\004\000\000"
+test_commit_utf8_checkout_other "UTF-32BE"     "Test Тест" "\000\000\000\124\000\000\000\145\000\000\000\163\000\000\000\164\000\000\000\040\000\000\004\042\000\000\004\065\000\000\004\101\000\000\004\102"
+
 test_done

gitgitgadget

Commit 1/2: t0028: fix test for UTF-16-LE-BOM Commit 2/2: t0028: add more tests Please refer to individual commit messages for more information.

Alexandr Miloslavskiy (2):
t0028: fix test for UTF-16-LE-BOM
t0028: add more tests

 t/t0028-working-tree-encoding.sh | 41 +++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

base-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-347%2FSyntevoAlex%2F%230189_t0028_fixes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-347/SyntevoAlex/#0189_t0028_fixes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/347

Range-diff vs v1:

1: fe850a8cd5 ! 1: d717a60932 t0028: fix test for UTF-16-LE-BOM
@@ -2,12 +2,12 @@

t0028: fix test for UTF-16-LE-BOM

- According to its name, the test its designed for UTF-16-LE-BOM.
+ According to its name, the test is designed for UTF-16-LE-BOM.
However, possibly due to copy&paste oversight, it was using UTF-32 file.

While the test succeeds (probably interprets extra \x00\x00 as embedded zero), I myself had an unrelated problem which caused the test to fail.
- When analyzing the failure I was quite puzzled by the fact that the
+ When analyzing the failure I was quite puzzled by the fact that the
test is obviously bugged. And it seems that I'm not alone:
https://public-inbox.org/git/CAH8yC8kSakS807d4jc_BtcUJOrcVT4No37AXSz=jePxhw-o9Dg@mail.gmail.com/T/#u

2: d5f6f00a51 = 2: 40e54cf5ce t0028: add more tests

gitgitgadget

We can probably drop the "probably" ?

Out of curiosity:
What made the test fail, and does it pass noe ?

Thanks for debugging and cleaning up my mess.
The patch is correct, and I have one or two small improvements in the wording.

How about the following:

According to its name, the test is designed for UTF-16-LE-BOM. However, possibly due to copy&paste oversight, it was using UTF-32.

While the test succeeds (extra \000\000 are interpreted as NUL), I myself had an unrelated problem which caused the test to fail. When analyzing the failure I was quite puzzled by the fact that the test is obviously bugged. And it seems that I'm not alone:
https://public-inbox.org/git/CAH8yC8kSakS807d4jc_BtcUJOrcVT4No37AXSz=jePxhw-o9Dg@mail.gmail.com/T/#u

Fix the test to follow its original intention.

Thanks for the tests, some nit-picks inline.

My interpretation is that the \000\000 must be handled correctly on all platforms, and that seems to be the case.

Would this make more sense:

After I discovered that UTF-16-LE-BOM test was bugged, I decided that better tests are required

Am 23.09.19 um 12:04 schrieb Alexandr Miloslavskiy via GitGitGadget:

What are we testing here? Is there some back-and-forth conversion going on, and are we testing that the conversion happens at all, or that the correct conversion/encoding is picked, or that the conversion that is finally chosen is correct? Why does it help to test more interesting chars (and would you not also regard codepoints outside the BMP the most interesting because they require surrogate codepoints in UTF-16)?

-- Hannes

It failed to a bug in Windows Debug build, which caused iconv() to fail in most cases, see:
https://public-inbox.org/git/[email protected]/

Sure thing, I will use the wording you suggested.

OK

OK

This is pretty much my first experience with test framework (and maybe third with shell scripts...). I will change as suggested.

According to my understanding (I'm not the author of test package), it is designed to test that various encodings are properly supported by git in the working tree.

The new tests are designed to avoid any back-and-forth, which actually happened for the previous UTF-16-LE-BOM test, which in turn hidden that the test was bugged.

Otherwise, the test verifies that if you requested some encoding, you get exactly that, and it covers various potential problems at once.

It helps to cover more potential problems. One could agree that converting latin characters is mostly about padding/dropping zero chars, but this approach could never work for the chars I used. As for "outside the BMP", I'm simply not experienced with that. If you are, you're welcome to further improve the tests I added.

From: Alexandr Miloslavskiy <[email protected]>

After I discovered that UTF-16-LE-BOM test was bugged, I decided that better tests are required. Possibly the best option here is to compare git results against hardcoded ground truth.

The new tests also cover more interesting chars where (ANSI != UTF-8).

---
 t/t0028-working-tree-encoding.sh | 39 ++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

index 5493cf3ca9..c4d98a1aae 100755
@@ -280,4 +280,43 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '
 	git reset
 '
 
+# $1: checkout encoding
+# $2: test string
+# $3: binary test string in checkout encoding
+test_commit_utf8_checkout_other () {
+	encoding="$1"
+	orig_string="$2"
+	expect_bytes="$3"
+	
+	test_expect_success "Commit UTF-8, checkout $encoding" '
+		test_when_finished "git checkout HEAD -- .gitattributes" &&
+		
+		test_ext="commit_utf8_checkout_$encoding" &&
+		test_file="test.$test_ext" &&
+		
+		# Commit as UTF-8
+		echo "*.$test_ext text working-tree-encoding=UTF-8" >.gitattributes &&
+		printf "$orig_string" >$test_file &&
+		git add $test_file &&
+		git commit -m "Test data" &&
+
+		# Checkout in tested encoding
+		rm $test_file &&
+		echo "*.$test_ext text working-tree-encoding=$encoding" >.gitattributes &&
+		git checkout HEAD -- $test_file &&
+		
+		# Test
+		printf $expect_bytes >$test_file.raw &&
+		test_cmp_bin $test_file.raw $test_file
+	'
+}
+
+test_commit_utf8_checkout_other "UTF-8"        "Test Тест" "\124\145\163\164\040\320\242\320\265\321\201\321\202"
+test_commit_utf8_checkout_other "UTF-16LE"     "Test Тест" "\124\000\145\000\163\000\164\000\040\000\042\004\065\004\101\004\102\004"
+test_commit_utf8_checkout_other "UTF-16BE"     "Test Тест" "\000\124\000\145\000\163\000\164\000\040\004\042\004\065\004\101\004\102"
+test_commit_utf8_checkout_other "UTF-16LE-BOM" "Test Тест" "\377\376\124\000\145\000\163\000\164\000\040\000\042\004\065\004\101\004\102\004"
+test_commit_utf8_checkout_other "UTF-16BE-BOM" "Test Тест" "\376\377\000\124\000\145\000\163\000\164\000\040\004\042\004\065\004\101\004\102"
+test_commit_utf8_checkout_other "UTF-32LE"     "Test Тест" "\124\000\000\000\145\000\000\000\163\000\000\000\164\000\000\000\040\000\000\000\042\004\000\000\065\004\000\000\101\004\000\000\102\004\000\000"
+test_commit_utf8_checkout_other "UTF-32BE"     "Test Тест" "\000\000\000\124\000\000\000\145\000\000\000\163\000\000\000\164\000\000\000\040\000\000\004\042\000\000\004\065\000\000\004\101\000\000\004\102"
+
 test_done

gitgitgadget

Commit 1/2: t0028: fix test for UTF-16-LE-BOM Commit 2/2: t0028: add more tests Please refer to individual commit messages for more information.

Alexandr Miloslavskiy (2):
t0028: fix test for UTF-16-LE-BOM
t0028: add more tests

 t/t0028-working-tree-encoding.sh | 41 +++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

base-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-347%2FSyntevoAlex%2F%230189_t0028_fixes-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-347/SyntevoAlex/#0189_t0028_fixes-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/347

Range-diff vs v2:

1: d717a60932 ! 1: 438ac961a5 t0028: fix test for UTF-16-LE-BOM
@@ -3,15 +3,15 @@
t0028: fix test for UTF-16-LE-BOM

According to its name, the test is designed for UTF-16-LE-BOM.
- However, possibly due to copy&paste oversight, it was using UTF-32 file.
+ However, possibly due to copy&paste oversight, it was using UTF-32.

- While the test succeeds (probably interprets extra \x00\x00 as embedded
- zero), I myself had an unrelated problem which caused the test to fail.
+ While the test succeeds (extra \000\000 are interpreted as NUL),
+ I myself had an unrelated problem which caused the test to fail.
When analyzing the failure I was quite puzzled by the fact that the test is obviously bugged. And it seems that I'm not alone:
https://public-inbox.org/git/CAH8yC8kSakS807d4jc_BtcUJOrcVT4No37AXSz=jePxhw-o9Dg@mail.gmail.com/T/#u

- This fix changes the test to follow its original intention.
+ Fix the test to follow its original intention.

2: 40e54cf5ce ! 2: e4410274e6 t0028: add more tests

     @@ -2,9 +2,9 @@
      
          t0028: add more tests
      
     -    After I discovered that UTF-16-LE-BOM test was bugged and still
     -    succeeded, I decided that better tests are required. Possibly the best
     -    option here is to compare git results against hardcoded ground truth.
     +    After I discovered that UTF-16-LE-BOM test was bugged, I decided that
     +    better tests are required. Possibly the best option here is to compare
     +    git results against hardcoded ground truth.
      
          The new tests also cover more interesting chars where (ANSI != UTF-8).
      
     @@ -25,26 +25,26 @@
      +	orig_string="$2"
      +	expect_bytes="$3"
      +	
     -+	test_expect_success "Commit utf-8, checkout ${encoding}" '
     ++	test_expect_success "Commit UTF-8, checkout $encoding" '
      +		test_when_finished "git checkout HEAD -- .gitattributes" &&
      +		
     -+		test_ext="commit_utf8_checkout_${encoding}" &&
     -+		test_file="test.${test_ext}" &&
     ++		test_ext="commit_utf8_checkout_$encoding" &&
     ++		test_file="test.$test_ext" &&
      +		
     -+		# Commit as utf-8
     -+		echo "*.${test_ext} text working-tree-encoding=utf-8" >.gitattributes &&
     -+		printf "${orig_string}" >"${test_file}" &&
     -+		git add "${test_file}" &&
     ++		# Commit as UTF-8
     ++		echo "*.$test_ext text working-tree-encoding=UTF-8" >.gitattributes &&
     ++		printf "$orig_string" >$test_file &&
     ++		git add $test_file &&
      +		git commit -m "Test data" &&
      +
      +		# Checkout in tested encoding
     -+		rm "${test_file}" &&
     -+		echo "*.${test_ext} text working-tree-encoding=${encoding}" >.gitattributes &&
     -+		git checkout HEAD -- "${test_file}" &&
     ++		rm $test_file &&
     ++		echo "*.$test_ext text working-tree-encoding=$encoding" >.gitattributes &&
     ++		git checkout HEAD -- $test_file &&
      +		
      +		# Test
     -+		printf "${expect_bytes}" > "${test_file}.raw" &&
     -+		test_cmp_bin "${test_file}.raw" "${test_file}"
     ++		printf $expect_bytes >$test_file.raw &&
     ++		test_cmp_bin $test_file.raw $test_file
      +	'
      +}
      +

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

According to its name, the test is designed for UTF-16-LE-BOM. However, possibly due to copy&paste oversight, it was using UTF-32.

While the test succeeds (extra \000\000 are interpreted as NUL), I myself had an unrelated problem which caused the test to fail. When analyzing the failure I was quite puzzled by the fact that the test is obviously bugged. And it seems that I'm not alone:
https://public-inbox.org/git/CAH8yC8kSakS807d4jc_BtcUJOrcVT4No37AXSz=jePxhw-o9Dg@mail.gmail.com/T/#u

Fix the test to follow its original intention.

---
 t/t0028-working-tree-encoding.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 1090e650ed..5493cf3ca9 100755
@@ -40,7 +40,7 @@ test_expect_success 'setup test files' '
 	printf "$text" | write_utf16 >test.utf16.raw &&
 	printf "$text" | write_utf32 >test.utf32.raw &&
 	printf "\377\376"                         >test.utf16lebom.raw &&
-	printf "$text" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&
+	printf "$text" | iconv -f UTF-8 -t UTF-16LE >>test.utf16lebom.raw &&
 
 	# Line ending tests
 	printf "one\ntwo\nthree\n" >lf.utf8.raw &&

gitgitgadget

Thanks for the update -

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

s/bugged/buggy/ perhaps? Usually people do not place hidden listening devices in tests ;-)

Yes, hinting those hidden listening devices was an oversight. Thanks for your help in putting them back undercover!

I understand that you already changed the commit message, so I'm not submitting updated patch.