[PATCH mptcp-next v5 08/12] selftests: mptcp: move test_fail out of check_expected_one

Geliang Tang posted 12 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH mptcp-next v5 08/12] selftests: mptcp: move test_fail out of check_expected_one
Posted by Geliang Tang 1 year, 11 months ago
From: Geliang Tang <tanggeliang@kylinos.cn>

This patch moves test_fail out of check_expected_one(), since test_fail
is a private function in userspace_pm.sh, and check_expected_one will be
exported in mptcp_lib.sh in the next commit.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/userspace_pm.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 27f308601005..473639a8009f 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -247,7 +247,7 @@ check_expected_one()
 
 	if [ "${prev_ret}" = "0" ]
 	then
-		test_fail
+		return 2
 	fi
 
 	_printf "\tExpected value for '%s': '%s', got '%s'.\n" \
@@ -263,13 +263,20 @@ check_expected()
 
 	for var in "${@}"
 	do
-		check_expected_one "${var}" "${rc}" || rc=1
+		check_expected_one "${var}" "${rc}" || rc="${?}"
+		if [ "${rc}" -eq 2 ]
+		then
+			break
+		fi
 	done
 
 	if [ ${rc} -eq 0 ]
 	then
 		test_pass
 		return 0
+	elif [ "${rc}" -eq 2 ]
+	then
+		test_fail
 	fi
 
 	return 1
-- 
2.40.1
Re: [PATCH mptcp-next v5 08/12] selftests: mptcp: move test_fail out of check_expected_one
Posted by Matthieu Baerts 1 year, 11 months ago
Hi Geliang,

On 26/02/2024 10:43, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch moves test_fail out of check_expected_one(), since test_fail
> is a private function in userspace_pm.sh, and check_expected_one will be
> exported in mptcp_lib.sh in the next commit.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/userspace_pm.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index 27f308601005..473639a8009f 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -247,7 +247,7 @@ check_expected_one()
>  
>  	if [ "${prev_ret}" = "0" ]
>  	then
> -		test_fail

I think, the goal to do it here was to print "FAIL", then the details.

> +		return 2

Just return 1, no?

>  	fi
>  
>  	_printf "\tExpected value for '%s': '%s', got '%s'.\n" \

What's the output now in case of issue? Does it still make sense to use
the '\t' here at the beginning?

> @@ -263,13 +263,20 @@ check_expected()
>  
>  	for var in "${@}"
>  	do
> -		check_expected_one "${var}" "${rc}" || rc=1
> +		check_expected_one "${var}" "${rc}" || rc="${?}"
> +		if [ "${rc}" -eq 2 ]
> +		then
> +			break

Why breaking? If you break, you only print one error, not all like before.

Please check the behaviour in case of error before and after the
modification.

> +		fi
>  	done
>  
>  	if [ ${rc} -eq 0 ]
>  	then
>  		test_pass
>  		return 0
> +	elif [ "${rc}" -eq 2 ]
> +	then
> +		test_fail

Just 'test_fail' after the 'if'.

>  	fi
>  
>  	return 1

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.