Re: [PATCH 2/2] t5556: replace test_i18ngrep with a simple grep

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 13, 2018, 07:51 UTC
Message-ID: <xmqqvaf1qqcx.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <69c7dc21-fb52-5982-f7d8-04518d06db6c@ramsayjones.plus.com>

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> I must admit that I didn't think about the effect of the useless
> "| sort" on the exit status!  What I saw was: a process that
> received no input, sorted nothing and produced no output - pretty
> much the definition of useless! ;-)

I am not sure what you mean by "receive no input, sort nothing and produce no output".

Ahh, OK, this is a funny one.  I think the original meant to do

	grep ... | grep -v ... | sort >actual

but it did

	grep ... | grep -v ... >actual | sort

instead by mistake.

And we have two possible "fixes" for that mistake. Either removing "|sort" (and replace its only effect, which is to hide brittleness of relying on exit status of the second grep, with something else) to declare that we do care about the order multiple warning messages are given by the last test in the script (by the way, the script is t5536, not t5556; the patch needs to be retitled), or keeping the "| sort" and move the redirection into ">actual" to the correct place, which is to follow through the intention of having that "sort" on the pipeline in the first place. I somewhat favor the former in this particular case myself, but the preference is not a very strong one.

Thanks.