diff mbox series

[4/4] network: Fix tst_brk TFAIL

Message ID 20240123162647.210424-5-pvorel@suse.cz
State Accepted
Headers show
Series shell: fix regression since 1878502f6 | expand

Commit Message

Petr Vorel Jan. 23, 2024, 4:26 p.m. UTC 
It needs to be replaced with tst_res TFAIL and return

Fixes: 1878502f6 ("tst_test.sh/tst_brk(): Allow only TBROK and TCONF")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/nfs/nfs_stress/nfs06.sh     |  5 ++++-
 testcases/network/stress/route/route-lib.sh   |  8 +++++--
 testcases/network/stress/ssh/ssh-stress.sh    | 16 ++++++++++----
 testcases/network/tcp_cmds/host/host01.sh     |  5 +++--
 .../network/tcp_cmds/ipneigh/ipneigh01.sh     | 21 ++++++++++++-------
 testcases/network/virt/virt_lib.sh            | 12 +++++++----
 testcases/network/xinetd/xinetd_tests.sh      |  7 ++++---
 7 files changed, 50 insertions(+), 24 deletions(-)

Comments

Martin Doucha Jan. 24, 2024, 12:34 p.m. UTC | #1 
Hi,
two comments below.

On 23. 01. 24 17:26, Petr Vorel wrote:
> It needs to be replaced with tst_res TFAIL and return
> 
> Fixes: 1878502f6 ("tst_test.sh/tst_brk(): Allow only TBROK and TCONF")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   testcases/network/nfs/nfs_stress/nfs06.sh     |  5 ++++-
>   testcases/network/stress/route/route-lib.sh   |  8 +++++--
>   testcases/network/stress/ssh/ssh-stress.sh    | 16 ++++++++++----
>   testcases/network/tcp_cmds/host/host01.sh     |  5 +++--
>   .../network/tcp_cmds/ipneigh/ipneigh01.sh     | 21 ++++++++++++-------
>   testcases/network/virt/virt_lib.sh            | 12 +++++++----
>   testcases/network/xinetd/xinetd_tests.sh      |  7 ++++---
>   7 files changed, 50 insertions(+), 24 deletions(-)
> 
> diff --git a/testcases/network/nfs/nfs_stress/nfs06.sh b/testcases/network/nfs/nfs_stress/nfs06.sh
> index 560df05b1..09e55fe34 100755
> --- a/testcases/network/nfs/nfs_stress/nfs06.sh
> +++ b/testcases/network/nfs/nfs_stress/nfs06.sh
> @@ -34,7 +34,10 @@ do_test()
>   
>   	tst_res TINFO "waiting for pids:$pids"
>   	for p in $pids; do
> -		wait $p || tst_brk TFAIL "fsstress process failed"
> +		if ! wait $p; then
> +			tst_res TFAIL "fsstress process failed"
> +			return
> +		fi
>   		tst_res TINFO "fsstress '$p' completed"
>   	done
>   	pids=
> diff --git a/testcases/network/stress/route/route-lib.sh b/testcases/network/stress/route/route-lib.sh
> index 163c15423..29aa2e913 100644
> --- a/testcases/network/stress/route/route-lib.sh
> +++ b/testcases/network/stress/route/route-lib.sh
> @@ -97,10 +97,14 @@ test_netlink()
>   	tst_res TINFO "running $cmd $opt"
>   	$cmd $opt || ret=$?
>   	if [ "$ret" -ne 0 ]; then
> -		[ $((ret & 3)) -ne 0 ] && \
> -			tst_brk TFAIL "$cmd failed"
> +		if [ $((ret & 3)) -ne 0 ]; then
> +			tst_res TFAIL "$cmd failed"
> +			return
> +		fi
> +
>   		[ $((ret & 32)) -ne 0 ] && \
>   			tst_brk TCONF "not supported configuration"
> +
>   		[ $((ret & 4)) -ne 0 ] && \
>   			tst_res TWARN "$cmd has warnings"
>   	fi
> diff --git a/testcases/network/stress/ssh/ssh-stress.sh b/testcases/network/stress/ssh/ssh-stress.sh
> index e7c4d45ce..c27c27a28 100755
> --- a/testcases/network/stress/ssh/ssh-stress.sh
> +++ b/testcases/network/stress/ssh/ssh-stress.sh
> @@ -93,8 +93,10 @@ IdentityFile $TST_TMPDIR/id_rsa\n\" > $RHOST_SSH_CONF"
>   
>   test_ssh_connectivity()
>   {
> -	tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"
> -	[ $? -ne 0 ] && tst_brk TFAIL "SSH not reachable"
> +	if ! tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"; then
> +		tst_res TFAIL "SSH not reachable"
> +		return
> +	fi
>   }
>   
>   test1()
> @@ -121,7 +123,10 @@ test1()
>   		[ $? -ne 0 ] && num=$((num + 1))
>   	done
>   
> -	[ $num -ne 0 ] && tst_brk TFAIL "$num ssh processes died unexpectedly during execution"
> +	if [ $num -ne 0 ]; then
> +		tst_res TFAIL "$num ssh processes died unexpectedly during execution"
> +		return
> +	fi
>   
>   	test_ssh_connectivity
>   
> @@ -216,7 +221,10 @@ test3()
>   
>   	# Setup an ssh tunnel from the remote host to testhost
>   	tst_rhost_run -c "$RHOST_SSH -f -N -L $lport:$rhost:$port </dev/null >/dev/null 2>&1"
> -	[ "$?" -ne 0 ] && tst_brk TFAIL "Failed to create an SSH session with port forwarding"
> +	if [ "$?" -ne 0 ]; then
> +		tst_res TFAIL "Failed to create an SSH session with port forwarding"
> +		return
> +	fi
>   	RHOST_PIDS=$(tst_rhost_run -c "pgrep -f '^ssh .*$lport:$rhost:$port'")
>   
>   	# Start the TCP traffic clients
> diff --git a/testcases/network/tcp_cmds/host/host01.sh b/testcases/network/tcp_cmds/host/host01.sh
> index 6a4067495..9ed44bae3 100755
> --- a/testcases/network/tcp_cmds/host/host01.sh
> +++ b/testcases/network/tcp_cmds/host/host01.sh
> @@ -21,11 +21,12 @@ do_test()
>   	if addr=$(host $lhost); then
>   		addr=$(echo "$addr" | grep address | tail -1 | awk '{print $NF}')
>   		if [ -z "$addr" ]; then
> -			tst_brk TFAIL "empty address"
> +			tst_res TFAIL "empty address"
> +			return
>   		fi
>   		EXPECT_PASS host $addr \>/dev/null
>   	else
> -		tst_brk TFAIL "host $lhost on local machine failed"
> +		tst_res TFAIL "host $lhost on local machine failed"
>   	fi
>   }
>   
> diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> index e67ff5cc8..4b818357e 100755
> --- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> +++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> @@ -57,8 +57,10 @@ do_test()
>   
>   	for i in $(seq 1 $NUMLOOPS); do
>   
> -		ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null || \
> -			tst_brk TFAIL "cannot ping $(tst_ipaddr rhost)"
> +		if ! ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null; then
> +			tst_res TFAIL "cannot ping $(tst_ipaddr rhost)"
> +			return
> +		fi
>   
>   		local k
>   		local ret=1
> @@ -66,19 +68,22 @@ do_test()
>   			$SHOW_CMD | grep -q $(tst_ipaddr rhost)
>   			if [ $? -eq 0 ]; then
>   				ret=0
> -				break;
> +				break
>   			fi
>   			tst_sleep 100ms
>   		done
>   
> -		[ "$ret" -ne 0 ] && \
> -			tst_brk TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
> +		if [ "$ret" -ne 0 ]; then
> +			tst_res TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
> +			return
> +		fi
>   
>   		$DEL_CMD
>   
> -		$SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)" && \
> -			tst_brk TFAIL "'$DEL_CMD' failed, entry has " \
> -				"$(tst_hwaddr rhost)' $i/$NUMLOOPS"
> +		if $SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)"; then
> +			tst_res TFAIL "'$DEL_CMD' failed, entry has $(tst_hwaddr rhost)' $i/$NUMLOOPS"
> +			return
> +		fi
>   	done
>   
>   	tst_res TPASS "verified adding/removing $entry_name cache entry"
> diff --git a/testcases/network/virt/virt_lib.sh b/testcases/network/virt/virt_lib.sh
> index d7e694eb7..0aadbfdb9 100644
> --- a/testcases/network/virt/virt_lib.sh
> +++ b/testcases/network/virt/virt_lib.sh
> @@ -174,8 +174,10 @@ virt_multiple_add_test()
>   	tst_res TINFO "add $NS_TIMES $virt_type, then delete"
>   
>   	for i in $(seq $start_id $max); do
> -		virt_add ltp_v$i id $i $opt || \
> -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> +		if ! virt_add ltp_v$i id $i $opt; then
> +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> +			return

