Re: [PATCH mptcp-next v7 6/8] selftests: mptcp: add test counter helpers

[Matthieu Baerts <matttbe@kernel.org>]

On 29/02/2024 12:58, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 29/02/2024 10:51, Geliang Tang wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> This patch adds two vars in mptcp_lib.sh, MPTCP_LIB_TEST_COUNTER for
>> the test counter and MPTCP_LIB_TEST_FORMAT for the test print format.
>> Also add two helpers, mptcp_lib_inc_test_counter(), increase the test
>> counter, and mptcp_lib_pr_title_counter(), print the test title with
>> counter. They are used in mptcp_join.sh first.
> 
> Please add the reason, something like: Each MPTCP selftest is having
> subtests, and it helps to give them a number to quickly identify them.
> This can be managed by mptcp_lib.sh, reusing what has been done here.
> The following commit will use these new helpers in the other tests.
> 
>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>> ---

(...)

>> +mptcp_lib_pr_title_counter() {
>> +	: "${MPTCP_LIB_TEST_COUNTER:?}"
>> +	: "${MPTCP_LIB_TEST_FORMAT:="%02u %-50s"}"
>> +
>> +	printf "${MPTCP_LIB_TEST_FORMAT}" "${MPTCP_LIB_TEST_COUNTER}" "${*}"
> 
> I didn't check: can you not print the title and increment the counter
> (via mptcp_lib_inc_test_counter) from here? I think that would be a good
> practice to do that instead of having to deal with the counter in
> different places.
> 
> We would only call "mptcp_lib_inc_test_counter()" from other scripts
> when a test is skipped.

OK, I just saw you added mptcp_lib_print_title() in the next patch. Why
did you not use it for mptcp_join.sh?

Can you not call "mptcp_lib_inc_test_counter" in the "if skip_test" from
mptcp_join.sh and use mptcp_lib_print_title()? (if you do that, I guess
we don't need mptcp_lib_pr_title_counter(), no?)

> 
>> +}
> 
> Cheers,
> Matt

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