Re: [PATCH v1 2/2] git-p4: suppress non test relevant output

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 21, 2015, 20:38 UTC
Message-ID: <xmqqd1tzfsfm.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1450629869-49522-3-git-send-email-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

Show 20 quoted lines

> From: Lars Schneider <larsxschneider@gmail.com>
>
> If tests are executed in verbose mode then the retry logic clutters the
> test output. Suppress that clutter.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>  t/lib-git-p4.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> index 30bf7ae..03f29c1 100644
> --- a/t/lib-git-p4.sh
> +++ b/t/lib-git-p4.sh
> @@ -174,7 +174,7 @@ retry_until_fail() {
>  	until ! "$@" 2>/dev/null || test $(time_in_seconds) -gt $timeout
>  	do
>  		sleep 1
> -	done
> +	done >/dev/null 2>&1

Eh, what does this squelch? The sleep in the body of the loop is silent, "test A -gt B" on the loop condition would be silent too, so you are squelching the invocation of "$@" whose standard error stream is already sent to 2>/dev/null?

If so, why not do it there instead? You seem to run only "kill" to send some signal to a process using this helper function, and it would be silent on its standard output stream (even though it may say "no such process" etc. on its standard error), so it is not clear to me what you are doing with this change here...