This is a library function so tst_brk TBROK is probably the correct fix 
here. Also, the message should say ltp_v$i instead of ltp_v0, but that's 
for a separate patch.

> +		fi
>   		ROD_SILENT "ip link set ltp_v$i up"
>   	done
>   
> @@ -196,8 +198,10 @@ virt_add_delete_test()
>   	tst_res TINFO "add/del $virt_type $NS_TIMES times"
>   
>   	for i in $(seq 0 $max); do
> -		virt_add ltp_v0 $opt || \
> -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> +		if ! virt_add ltp_v0 $opt; then
> +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> +			return

Same here.

> +		fi
>   		ROD_SILENT "ip link set ltp_v0 up"
>   		ROD_SILENT "ip link delete ltp_v0"
>   	done
> diff --git a/testcases/network/xinetd/xinetd_tests.sh b/testcases/network/xinetd/xinetd_tests.sh
> index 505dae5d7..25ec91d26 100755
> --- a/testcases/network/xinetd/xinetd_tests.sh
> +++ b/testcases/network/xinetd/xinetd_tests.sh
> @@ -91,9 +91,10 @@ xinetd_test()
>   
>   	for a in $check_addr; do
>   		p=$(echo $pattern | sed "s/ADDR/$a/")
> -		echo '' | telnet $a 2>&1 | grep -qiE "$p"
> -		[ $? -ne 0 ] && \
> -			tst_brk TFAIL "not expected output for 'telnet $a'"
> +		if ! echo '' | telnet $a 2>&1 | grep -qiE "$p"; then
> +			tst_res TFAIL "not expected output for 'telnet $a'"
> +			return
> +		fi
>   	done
>   	tst_res TPASS "expected output with telnet $desc"
>   }
Petr Vorel Jan. 24, 2024, 1:48 p.m. UTC | #2 
Hi Martin,

> Hi,
> two comments below.

...
> > +++ b/testcases/network/virt/virt_lib.sh
> > @@ -174,8 +174,10 @@ virt_multiple_add_test()
> >   	tst_res TINFO "add $NS_TIMES $virt_type, then delete"
> >   	for i in $(seq $start_id $max); do
> > -		virt_add ltp_v$i id $i $opt || \
> > -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> > +		if ! virt_add ltp_v$i id $i $opt; then
> > +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> > +			return

> This is a library function so tst_brk TBROK is probably the correct fix
> here. Also, the message should say ltp_v$i instead of ltp_v0, but that's for
> a separate patch.

Thanks! tst_brk TBROK makes sense here.

ltp_v$i fixed in separate commit with your credit (merged now).

> > +		fi
> >   		ROD_SILENT "ip link set ltp_v$i up"
> >   	done
> > @@ -196,8 +198,10 @@ virt_add_delete_test()
> >   	tst_res TINFO "add/del $virt_type $NS_TIMES times"
> >   	for i in $(seq 0 $max); do
> > -		virt_add ltp_v0 $opt || \
> > -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> > +		if ! virt_add ltp_v0 $opt; then
> > +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> > +			return

> Same here.

Yes (tst_brk TBROK).

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/network/nfs/nfs_stress/nfs06.sh b/testcases/network/nfs/nfs_stress/nfs06.sh
index 560df05b1..09e55fe34 100755
--- a/testcases/network/nfs/nfs_stress/nfs06.sh
+++ b/testcases/network/nfs/nfs_stress/nfs06.sh
@@ -34,7 +34,10 @@  do_test()
 
 	tst_res TINFO "waiting for pids:$pids"
 	for p in $pids; do
