Re: [LTP] [PATCH 2/2] testcases/lib: tst_net.sh: Do not use stderr in tst_rhost_run()

[Petr Vorel <pvorel@suse.cz>]

> Hi!
> > This is another place that broke the network test on the unrelated
> > changes that caused the SAFE_CLONE() to produce TINFO messages. The test
> > library prints messages into the stdout, because this is something that
>                                       ^
> 				      stderr

+1.

> > is not supposed to be the command output but rather diagnostic messages.
> > So there was no good reason to include the stderr in the data we got
> > from the tst_rhost_run().

> > If this patch does not break anything it should be pushed since this
> > will fix the tst_ns_* helpers even if the they start print diagnostics
> > TINFO messages again.

> > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> > ---
> >  testcases/lib/tst_net.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
> > index ee0ae1cad..60bc25b79 100644
> > --- a/testcases/lib/tst_net.sh
> > +++ b/testcases/lib/tst_net.sh
> > @@ -257,7 +257,7 @@ tst_rhost_run()
> >  		tst_res_ TINFO "$use: $rcmd \"$sh_cmd\" $out 2>&1"
> >  	fi

> > -	output=$($rcmd "$sh_cmd" $out 2>&1 || echo 'RTERR')
> > +	output=$($rcmd "$sh_cmd" $out || echo 'RTERR')

I'm not sure about this. Maybe it's safe, because stderr gets to the output.
But in case some network test need to verify something which is defined on
stderr it will fails (when tst_rhost_run is run with -s).

FYI *without* this patch:

	# route-redirect.sh
	...
	route-redirect 1 TINFO: timeout per run is 0h 5m 0s
	route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on '': 'sh: 1: ns-icmp_redirector: not found'
	route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on '': 'ns-icmp_redirector: no process found'
	ns-icmp_redirector: no process found

and *with* this patch:

	# route-redirect.sh
	...
	route-redirect 1 TINFO: timeout per run is 0h 5m 0s
	sh: 1: ns-icmp_redirector: not found
	route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on '': ''
	ns-icmp_redirector: no process found
	route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on '': ''

Info is there, but not on the same line (second '').

Also with the change below it would be more meaningful (fixing first empty ''):

route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on NETNS: 'sh: 1: ns-icmp_redirector: not found'
route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on NETNS: 'ns-icmp_redirector: no process found'
ns-icmp_redirector: no process found

I also wrote conditional quiet on stderr (not sent to ML), but actually if we
merge this and need sometimes to parse stderr, the opposite would be needed.

Kind regards,
Petr

+++ testcases/lib/tst_net.sh
@@ -263,7 +263,7 @@ tst_rhost_run()
 	if [ $ret -eq 1 ]; then
 		output=$(echo "$output" | sed 's/RTERR//')
 		[ "$safe" ] && \
-			tst_brk_ TBROK "'$cmd' failed on '$RHOST': '$output'"
+			tst_brk_ TBROK "'$cmd' failed on $use: '$output'"
 	fi
 
 	[ -z "$out" -a -n "$output" ] && echo "$output"

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp