threads / patch / 34608

patchAdd missing test file for UTF-16.

Subject: [PATCH] Add missing test file for UTF-16.

## tl;dr

6 messages between Aug 3, 2013 and Aug 6, 2013. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

brian m. carlson· Aug 3, 2013, 17:26 UTC · lore
The test file that the UTF-16 rejection test looks for is missing, but this went
unnoticed because the test is expected to fail anyway; as a consequence, the
test fails because the file containing the commit message is missing, and not
because the test file contains a NUL byte.  Fix this by including a sample text
file containing a commit message encoded in UTF-16.
---
 t/t3900/UTF-16.txt | Bin 0 -> 146 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 t/t3900/UTF-16.txt
Show changes to t/t3900/UTF-16.txt +0 −0
diff --git a/t/t3900/UTF-16.txt b/t/t3900/UTF-16.txt
new file mode 100644
index 0000000000000000000000000000000000000000..2257f05a992a4b9500f6ff33752cbdf8fb58c99d
GIT binary patch
literal 146
zcmW-aJqm<C6obFoQ_KYvZ0uz~uwFfY_=AGZf}@2;@>bjKEzFY?FCi}po-doQaNvlX
zuj<&j$Vh~SS#Fd&>8^}o3xQtzWRNy5zCGpzu|P`62Tv_HZgu?B*(r7K7qg7DI9e@K
J`p+qB@d1eo8QA~;

literal 0
HcmV?d00001

-- 
1.8.4.rc1
Duy Nguyen· Aug 4, 2013, 02:31 UTC · re: brian m. carlson · lore

Re: [PATCH] Add missing test file for UTF-16.

On Sun, Aug 4, 2013 at 12:26 AM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 5 quoted lines
> The test file that the UTF-16 rejection test looks for is missing, but this went
> unnoticed because the test is expected to fail anyway; as a consequence, the
> test fails because the file containing the commit message is missing, and not
> because the test file contains a NUL byte.  Fix this by including a sample text
> file containing a commit message encoded in UTF-16.
Tested-by: Duy Nguyen <pclouds@gmai.com>
and sorry, my bad. I think we need your sign-off in this patch.
Show 20 quoted lines
> ---
>  t/t3900/UTF-16.txt | Bin 0 -> 146 bytes
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  create mode 100644 t/t3900/UTF-16.txt
>
> diff --git a/t/t3900/UTF-16.txt b/t/t3900/UTF-16.txt
> new file mode 100644
> index 0000000000000000000000000000000000000000..2257f05a992a4b9500f6ff33752cbdf8fb58c99d
> GIT binary patch
> literal 146
> zcmW-aJqm<C6obFoQ_KYvZ0uz~uwFfY_=AGZf}@2;@>bjKEzFY?FCi}po-doQaNvlX
> zuj<&j$Vh~SS#Fd&>8^}o3xQtzWRNy5zCGpzu|P`62Tv_HZgu?B*(r7K7qg7DI9e@K
> J`p+qB@d1eo8QA~;
>
> literal 0
> HcmV?d00001
>
> --
> 1.8.4.rc1
>
-- 
Duy
brian m. carlson· Aug 4, 2013, 16:06 UTC · re: Duy Nguyen · lore

Re: [PATCH] Add missing test file for UTF-16.

On Sun, Aug 04, 2013 at 09:31:01AM +0700, Duy Nguyen wrote:
> Tested-by: Duy Nguyen <pclouds@gmai.com>
I think you're missing an "l" in the domain name there.
> and sorry, my bad. I think we need your sign-off in this patch.

I just ran across it because I'm going through all the TODO tests and seeing which ones I can fix, and I generally just CC the last three or four people who touched the files in question.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Junio C Hamano· Aug 5, 2013, 16:44 UTC · re: Duy Nguyen · lore

Re: [PATCH] Add missing test file for UTF-16.

Duy Nguyen <pclouds@gmail.com> writes:
Show 11 quoted lines
> On Sun, Aug 4, 2013 at 12:26 AM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>> The test file that the UTF-16 rejection test looks for is missing, but this went
>> unnoticed because the test is expected to fail anyway; as a consequence, the
>> test fails because the file containing the commit message is missing, and not
>> because the test file contains a NUL byte.  Fix this by including a sample text
>> file containing a commit message encoded in UTF-16.
>
> Tested-by: Duy Nguyen <pclouds@gmai.com>
>
> and sorry, my bad. I think we need your sign-off in this patch.

I think 37576c14 (commit_tree(): refuse commit messages that contain NULs, 2011-12-15) that marked this test with "test_expect_failure" is broken with or without this fix. It should be more like so:

Show changes to t/t3900-i18n-commit.sh +2 −2
diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index 37ddabb..5e72d72 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -34,9 +34,9 @@ test_expect_success 'no encoding header for base case' '
 	test z = "z$E"
 '
 
-test_expect_failure 'UTF-16 refused because of NULs' '
+test_expect_success 'UTF-16 refused because of NULs' '
 	echo UTF-16 >F &&
-	git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
+	test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
 '
Duy Nguyen· Aug 6, 2013, 11:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add missing test file for UTF-16.

On Mon, Aug 5, 2013 at 11:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 32 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Sun, Aug 4, 2013 at 12:26 AM, brian m. carlson
>> <sandals@crustytoothpaste.net> wrote:
>>> The test file that the UTF-16 rejection test looks for is missing, but this went
>>> unnoticed because the test is expected to fail anyway; as a consequence, the
>>> test fails because the file containing the commit message is missing, and not
>>> because the test file contains a NUL byte.  Fix this by including a sample text
>>> file containing a commit message encoded in UTF-16.
>>
>> Tested-by: Duy Nguyen <pclouds@gmai.com>
>>
>> and sorry, my bad. I think we need your sign-off in this patch.
>
> I think 37576c14 (commit_tree(): refuse commit messages that contain
> NULs, 2011-12-15) that marked this test with "test_expect_failure" is
> broken with or without this fix.  It should be more like so:
>
> diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
> index 37ddabb..5e72d72 100755
> --- a/t/t3900-i18n-commit.sh
> +++ b/t/t3900-i18n-commit.sh
> @@ -34,9 +34,9 @@ test_expect_success 'no encoding header for base case' '
>         test z = "z$E"
>  '
>
> -test_expect_failure 'UTF-16 refused because of NULs' '
> +test_expect_success 'UTF-16 refused because of NULs' '
>         echo UTF-16 >F &&
> -       git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
> +       test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
>  '

The intention was "UTF-16 is not supported yet but we want to". But I don't think we (at least I) will put any effort on that front to allow NUL in commit message, so the patch, as in "we do not support UTF-16", is fine.

-- 
Duy
Junio C Hamano· Aug 6, 2013, 17:24 UTC · re: Duy Nguyen · lore

Re: [PATCH] Add missing test file for UTF-16.

Duy Nguyen <pclouds@gmail.com> writes:
> The intention was "UTF-16 is not supported yet but we want to". But I
> don't think we (at least I) will put any effort on that front to allow
> NUL in commit message, so the patch, as in "we do not support UTF-16",
> is fine.
Agreed.  Here is what I queued.
-- >8 --
Subject: [PATCH] t3900: test rejecting log message with NULs correctly

It is not like that our longer term desire is to someday start accepting log messages with NULs in them, so it is wrong to mark a test that demonstrates "git commit" that correctly fails given such an input as "expect-failure". "git commit" should fail today, and it should fail the same way in the future given a message with NUL in it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3900-i18n-commit.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to t/t3900-i18n-commit.sh +2 −2
diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index d48a7c0..a9e5662 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -34,9 +34,9 @@ test_expect_success 'no encoding header for base case' '
 	test z = "z$E"
 '
 
-test_expect_failure 'UTF-16 refused because of NULs' '
+test_expect_success 'UTF-16 refused because of NULs' '
 	echo UTF-16 >F &&
-	git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
+	test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
 '

← back to recent threads