-		wait $p || tst_brk TFAIL "fsstress process failed"
+		if ! wait $p; then
+			tst_res TFAIL "fsstress process failed"
+			return
+		fi
 		tst_res TINFO "fsstress '$p' completed"
 	done
 	pids=
diff --git a/testcases/network/stress/route/route-lib.sh b/testcases/network/stress/route/route-lib.sh
index 163c15423..29aa2e913 100644
--- a/testcases/network/stress/route/route-lib.sh
+++ b/testcases/network/stress/route/route-lib.sh
@@ -97,10 +97,14 @@  test_netlink()
 	tst_res TINFO "running $cmd $opt"
 	$cmd $opt || ret=$?
 	if [ "$ret" -ne 0 ]; then
-		[ $((ret & 3)) -ne 0 ] && \
-			tst_brk TFAIL "$cmd failed"
+		if [ $((ret & 3)) -ne 0 ]; then
+			tst_res TFAIL "$cmd failed"
+			return
+		fi
+
 		[ $((ret & 32)) -ne 0 ] && \
 			tst_brk TCONF "not supported configuration"
+
 		[ $((ret & 4)) -ne 0 ] && \
 			tst_res TWARN "$cmd has warnings"
 	fi
diff --git a/testcases/network/stress/ssh/ssh-stress.sh b/testcases/network/stress/ssh/ssh-stress.sh
index e7c4d45ce..c27c27a28 100755
--- a/testcases/network/stress/ssh/ssh-stress.sh
+++ b/testcases/network/stress/ssh/ssh-stress.sh
@@ -93,8 +93,10 @@  IdentityFile $TST_TMPDIR/id_rsa\n\" > $RHOST_SSH_CONF"
 
 test_ssh_connectivity()
 {
-	tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"
-	[ $? -ne 0 ] && tst_brk TFAIL "SSH not reachable"
+	if ! tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"; then
+		tst_res TFAIL "SSH not reachable"
+		return
+	fi
 }
 
 test1()
@@ -121,7 +123,10 @@  test1()
 		[ $? -ne 0 ] && num=$((num + 1))
 	done
 
-	[ $num -ne 0 ] && tst_brk TFAIL "$num ssh processes died unexpectedly during execution"
+	if [ $num -ne 0 ]; then
+		tst_res TFAIL "$num ssh processes died unexpectedly during execution"
+		return
+	fi
 
 	test_ssh_connectivity
 
@@ -216,7 +221,10 @@  test3()
 
 	# Setup an ssh tunnel from the remote host to testhost
 	tst_rhost_run -c "$RHOST_SSH -f -N -L $lport:$rhost:$port </dev/null >/dev/null 2>&1"
-	[ "$?" -ne 0 ] && tst_brk TFAIL "Failed to create an SSH session with port forwarding"
+	if [ "$?" -ne 0 ]; then
+		tst_res TFAIL "Failed to create an SSH session with port forwarding"
+		return
+	fi
 	RHOST_PIDS=$(tst_rhost_run -c "pgrep -f '^ssh .*$lport:$rhost:$port'")
 
 	# Start the TCP traffic clients
diff --git a/testcases/network/tcp_cmds/host/host01.sh b/testcases/network/tcp_cmds/host/host01.sh
index 6a4067495..9ed44bae3 100755
--- a/testcases/network/tcp_cmds/host/host01.sh
+++ b/testcases/network/tcp_cmds/host/host01.sh
@@ -21,11 +21,12 @@  do_test()
 	if addr=$(host $lhost); then
 		addr=$(echo "$addr" | grep address | tail -1 | awk '{print $NF}')
 		if [ -z "$addr" ]; then
