Re: [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

On Fri, 6 Jan 2023, Matthieu Baerts wrote:

> Hi Mat,
>
> Thank you for the review!
>
> 6 Jan 2023 02:19:26 Mat Martineau <mathew.j.martineau@linux.intel.com>:
>
>> On Wed, 4 Jan 2023, Matthieu Baerts wrote:
>>
>>> This script is running a few tests after having setup the environment.
>>>
>>> Printing titles helps understand what is being tested.
>>>
>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>> ---
>>>
>>> Notes:
>>>    v3->v4:
>>>      - Add a title for the Cleanup (+ 'Done' at the end)
>>>
>>> .../selftests/net/mptcp/userspace_pm.sh       | 24 ++++++++++++++++++-
>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> index ab2d581f28a1..f9a03e6e968f 100755
>>> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> @@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
>>> ns1="ns1-$rndh"
>>> ns2="ns2-$rndh"
>>>
>>> +print_title()
>>> +{
>>> +   stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
>>
>> The indented titles seem kind of inverted to me:
>>
>> """
>>     Init:
>> Created network namespaces ns1, ns2                     [OK]
>>     Make connections:
>> Established IPv4 MPTCP Connection ns2 => ns1            [OK]
>> Established IPv6 MPTCP Connection ns2 => ns1            [OK]
>>     Announce tests:
>> ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token           [OK]
>> ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port        [OK]
>> ...
>> """
>>
>> It would read better to have the titles left-justified and the test results indented, but that makes all of the test results extra long. Seems like you are trying to avoid that (for good reason!).
>
> Indeed :)
>
>> What do you think about copying the "INFO: ..." style from mptcp_connect.sh?
>
> Good idea, that works for me!
>
> I can do the modification (s/\t/INFO: /) when applying the patches if that's OK for you (if there is nothing else to change of course).
>

Sure, that's fine with me. For patches 9-12:

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel