Re: [PATCH 19/21] t9003: become resilient to GETTEXT_POISON

From: Vasco Almeida <vascomalmeida@sapo.pt>
Date: May 19, 2016, 21:31 UTC
Message-ID: <573E30C8.4070600@sapo.pt>
In-Reply-To: <CAPig+cT3yf7D4xOmOhy5Y21qwHuA5Ny9ULEJhC1OBgrhiayQ3g@mail.gmail.com>

Às 18:34 de 19-05-2016, Eric Sunshine escreveu:

Show 31 quoted lines

> On Wed, May 18, 2016 at 11:27 AM, Vasco Almeida <vascomalmeida@sapo.pt> wrote:
>> The test t9003-help-autocorrect.sh fails when run under GETTEXT_POISON,
>> because it's expecting to filter out the original output. Accommodate
>> gettext poison case by also filtering out the default simulated output.
>>
>> Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
>> ---
>> diff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh
>> @@ -31,10 +31,14 @@ test_expect_success 'autocorrect showing candidates' '
>>         git config help.autocorrect 0 &&
>>
>>         test_must_fail git lfg 2>actual &&
>> -       sed -e "1,/^Did you mean this/d" actual | grep lgf &&
>> +       sed -e "1,/^Did you mean this/d" actual |
>> +       sed -e "/GETTEXT POISON/d" actual |
> 
> Why not do so with a single sed invocation?
> 
>    sed -e "..." -e "..." |
> 
>> +       grep lgf &&
>>
>>         test_must_fail git distimdist 2>actual &&
>> -       sed -e "1,/^Did you mean this/d" actual | grep distimdistim
>> +       sed -e "1,/^Did you mean this/d" actual |
>> +       sed -e "/GETTEXT POISON/d" actual |
> 
> Ditto.
> 
>> +       grep distimdistim
>>  '

I tried but it seems not to work.

Actually I did this wrong, I haven't thought this through.

Under gettext poison: sed -e "1,/^Did you mean this/d" removes all lines, gives no output. And then the one second, sed -e "/GETTEXT POISON/d", outputs "lgf" as expected.

But running normally (without gettext poison): 1st sed outputs "lgf" as expected And then second one output the entire 'actual' file, like if it were cat, undoing the first sed.

I think the sed here is superfluous in the first place. Should we remove it? If it weren't the case of gettext poison it was ok to have sed there, but it makes the test fail under gettext poison.