No more tests were executed after a failure but it is still interesting
to get results for all the tests to better understand what's still OK
and what's not after a modification.

Now we only exit earlier if the basic tests are failing: no ping going
through namespaces or unable to transfer data on the loopback interface.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 27 ++++++++++++++++------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 780b8fef2261..dba58875ef4c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -7,6 +7,7 @@ time_start=$(date +%s)
 
 optstring="S:R:d:e:l:r:h4cm:f:tC"
 ret=0
+final_ret=0
 sin=""
 sout=""
 cin_disconnect=""
@@ -837,14 +838,25 @@ display_time()
 	echo "Time: ${time_run} seconds"
 }
 
-stop_if_error()
+log_if_error()
 {
 	local msg="$1"
 
 	if [ ${ret} -ne 0 ]; then
 		echo "FAIL: ${msg}" 1>&2
+
+		final_ret=${ret}
+		ret=0
+
+		return ${final_ret}
+	fi
+}
+
+stop_if_error()
+{
+	if ! log_if_error "${@}"; then
 		display_time
-		exit ${ret}
+		exit ${final_ret}
 	fi
 }
 
@@ -934,23 +946,24 @@ for sender in $ns1 $ns2 $ns3 $ns4;do
 	run_tests "$ns4" $sender 10.0.3.1
 	run_tests "$ns4" $sender dead:beef:3::1
 
-	stop_if_error "Tests with $sender as a sender have failed"
+	log_if_error "Tests with $sender as a sender have failed"
 done
 
 run_tests_peekmode "saveWithPeek"
 run_tests_peekmode "saveAfterPeek"
-stop_if_error "Tests with peek mode have failed"
+log_if_error "Tests with peek mode have failed"
 
 # MPTFO (MultiPath TCP Fatopen tests)
 run_tests_mptfo
-stop_if_error "Tests with MPTFO have failed"
+log_if_error "Tests with MPTFO have failed"
 
 # connect to ns4 ip address, ns2 should intercept/proxy
 run_test_transparent 10.0.3.1 "tproxy ipv4"
 run_test_transparent dead:beef:3::1 "tproxy ipv6"
-stop_if_error "Tests with tproxy have failed"
+log_if_error "Tests with tproxy have failed"
 
 run_tests_disconnect
+log_if_error "Tests of the full disconnection have failed"
 
 display_time
-exit $ret
+exit ${final_ret}

-- 
2.40.1

