threads / patch / 42917

patcht5510: become resilient to GETTEXT_POISON

Subject: [PATCH] t5510: become resilient to GETTEXT_POISON

## tl;dr

6 messages between Jul 25, 2016 and Jul 26, 2016. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Vasco Almeida· Jul 25, 2016, 09:31 UTC · lore

Replace gettext poison text with appropriate values to be able to cut the right output of git fetch command for comparison.

The first gettext poison falls from the previous line into the next because the poison does not add a newline, so we must replace it with nothing.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 t/t5510-fetch.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
Show changes to t/t5510-fetch.sh +8 −2
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 6bd4853..b261223 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -694,7 +694,10 @@ test_expect_success 'fetch aligned output' '
 	(
 		cd full-output &&
 		git -c fetch.output=full fetch origin 2>&1 | \
-			grep -e "->" | cut -c 22- >../actual
+			grep -e "->" | \
+			sed -e "/master/ s/# GETTEXT POISON #//" \
+			    -e "/tag/ s/# GETTEXT POISON #/[new tag]        /" | \
+			cut -c 22- >../actual
 	) &&
 	cat >expect <<-\EOF &&
 	master               -> origin/master
@@ -709,7 +712,10 @@ test_expect_success 'fetch compact output' '
 	(
 		cd compact &&
 		git -c fetch.output=compact fetch origin 2>&1 | \
-			grep -e "->" | cut -c 22- >../actual
+			grep -e "->" | \
+			sed -e "/master/ s/# GETTEXT POISON #//" \
+			    -e "/extraa/ s/# GETTEXT POISON #/[new tag]        /" | \
+			cut -c 22- >../actual
 	) &&
 	cat >expect <<-\EOF &&
 	master     -> origin/*

-- 
2.7.4
Junio C Hamano· Jul 25, 2016, 15:16 UTC · re: Vasco Almeida · lore

Re: [PATCH] t5510: become resilient to GETTEXT_POISON

On Mon, Jul 25, 2016 at 2:31 AM, Vasco Almeida <vascomalmeida@sapo.pt> wrote:
> Replace gettext poison text with appropriate values to be able to cut
> the right output of git fetch command for comparison.

Hmm, as these tests are _all_ about human-readable output, it probably is sufficient to skip them using prerequiste, I would think. We do not want each individual test to have too intimate knowledge on how POISON strings look like.

Duy Nguyen· Jul 25, 2016, 16:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] t5510: become resilient to GETTEXT_POISON

On Mon, Jul 25, 2016 at 5:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> On Mon, Jul 25, 2016 at 2:31 AM, Vasco Almeida <vascomalmeida@sapo.pt> wrote:
>> Replace gettext poison text with appropriate values to be able to cut
>> the right output of git fetch command for comparison.
>
> Hmm, as these tests are _all_ about human-readable output, it probably is
> sufficient to skip them using prerequiste, I would think. We do not want each
> individual test to have too intimate knowledge on how POISON strings look
> like.

Yeah these tests are about alignment, they are probably useless anyway after the text is poisoned (and has the same length).

-- 
Duy
Vasco Almeida· Jul 26, 2016, 12:58 UTC · re: Vasco Almeida · lore

[PATCH] t5510: skip tests under GETTEXT_POISON build

Skip tests when running under GETTEXT_POISON build and run them with C_LOCALE_OUTPUT prerequisite.

These tests are irrelevant under GETTEXT_POISON because they test text output alignment which GETTEXT_POISON turns useless.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 t/t5510-fetch.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to t/t5510-fetch.sh +2 −2
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 6bd4853..668c54b 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -688,7 +688,7 @@ test_expect_success 'fetching with auto-gc does not lock up' '
 	)
 '
 
-test_expect_success 'fetch aligned output' '
+test_expect_success C_LOCALE_OUTPUT 'fetch aligned output' '
 	git clone . full-output &&
 	test_commit looooooooooooong-tag &&
 	(
@@ -703,7 +703,7 @@ test_expect_success 'fetch aligned output' '
 	test_cmp expect actual
 '
 
-test_expect_success 'fetch compact output' '
+test_expect_success C_LOCALE_OUTPUT 'fetch compact output' '
 	git clone . compact &&
 	test_commit extraaa &&
 	(

-- 
2.7.4
Junio C Hamano· Jul 26, 2016, 16:53 UTC · re: Vasco Almeida · lore

Re: [PATCH] t5510: skip tests under GETTEXT_POISON build

Vasco Almeida <vascomalmeida@sapo.pt> writes:
Show 33 quoted lines
> Skip tests when running under GETTEXT_POISON build and run them with
> C_LOCALE_OUTPUT prerequisite.
>
> These tests are irrelevant under GETTEXT_POISON because they test text
> output alignment which GETTEXT_POISON turns useless.
>
> Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> ---
>  t/t5510-fetch.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index 6bd4853..668c54b 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -688,7 +688,7 @@ test_expect_success 'fetching with auto-gc does not lock up' '
>  	)
>  '
>  
> -test_expect_success 'fetch aligned output' '
> +test_expect_success C_LOCALE_OUTPUT 'fetch aligned output' '
>  	git clone . full-output &&
>  	test_commit looooooooooooong-tag &&
>  	(
> @@ -703,7 +703,7 @@ test_expect_success 'fetch aligned output' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success 'fetch compact output' '
> +test_expect_success C_LOCALE_OUTPUT 'fetch compact output' '
>  	git clone . compact &&
>  	test_commit extraaa &&
>  	(
Makes sense, will queue.

This is a tangent, but it may make sense for us to start thinking about retiring one of the two prerequisites, GETTEXT_POISON and C_LOCALE_OUTPUT. Back when 5e9637c6 (i18n: add infrastructure for translating Git with gettext, 2011-11-18) introduced the former, test_have_prereq did not support a negated prerequisite, so the commit added GETTEXT_POISON prerequisite; if we had the modern test_have_prereq, we would have written

    test_expect_success GETTEXT_POISON '...'
that appear in t0205 as
    test_expect_success !C_LOCALE_OUTPUT '...'
I would think.
Ævar Arnfjörð Bjarmason· Jul 26, 2016, 20:11 UTC · re: Junio C Hamano · lore

Re: [PATCH] t5510: skip tests under GETTEXT_POISON build

On Tue, Jul 26, 2016 at 6:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> [...] Back when 5e9637c6 (i18n: add infrastructure for
> translating Git with gettext, 2011-11-18) introduced the former,
> test_have_prereq did not support a negated prerequisite, so the
> commit added GETTEXT_POISON prerequisite; if we had the modern
> test_have_prereq, we would have written
>
>     test_expect_success GETTEXT_POISON '...'
>
> that appear in t0205 as
>
>     test_expect_success !C_LOCALE_OUTPUT '...'
>
> I would think.

Maybe the names of the test prerequisites should be merged. I can't think of a rea

As for the GETTEXT_POISON facility in general, I haven't worked much if at all on the i18n toolchain since I initially wrote the gettext support so I think at this point it's for others to say whether stuff like this is useful.

But for what it's worth the v1.7.4.1-65-gbb946bb commit explains better what it's for:

    This is a debugging aid for people who are working on the i18n part of
    the system, to make sure that they are not marking plumbing messages
    that should never be translated with _().

I.e. so the person gettext-izing something can actively spot issues with marking strings for translations right away.

← back to recent threads