Re: [PATCH mptcp-next 1/3] selftests: mptcp: pm nl: also list skipped tests

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

Thank you for the review!

On 13/02/2024 14:35, Geliang Tang wrote:
> Hi Matt,
> 
> Matthieu Baerts (NGI0) <matttbe@kernel.org> 于2024年2月8日周四 02:18写道：
>>
>> If the feature is not supported by older kernels, and instead of just
>> ignoring some tests, we should mark them as skipped, so we can still
>> track them.
>>
>> Fixes: d85555ac11f9 ("selftests: mptcp: pm_netlink: format subtests results in TAP")
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  tools/testing/selftests/net/mptcp/pm_netlink.sh | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
>> index 8f4ff123a7eb..79e83a2c95de 100755
>> --- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
>> +++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
>> @@ -194,6 +194,12 @@ subflow 10.0.1.1" "          (nofullmesh)"
>>         ip netns exec $ns1 ./pm_nl_ctl set id 1 flags backup,fullmesh
>>         check "ip netns exec $ns1 ./pm_nl_ctl dump" "id 1 flags \
>>  subflow,backup,fullmesh 10.0.1.1" "          (backup,fullmesh)"
>> +else
>> +       for st in fullmesh nofullmesh backup,fullmesh; do
>> +               st="          (${st})"
>> +               printf "%-50s%s\n" "${st}" "[SKIP]"
> 
> I prefer to use "[ SKIP ]" here.

I would prefer too, but in this file, we are currently using:

  [ OK ]
  [FAIL]

I think it is then better to continue using 4 letters between the [], no?

  [ OK ]
  [FAIL]
  [SKIP]

If we want to change the format, and even add colours, we can do that in
net-next, not as part of a fix for -net.

> Otherwise, this series looks good!
>
> Reviewed-by: Geliang Tang <geliang@kernel.org>

If you don't mind, when a whole series is OK for you, can you share your
"Reviewed-By" tag on the cover-letter please? By doing that, your tag
will be "propagated" to each patch. While here, 'b4' will see it as only
valid for patch 1/3, but not the rest of the series.

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