Re: [PATCH mptcp-net 1/5] selftests: mptcp: simult flows: fix some subtest names

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

On Tue, Feb 13, 2024 at 01:06:40PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for the review!
> 
> On 13/02/2024 12:34, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Fri, Feb 09, 2024 at 06:28:39PM +0100, Matthieu Baerts (NGI0) wrote:
> >> The selftest was correctly recording all the results, but the 'reverse
> >> direction' part was missing in the name when needed.
> >>
> >> It is important to have a unique (sub)test name in TAP, because some CI
> >> environments drop tests with duplicated name.
> >>
> >> Fixes: 675d99338e7a ("selftests: mptcp: simult flows: format subtests results in TAP")
> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> >> ---
> >>  tools/testing/selftests/net/mptcp/simult_flows.sh | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> >> index 619be0e1acf5..f377ef01970b 100755
> >> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> >> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> >> @@ -250,7 +250,8 @@ run_test()
> >>  		[ $bail -eq 0 ] || exit $ret
> >>  	fi
> >>  
> >> -	printf "%-60s" "$msg - reverse direction"
> >> +	msg="${msg} - reverse direction"
> > 
> > How about using '+=' operator to append a string like:
> > 
> > 	msg+=" - reverse direction"
> > 
> > The same in patch 3 and patch 4:
> > 
> > 	msg+=" after flush"
> > 
> > WDYT?
> 
> I usually prefer not to do that, because that's Bash specific, and it
> doesn't work with 'sh'.
> 
> Here, we explicitly use '/bin/bash', so nothing is blocking me, just an
> habit when often switching between "bash" and "sh" :)
> I can do the modification if you prefer.

Both are fine to me. You can modify them or not when merging them. All
tests passed on my side.

Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

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