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

[Luke Diamand]

On 22/12/15 08:47, Lars Schneider wrote:


On 21 Dec 2015, at 21:38, Junio C Hamano  wrote:





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...


If I run git-p4 tests in verbose mode (e.g. "./t9823-git-p4-mock-lfs.sh -v") 
without this patch then the last lines of the output look like this:


Output Start >>>

expecting success:
kill_p4d

./lib-git-p4.sh: line 172: 26289 Killed: 9   while true; do
 if test $(time_in_seconds) -gt $timeout; then
 kill -9 $pid; exit 1;
 fi; sleep 1;
done
ok 8 - kill p4d

# passed all 8 test(s)
1..8
<<< Output end <<<

However, I want them to look like this:


Output Start >>>

expecting success:
kill_p4d

ok 8 - kill p4d

# passed all 8 test(s)
1..8
<<< Output end <<<

This is achieved with the patch. I am no shell expert ... is there a nicer way 
to achieve the same?


I get your desired output with the unmodified code from origin/next:

$ ./t9823-git-p4-mock-lfs.sh -v
expecting success:
kill_p4d

ok 8 - kill p4d

# passed all 8 test(s)
1..8

But that's because my shell is symlinked to /bin/dash. I suspect you are 
using bash - when I run this with bash I get your command output. 
Possibly a bash bug?


As Junio says, it seems a bit weird that we have to redirect the stderr 
of that entire expression.


Luke
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Lars Schneider]

On 21 Dec 2015, at 21:38, Junio C Hamano  wrote:

> larsxschnei...@gmail.com writes:
> 
>> From: Lars Schneider 
>> 
>> If tests are executed in verbose mode then the retry logic clutters the
>> test output. Suppress that clutter.
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> 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...

If I run git-p4 tests in verbose mode (e.g. "./t9823-git-p4-mock-lfs.sh -v") 
without this patch then the last lines of the output look like this:

>>> Output Start >>>
expecting success:
kill_p4d

./lib-git-p4.sh: line 172: 26289 Killed: 9   while true; do
if test $(time_in_seconds) -gt $timeout; then
kill -9 $pid; exit 1;
fi; sleep 1;
done
ok 8 - kill p4d

# passed all 8 test(s)
1..8 
<<< Output end <<<

However, I want them to look like this:

>>> Output Start >>>
expecting success:
kill_p4d

ok 8 - kill p4d

# passed all 8 test(s)
1..8
<<< Output end <<<

This is achieved with the patch. I am no shell expert ... is there a nicer way 
to achieve the same?

Thanks,
Lars


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> From: Lars Schneider 
>
> If tests are executed in verbose mode then the retry logic clutters the
> test output. Suppress that clutter.
>
> Signed-off-by: Lars Schneider 
> ---
>  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...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[larsxschneider]

From: Lars Schneider 

If tests are executed in verbose mode then the retry logic clutters the
test output. Suppress that clutter.

Signed-off-by: Lars Schneider 
---
 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
 }

 kill_p4d() {
--
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html