Re: [PATCH 3/3] commit: add an option the reword HEAD

2 messages, 2 authors, 2020-09-22 · open the first message on its own page

Re: [PATCH 3/3] commit: add an option the reword HEAD

From: Junio C Hamano <hidden>
Date: 2020-09-21 19:27:38

Phillip Wood [off-list ref] writes:
quoted
quoted
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
@@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '
+test_reword_opt () {
+       test_expect_success C_LOCALE_OUTPUT "--reword incompatible with $1" "
+               echo 'fatal: cannot combine --reword with $1' >expect &&
+               test_must_fail git commit --reword $1 2>actual &&
+               test_cmp expect actual
+       "
+}
These error messages are subject to localization, so you'd want to
use
test_i18ncmp() here, I think.
Same comment for other new tests.
I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather
than grep so I could check the exact output.
I do not think it is a good idea.  Dropping the C_LOCALE_OUTPUT
prerequisite and using test_i18ncmpw would be more appropriate.

A test run without GIT_TEST_GETTEXT_POISON will do the byte-for-byte
comparison like test_cmp.  It is only the poison test, whose purpose
is to catch commands that by mistake translated their messages, that
would want to mark a test that checks end-user facing messages like
this one as special with test_i18ncmp.
... I should probably check that nothing is printed to stdout in
these tests
Perhaps, but that is not the point of "do we diagnose options thare
are incompatble with --reword?" test.

Re: [PATCH 3/3] commit: add an option the reword HEAD

From: Phillip Wood <hidden>
Date: 2020-09-22 13:38:09

Hi Junio

On 21/09/2020 20:27, Junio C Hamano wrote:
Phillip Wood [off-list ref] writes:
quoted
quoted
quoted
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
@@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '
+test_reword_opt () {
+       test_expect_success C_LOCALE_OUTPUT "--reword incompatible with $1" "
+               echo 'fatal: cannot combine --reword with $1' >expect &&
+               test_must_fail git commit --reword $1 2>actual &&
+               test_cmp expect actual
+       "
+}
These error messages are subject to localization, so you'd want to
use
test_i18ncmp() here, I think.
Same comment for other new tests.
I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather
than grep so I could check the exact output.
I do not think it is a good idea.  Dropping the C_LOCALE_OUTPUT
prerequisite and using test_i18ncmpw would be more appropriate.

A test run without GIT_TEST_GETTEXT_POISON will do the byte-for-byte
comparison like test_cmp.  It is only the poison test, whose purpose
is to catch commands that by mistake translated their messages, that
would want to mark a test that checks end-user facing messages like
this one as special with test_i18ncmp.
Thanks I wasn't aware of test_i18ncmp
quoted
... I should probably check that nothing is printed to stdout in
these tests
Perhaps, but that is not the point of "do we diagnose options thare
are incompatble with --reword?" test.
I think it depends if one views the test as checking "do we diagnose 
options there are incompatible with --reword?" or "what do we show the 
user when there are options that are incompatible with --reword". For 
the former we just want to check that the correct error message is 
printed, for the latter we want to check that only what we expect to be 
printed is actually printed.

Best Wishes

Phillip
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help