browsers! ;)
On Wed, 2023-06-21 at 19:18 +0200, Matthieu Baerts wrote:
> No more tests were executed after a failure but it is still interesting
> to get results for all the tests to better understand what's still OK
> and what's not after a modification.
> 
> Now we only exit earlier if the basic tests are failing: no ping going
> through namespaces or unable to transfer data on the loopback interface.
> 
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 27 ++++++++++++++++------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 780b8fef2261..dba58875ef4c 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -7,6 +7,7 @@ time_start=$(date +%s)
>  
>  optstring="S:R:d:e:l:r:h4cm:f:tC"
>  ret=0
> +final_ret=0
>  sin=""
>  sout=""
>  cin_disconnect=""
> @@ -837,14 +838,25 @@ display_time()
>  	echo "Time: ${time_run} seconds"
>  }
>  
> -stop_if_error()
> +log_if_error()
>  {
>  	local msg="$1"
>  
>  	if [ ${ret} -ne 0 ]; then
>  		echo "FAIL: ${msg}" 1>&2
> +
> +		final_ret=${ret}
> +		ret=0
> +
> +		return ${final_ret}
> +	fi

It's not clear to me which is the return value for log_if_error() in
the '${ret} -eq 0' case.

What about moving the return after the 'if' statement?

Cheers,

Paolo

On 30/06/2023 12:52, Paolo Abeni wrote:
> browsers! ;)
> On Wed, 2023-06-21 at 19:18 +0200, Matthieu Baerts wrote:
>> No more tests were executed after a failure but it is still interesting
>> to get results for all the tests to better understand what's still OK
>> and what's not after a modification.
>>
>> Now we only exit earlier if the basic tests are failing: no ping going
>> through namespaces or unable to transfer data on the loopback interface.
>>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 27 ++++++++++++++++------
>>  1 file changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>> index 780b8fef2261..dba58875ef4c 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>> @@ -7,6 +7,7 @@ time_start=$(date +%s)
>>  
>>  optstring="S:R:d:e:l:r:h4cm:f:tC"
>>  ret=0
>> +final_ret=0
>>  sin=""
>>  sout=""
>>  cin_disconnect=""
>> @@ -837,14 +838,25 @@ display_time()
>>  	echo "Time: ${time_run} seconds"
>>  }
>>  
>> -stop_if_error()
>> +log_if_error()
>>  {
>>  	local msg="$1"
>>  
>>  	if [ ${ret} -ne 0 ]; then
>>  		echo "FAIL: ${msg}" 1>&2
>> +
>> +		final_ret=${ret}
>> +		ret=0
>> +
>> +		return ${final_ret}
>> +	fi
> 
> It's not clear to me which is the return value for log_if_error() in
> the '${ret} -eq 0' case.

It will be "0" because there was no command launched in this case.

> What about moving the return after the 'if' statement?

I don't think I can do that now that we have "ret" and "final_ret":
- "ret" is reset after each section when calling log_if_error(): it is
the result per section.
- "final_ret" is one we will use for the final exit: it is set to 0 at
the beginning and it can only be set to a non zero value later in case
of error. It represent the global result.

So here, in case of error, I cannot return "ret" because it is reset to
0 and if there was no error, I cannot return "final_ret" because it will
be set to 1 if there was an error in the previous section.

But I can add an explicit "return 0" if there is no error:

  if [ ${ret} -eq 0 ]; then
  	return 0

  echo "FAIL: ${msg}" 1>&2

  final_ret=${ret}
  ret=0

  return ${final_ret}

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

On Fri, 2023-06-30 at 16:41 +0200, Matthieu Baerts wrote:
> On 30/06/2023 12:52, Paolo Abeni wrote:
> > browsers! ;)
> > On Wed, 2023-06-21 at 19:18 +0200, Matthieu Baerts wrote:
> > > No more tests were executed after a failure but it is still interesting
> > > to get results for all the tests to better understand what's still OK
> > > and what's not after a modification.
> > > 
> > > Now we only exit earlier if the basic tests are failing: no ping going
> > > through namespaces or unable to transfer data on the loopback interface.
> > > 
> > > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> > > ---
> > >  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 27 ++++++++++++++++------
> > >  1 file changed, 20 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> > > index 780b8fef2261..dba58875ef4c 100755
> > > --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> > > +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> > > @@ -7,6 +7,7 @@ time_start=$(date +%s)
> > >  
> > >  optstring="S:R:d:e:l:r:h4cm:f:tC"
> > >  ret=0
> > > +final_ret=0
> > >  sin=""
> > >  sout=""
> > >  cin_disconnect=""
> > > @@ -837,14 +838,25 @@ display_time()
> > >  	echo "Time: ${time_run} seconds"
> > >  }
> > >  
> > > -stop_if_error()
> > > +log_if_error()
> > >  {
> > >  	local msg="$1"
> > >  
> > >  	if [ ${ret} -ne 0 ]; then
> > >  		echo "FAIL: ${msg}" 1>&2
> > > +
> > > +		final_ret=${ret}
> > > +		ret=0
> > > +
> > > +		return ${final_ret}
> > > +	fi
> > 
> > It's not clear to me which is the return value for log_if_error() in
> > the '${ret} -eq 0' case.
> 
> It will be "0" because there was no command launched in this case.
> 
> > What about moving the return after the 'if' statement?
> 
> I don't think I can do that now that we have "ret" and "final_ret":
> - "ret" is reset after each section when calling log_if_error(): it is
> the result per section.
> - "final_ret" is one we will use for the final exit: it is set to 0 at
> the beginning and it can only be set to a non zero value later in case
> of error. It represent the global result.
> 
> So here, in case of error, I cannot return "ret" because it is reset to
> 0 and if there was no error, I cannot return "final_ret" because it will
> be set to 1 if there was an error in the previous section.
> 
> But I can add an explicit "return 0" if there is no error:
> 
>   if [ ${ret} -eq 0 ]; then
>   	return 0
> 
>   echo "FAIL: ${msg}" 1>&2
> 
>   final_ret=${ret}
>   ret=0
> 
>   return ${final_ret}

LGTM, thanks!

/P