Re: [PATCH mptcp-next v7 4/8] selftests: mptcp: simult_flows: fix misaligned output

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 29/02/2024 13:01, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, Feb 29, 2024 at 11:27:29AM +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 29/02/2024 11:07, Geliang Tang wrote:
>>> On Thu, Feb 29, 2024 at 05:51:07PM +0800, Geliang Tang wrote:
>>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>>
>>>> The last line in the output of simult_flows.sh misaligns with the other
>>>> lines, and all lines are over maximum 75 chars per line.
>>>>
>>>> This patch aligns them by using the newly added print_title() helper,
>>>> and truncate them within 75 chars per line to allow it to be displayed
>>>> normally in regular terminals without breaking lines.
>>>
>>> This part of commit log needs to be updated:
>>>
>>>     This patch aligns them by reducing the number of spaces before [ OK ] to
>>>     1, and truncate them within 75 chars per line to allow it to be displayed
>>>     normally in regular terminals without breaking lines.
>>>
>>> Thanks,
>>> -Geliang
>>
>> I don't know if the 2nd part of my previous comment was clear:
>>
>>> But also, why not having a wider output instead? Why the 75 chars limit?
> 
> "... to allow it to be displayed normally in regular terminals (80 chars)
> without breaking lines." 2 chars is reserved for "# " added at the beginning
> of each line when running tests in virtme-docker. So I chose 75 chars.

Ah OK, I didn't get that "regular terminals are 80 chars wide". To be
honest, I don't know if this is still true. I mean: I never use a
terminal that is only 80 chars wide, at least 100/120. That's why I was
suggesting to show the full info, instead of trunking it.

>>> I mean: yes, the test names should not be too long. But here, probably
>>> too late to change, only modify (increase) the value in print_title(), no?
> 
> I thinks it's not too late, it's the right time to change this, since
> printing title with test counter increases 3 chars for each line,
> becoming much longer. It makes sense to reduce it this time.

But are we not loosing info? e.g. the last one is already:

  "unbalanced bwidth with opposed, unbalanced delay - rev"

If you remove the last 3 chars for the counter, we don't know the
difference with the previous test.

>> Why having this limit of 75 char? In other words, why not printing a
>> larger title, e.g. 90 chars or more if needed?
>>
>>   print_title() {
>>       printf "%-90s" "${1}"
>>   }
> 
> And the lines is too long already, so it makes sense to reduce it, not
> increase it, right?
> 
> I thinks another option is too keep the length unchanged, not reducing
> it, nor increasing it too.

Yes, maybe better, not to "lose" bits. So increasing the title to be
able to align on the last one, and reducing the "${maxtime}" to save a
bit of space?

About the "maxtime", be careful that "selftests: mptcp: simult flows:
re-adapt BW" patch is "blocked" for the moment. So in net and net-next,
we have something like:

  7384 max 7906
  11345 max 11921

(that's 2 more chars than the current version in our tree)

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