t0028 fix test + more tests

PATCH Superseded Alexandr Miloslavskiy via GitGitGadget 5 participants 18 messages Sep 23, 2019 – Sep 30, 2019

Share & tools

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-23 08:33 UTC #0 lore raw

cover 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

Signature · 1 line

gitgitgadget

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-23 08:33 UTC #1 lore raw

PATCH 1/2 t0028: fix test for UTF-16-LE-BOM

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.

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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

t/t0028-working-tree-encoding.sh +1 -1

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 &&

Signature · 1 line

gitgitgadget

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-23 08:33 UTC #2 lore raw

PATCH 2/2 t0028: add more tests

In reply to Alexandr Miloslavskiy via 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).

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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

t/t0028-working-tree-encoding.sh +39 -0

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

Signature · 1 line

gitgitgadget

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-23 10:04 UTC #3 lore raw

v2 cover Update: fixed typos in commit message

In reply to Alexandr Miloslavskiy via 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

Signature · 1 line

gitgitgadget

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-23 10:04 UTC #4 lore raw

PATCH v2 1/2 t0028: fix test for UTF-16-LE-BOM

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.

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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

t/t0028-working-tree-encoding.sh +1 -1

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 &&

Signature · 1 line

gitgitgadget

Torsten Bögershausen

2019-09-24 03:46 UTC #5 lore raw

Re: [PATCH v2 1/2] t0028: fix test for UTF-16-LE-BOM

Quoted 6 lines · Alexandr Miloslavskiy via 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

We can probably drop the "probably" ?

Quoted 1 line

zero), I myself had an unrelated problem which caused the test to fail.

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

Quoted 5 lines

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.

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.

Quoted 23 lines

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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

t/t0028-working-tree-encoding.sh +1 -2

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

Alexandr Miloslavskiy

2019-09-24 10:02 UTC #6 lore raw

Re: [PATCH v2 1/2] t0028: fix test for UTF-16-LE-BOM

Quoted 2 lines · Torsten Bögershausen

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

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]/

Quoted 1 line

How about the following:

Sure thing, I will use the wording you suggested.

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-23 10:04 UTC #7 lore raw

PATCH v2 2/2 t0028: add more tests

In reply to Alexandr Miloslavskiy via GitGitGadget

From: Alexandr Miloslavskiy <[email protected]>

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

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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

t/t0028-working-tree-encoding.sh +39 -0

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

Signature · 1 line

gitgitgadget

Torsten Bögershausen

2019-09-24 04:09 UTC #8 lore raw

Re: [PATCH v2 2/2] t0028: add more tests

Quoted 1 line · Alexandr Miloslavskiy via GitGitGadget

From: Alexandr Miloslavskiy <[email protected]>

Thanks for the tests, some nit-picks inline.

Quoted 3 lines

After I discovered that UTF-16-LE-BOM test was bugged and still succeeded...

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

Quoted 19 lines

... 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).
Signed-off-by Alexandr Miloslavskiy <[email protected]>
---
 t/t0028-working-tree-encoding.sh | 39 ++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
t/t0028-working-tree-encoding.sh +0 -0
index 5493cf3ca9..d0dd5dd0ea 100755
@@ -280,4 +280,43 @@ test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' '
 	git reset
 '

t/t0028-working-tree-encoding.sh test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' ' · line 291

Quoted 10 lines

t/t0028-working-tree-encoding.sh +9 -0

 
+# $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}" '

General remark:
Do we need the {} here?
${encoding} could be simpler written as $encoding

t/t0028-working-tree-encoding.sh test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' ' · line 297

Quoted 6 lines

t/t0028-working-tree-encoding.sh +6 -0

+		test_when_finished "git checkout HEAD -- .gitattributes" &&
+
+		test_ext="commit_utf8_checkout_${encoding}" &&
+		test_file="test.${test_ext}" &&
+
+		# Commit as utf-8

Another nit-pick:
Looking at the other test cases, should utf-8 be written as UTF-8 for consistency ?

t/t0028-working-tree-encoding.sh test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' ' · line 310

Quoted 13 lines

t/t0028-working-tree-encoding.sh +13 -0

+		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}"

More a style-nit: could we simply write like this:
printf $expect_bytes > $test_file.raw && test_cmp_bin $test_file.raw $test_file

(Even on other places)

Quoted 15 lines

t/t0028-working-tree-encoding.sh +11 -1

+	'
+}
+
+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

Alexandr Miloslavskiy

2019-09-24 10:04 UTC #9 lore raw

Re: [PATCH v2 2/2] t0028: add more tests

Quoted 3 lines · Torsten Bögershausen

Would this make more sense:
After I discovered that UTF-16-LE-BOM test was bugged, I decided that better tests are required

Quoted 2 lines

Looking at the other test cases, should utf-8 be written as UTF-8 for consistency ?

Quoted 7 lines

General remark:
Do we need the {} here?
${encoding} could be simpler written as $encoding
More a style-nit: could we simply write like this:
printf $expect_bytes > $test_file.raw && test_cmp_bin $test_file.raw $test_file

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

Johannes Sixt

2019-09-24 06:21 UTC #10 lore raw

Re: [PATCH v2 2/2] t0028: add more tests

In reply to Alexandr Miloslavskiy via GitGitGadget

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

Quoted 7 lines

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).

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

Alexandr Miloslavskiy

2019-09-24 10:31 UTC #11 lore raw

Re: [PATCH v2 2/2] t0028: add more tests

Quoted 6 lines · Johannes Sixt

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)?

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.

Quoted 3 lines

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)?

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.

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-24 10:40 UTC #12 lore raw

v3 cover Update: fixed typos in commit message

In reply to Alexandr Miloslavskiy via 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.

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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
      +	'
      +}
      +

Signature · 1 line

gitgitgadget

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-24 10:40 UTC #13 lore raw

PATCH v3 1/2 t0028: fix test for UTF-16-LE-BOM

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.

Fix the test to follow its original intention.

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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

t/t0028-working-tree-encoding.sh +1 -1

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 &&

Signature · 1 line

gitgitgadget

Alexandr Miloslavskiy via GitGitGadget

author

2019-09-24 10:40 UTC #14 lore raw

PATCH v3 2/2 t0028: add more tests

In reply to Alexandr Miloslavskiy via GitGitGadget

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).

Signed-off-by Alexandr Miloslavskiy <[email protected]>

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

t/t0028-working-tree-encoding.sh +39 -0

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

Signature · 1 line

gitgitgadget

Junio C Hamano

2019-09-28 04:48 UTC #15 lore raw

Re: [PATCH v3 2/2] t0028: add more tests

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

Quoted 3 lines · writes:

From: Alexandr Miloslavskiy <[email protected]>
After I discovered that UTF-16-LE-BOM test was bugged, I decided that

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

Quoted 10 lines

better tests are required. Possibly the best option here is to compare git results against hardcoded ground truth.
...

t/t0028-working-tree-encoding.sh +7 -0

+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"

Alexandr Miloslavskiy

2019-09-30 07:29 UTC #16 lore raw

Re: [PATCH v3 2/2] t0028: add more tests

Quoted 2 lines · Junio C Hamano

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.

Torsten Bögershausen

Reviewed-by on the v3 series

2019-09-26 19:37 UTC #17 lore raw

Re: [PATCH v3 0/2] Update: fixed typos in commit message

In reply to Alexandr Miloslavskiy via GitGitGadget

Quoted 7 lines · Alexandr Miloslavskiy via 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

Thanks for the update -

Reviewed-by Torsten Bögershausen <[email protected]>