-			tst_brk TFAIL "empty address"
+			tst_res TFAIL "empty address"
+			return
 		fi
 		EXPECT_PASS host $addr \>/dev/null
 	else
-		tst_brk TFAIL "host $lhost on local machine failed"
+		tst_res TFAIL "host $lhost on local machine failed"
 	fi
 }
 
diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
index e67ff5cc8..4b818357e 100755
--- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
+++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
@@ -57,8 +57,10 @@  do_test()
 
 	for i in $(seq 1 $NUMLOOPS); do
 
-		ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null || \
-			tst_brk TFAIL "cannot ping $(tst_ipaddr rhost)"
+		if ! ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null; then
+			tst_res TFAIL "cannot ping $(tst_ipaddr rhost)"
+			return
+		fi
 
 		local k
 		local ret=1
@@ -66,19 +68,22 @@  do_test()
 			$SHOW_CMD | grep -q $(tst_ipaddr rhost)
 			if [ $? -eq 0 ]; then
 				ret=0
-				break;
+				break
 			fi
 			tst_sleep 100ms
 		done
 
-		[ "$ret" -ne 0 ] && \
-			tst_brk TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
+		if [ "$ret" -ne 0 ]; then
+			tst_res TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
+			return
+		fi
 
 		$DEL_CMD
 
-		$SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)" && \
-			tst_brk TFAIL "'$DEL_CMD' failed, entry has " \
-				"$(tst_hwaddr rhost)' $i/$NUMLOOPS"
+		if $SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)"; then
+			tst_res TFAIL "'$DEL_CMD' failed, entry has $(tst_hwaddr rhost)' $i/$NUMLOOPS"
+			return
+		fi
 	done
 
 	tst_res TPASS "verified adding/removing $entry_name cache entry"
diff --git a/testcases/network/virt/virt_lib.sh b/testcases/network/virt/virt_lib.sh
index d7e694eb7..0aadbfdb9 100644
--- a/testcases/network/virt/virt_lib.sh
+++ b/testcases/network/virt/virt_lib.sh
@@ -174,8 +174,10 @@  virt_multiple_add_test()
 	tst_res TINFO "add $NS_TIMES $virt_type, then delete"
 
 	for i in $(seq $start_id $max); do
-		virt_add ltp_v$i id $i $opt || \
-			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
+		if ! virt_add ltp_v$i id $i $opt; then
+			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
+			return
+		fi
 		ROD_SILENT "ip link set ltp_v$i up"
 	done
 
@@ -196,8 +198,10 @@  virt_add_delete_test()
 	tst_res TINFO "add/del $virt_type $NS_TIMES times"
 
 	for i in $(seq 0 $max); do
-		virt_add ltp_v0 $opt || \
-			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
+		if ! virt_add ltp_v0 $opt; then
+			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
+			return
+		fi
 		ROD_SILENT "ip link set ltp_v0 up"
 		ROD_SILENT "ip link delete ltp_v0"
 	done
diff --git a/testcases/network/xinetd/xinetd_tests.sh b/testcases/network/xinetd/xinetd_tests.sh
index 505dae5d7..25ec91d26 100755
--- a/testcases/network/xinetd/xinetd_tests.sh
+++ b/testcases/network/xinetd/xinetd_tests.sh
@@ -91,9 +91,10 @@  xinetd_test()
 
 	for a in $check_addr; do
 		p=$(echo $pattern | sed "s/ADDR/$a/")
-		echo '' | telnet $a 2>&1 | grep -qiE "$p"
-		[ $? -ne 0 ] && \
-			tst_brk TFAIL "not expected output for 'telnet $a'"
+		if ! echo '' | telnet $a 2>&1 | grep -qiE "$p"; then
+			tst_res TFAIL "not expected output for 'telnet $a'"
+			return
+		fi
 	done
 	tst_res TPASS "expected output with telnet $desc"
 }