1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v6 0/9] add helpers and vars in mptcp_lib.sh, part 3
  4. View patch

[PATCH mptcp-next v6 1/9] selftests: mptcp: add mptcp_lib_pr_ok/skip/fail/info

Geliang Tang posted 9 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH mptcp-next v6 1/9] selftests: mptcp: add mptcp_lib_pr_ok/skip/fail/info
Posted by Geliang Tang 1 year, 11 months ago 
From: Geliang Tang <tanggeliang@kylinos.cn>

To unify the output formats of all test scripts, this patch adds
four more helpers:

	mptcp_lib_pr_ok()
	mptcp_lib_pr_skip()
	mptcp_lib_pr_fail()
	mptcp_lib_pr_info()

to print out [ OK ], [SKIP], [FAIL] and 'INFO:' with colors. Use
them in mptcp_join.sh and userspace_pm.sh first, and then use them
in all scripts in the next commit.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh |  6 ++---
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 23 +++++++++++++++++++
 .../selftests/net/mptcp/userspace_pm.sh       | 13 ++++-------
 3 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 1df2d24979a0..53d51a5f68f9 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -190,17 +190,17 @@ print_info()
 
 print_ok()
 {
-	mptcp_lib_print_ok "[ ok ]${1:+ ${*}}"
+	mptcp_lib_pr_ok "${@}"
 }
 
 print_fail()
 {
-	mptcp_lib_print_err "[fail]${1:+ ${*}}"
+	mptcp_lib_pr_fail "${@}"
 }
 
 print_skip()
 {
-	mptcp_lib_print_warn "[skip]${1:+ ${*}}"
+	mptcp_lib_pr_skip "${@}"
 }
 
 # [ $1: fail msg ]
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 763a2989ca6d..3f5b16fe5220 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -48,6 +48,29 @@ mptcp_lib_print_err() {
 	mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
 }
 
+mptcp_lib_pr_ok() {
+	mptcp_lib_print_ok "[ OK ]"
+}
+
+mptcp_lib_pr_skip() {
+	local msg="[SKIP]"
+
+	mptcp_lib_print_warn "${msg}"
+}
+
+mptcp_lib_pr_fail() {
+	local msg="[FAIL]"
+
+	if [ "${#}" -gt 0 ]; then
+		msg+="${1:+ ${*}}"
+	fi
+	mptcp_lib_print_err "${msg}"
+}
+
+mptcp_lib_pr_info() {
+	mptcp_lib_print_info "INFO:${1:+ ${*}}"
+}
+
 # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when validating all
 # features using the last version of the kernel and the selftests to make sure
 # a test is not being skipped by mistake.
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index b0cce8f065d8..5f32ac451596 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -61,7 +61,7 @@ _printf() {
 
 print_title()
 {
-	_printf "INFO: %s\n" "${1}"
+	mptcp_lib_print_info "INFO: ${1}"
 }
 
 # $1: test name
@@ -72,27 +72,22 @@ print_test()
 	_printf "%-68s" "${test_name}"
 }
 
-print_results()
-{
-	_printf "[%s]\n" "${1}"
-}
-
 test_pass()
 {
-	print_results " OK "
+	mptcp_lib_pr_ok
 	mptcp_lib_result_pass "${test_name}"
 }
 
 test_skip()
 {
-	print_results "SKIP"
+	mptcp_lib_pr_skip
 	mptcp_lib_result_skip "${test_name}"
 }
 
 # $1: msg
 test_fail()
 {
-	print_results "FAIL"
+	mptcp_lib_pr_fail "${1:+ ${*}}"
 	ret=1
 
 	if [ -n "${1}" ]; then
-- 
2.40.1
Re: [PATCH mptcp-next v6 1/9] selftests: mptcp: add mptcp_lib_pr_ok/skip/fail/info
Posted by Matthieu Baerts 1 year, 11 months ago 
Hi Geliang,

On 28/02/2024 08:43, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To unify the output formats of all test scripts, this patch adds
> four more helpers:
> 
> 	mptcp_lib_pr_ok()
> 	mptcp_lib_pr_skip()
> 	mptcp_lib_pr_fail()
> 	mptcp_lib_pr_info()
> 
> to print out [ OK ], [SKIP], [FAIL] and 'INFO:' with colors. Use
> them in mptcp_join.sh and userspace_pm.sh first, and then use them
> in all scripts in the next commit.

Please add: please note that the mptcp_join.sh tests will now print
these keywords with capital letters, like most of the other tests.

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh |  6 ++---
>  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 23 +++++++++++++++++++
>  .../selftests/net/mptcp/userspace_pm.sh       | 13 ++++-------
>  3 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 1df2d24979a0..53d51a5f68f9 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -190,17 +190,17 @@ print_info()
>  
>  print_ok()
>  {
> -	mptcp_lib_print_ok "[ ok ]${1:+ ${*}}"
> +	mptcp_lib_pr_ok "${@}"
>  }
>  
>  print_fail()
>  {
> -	mptcp_lib_print_err "[fail]${1:+ ${*}}"
> +	mptcp_lib_pr_fail "${@}"
>  }
>  
>  print_skip()
>  {
> -	mptcp_lib_print_warn "[skip]${1:+ ${*}}"
> +	mptcp_lib_pr_skip "${@}"
>  }
>  
>  # [ $1: fail msg ]
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 763a2989ca6d..3f5b16fe5220 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -48,6 +48,29 @@ mptcp_lib_print_err() {
>  	mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
>  }
>  
> +mptcp_lib_pr_ok() {
> +	mptcp_lib_print_ok "[ OK ]"

Here, you will potentially have more info to print (at least from
mptcp_join.sh, it can be called with extra args to print):

  mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"


> +}
> +
> +mptcp_lib_pr_skip() {
> +	local msg="[SKIP]"
> +
> +	mptcp_lib_print_warn "${msg}"

Same here:

  mptcp_lib_print_warn "[SKIP]${1:+ ${*}}"

> +}
> +
> +mptcp_lib_pr_fail() {
> +	local msg="[FAIL]"
> +
> +	if [ "${#}" -gt 0 ]; then
> +		msg+="${1:+ ${*}}"
> +	fi
> +	mptcp_lib_print_err "${msg}"

And here, it can be simplified:

  mptcp_lib_print_err "[FAIL]${1:+ ${*}}"

> +}
> +
> +mptcp_lib_pr_info() {
> +	mptcp_lib_print_info "INFO:${1:+ ${*}}"

I guess here, we will always have arguments, no?

  mptcp_lib_print_info "INFO: ${*}"

> +}
> +
>  # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when validating all
>  # features using the last version of the kernel and the selftests to make sure
>  # a test is not being skipped by mistake.
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index b0cce8f065d8..5f32ac451596 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -61,7 +61,7 @@ _printf() {
>  
>  print_title()
>  {
> -	_printf "INFO: %s\n" "${1}"
> +	mptcp_lib_print_info "INFO: ${1}"

Use mptcp_lib_pr_info() directly, no?

>  }
>  
>  # $1: test name
> @@ -72,27 +72,22 @@ print_test()
>  	_printf "%-68s" "${test_name}"
>  }
>  
> -print_results()
> -{
> -	_printf "[%s]\n" "${1}"
> -}
> -
>  test_pass()
>  {
> -	print_results " OK "
> +	mptcp_lib_pr_ok
>  	mptcp_lib_result_pass "${test_name}"
>  }
>  
>  test_skip()
>  {
> -	print_results "SKIP"
> +	mptcp_lib_pr_skip
>  	mptcp_lib_result_skip "${test_name}"
>  }
>  
>  # $1: msg
>  test_fail()
>  {
> -	print_results "FAIL"
> +	mptcp_lib_pr_fail "${1:+ ${*}}"

I guess you can pass "${@}", no?

  mptcp_lib_pr_fail "${@}"

>  	ret=1
>  
>  	if [ -n "${1}" ]; then

Here, I guess you don't need the 'if' to print more info, there will be
printed with 'mptcp_lib_pr_fail "${@}"' just before, no?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v6 1/9] selftests: mptcp: add mptcp_lib_pr_ok/skip/fail/info
Posted by Geliang Tang 1 year, 11 months ago 
Hi Matt,

On Wed, 2024-02-28 at 11:31 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 28/02/2024 08:43, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > To unify the output formats of all test scripts, this patch adds
> > four more helpers:
> > 
> > 	mptcp_lib_pr_ok()
> > 	mptcp_lib_pr_skip()
> > 	mptcp_lib_pr_fail()
> > 	mptcp_lib_pr_info()
> > 
> > to print out [ OK ], [SKIP], [FAIL] and 'INFO:' with colors. Use
> > them in mptcp_join.sh and userspace_pm.sh first, and then use them
> > in all scripts in the next commit.
> 
> Please add: please note that the mptcp_join.sh tests will now print
> these keywords with capital letters, like most of the other tests.
> 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  .../testing/selftests/net/mptcp/mptcp_join.sh |  6 ++---
> >  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 23
> > +++++++++++++++++++
> >  .../selftests/net/mptcp/userspace_pm.sh       | 13 ++++-------
> >  3 files changed, 30 insertions(+), 12 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index 1df2d24979a0..53d51a5f68f9 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -190,17 +190,17 @@ print_info()
> >  
> >  print_ok()
> >  {
> > -	mptcp_lib_print_ok "[ ok ]${1:+ ${*}}"
> > +	mptcp_lib_pr_ok "${@}"
> >  }
> >  
> >  print_fail()
> >  {
> > -	mptcp_lib_print_err "[fail]${1:+ ${*}}"
> > +	mptcp_lib_pr_fail "${@}"
> >  }
> >  
> >  print_skip()
> >  {
> > -	mptcp_lib_print_warn "[skip]${1:+ ${*}}"
> > +	mptcp_lib_pr_skip "${@}"
> >  }
> >  
> >  # [ $1: fail msg ]
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > index 763a2989ca6d..3f5b16fe5220 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > @@ -48,6 +48,29 @@ mptcp_lib_print_err() {
> >  	mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
> >  }
> >  
> > +mptcp_lib_pr_ok() {
> > +	mptcp_lib_print_ok "[ OK ]"
> 
> Here, you will potentially have more info to print (at least from
> mptcp_join.sh, it can be called with extra args to print):
> 
>   mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"

I have to use '[ OK ]' here, shellcheck complains about '[ OK ]${1:+
${*}}':

mptcp_lib_pr_ok
                ^-------------^ SC2119 (info): Use mptcp_lib_pr_ok "$@"
if function's $1 should mean script's $1.

Thanks,
-Geliang

> 
> 
> > +}
> > +
> > +mptcp_lib_pr_skip() {
> > +	local msg="[SKIP]"
> > +
> > +	mptcp_lib_print_warn "${msg}"
> 
> Same here:
> 
>   mptcp_lib_print_warn "[SKIP]${1:+ ${*}}"
> 
> > +}
> > +
> > +mptcp_lib_pr_fail() {
> > +	local msg="[FAIL]"
> > +
> > +	if [ "${#}" -gt 0 ]; then
> > +		msg+="${1:+ ${*}}"
> > +	fi
> > +	mptcp_lib_print_err "${msg}"
> 
> And here, it can be simplified:
> 
>   mptcp_lib_print_err "[FAIL]${1:+ ${*}}"
> 
> > +}
> > +
> > +mptcp_lib_pr_info() {
> > +	mptcp_lib_print_info "INFO:${1:+ ${*}}"
> 
> I guess here, we will always have arguments, no?
> 
>   mptcp_lib_print_info "INFO: ${*}"
> 
> > +}
> > +
> >  # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when
> > validating all
> >  # features using the last version of the kernel and the selftests
> > to make sure
> >  # a test is not being skipped by mistake.
> > diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > index b0cce8f065d8..5f32ac451596 100755
> > --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > @@ -61,7 +61,7 @@ _printf() {
> >  
> >  print_title()
> >  {
> > -	_printf "INFO: %s\n" "${1}"
> > +	mptcp_lib_print_info "INFO: ${1}"
> 
> Use mptcp_lib_pr_info() directly, no?
> 
> >  }
> >  
> >  # $1: test name
> > @@ -72,27 +72,22 @@ print_test()
> >  	_printf "%-68s" "${test_name}"
> >  }
> >  
> > -print_results()
> > -{
> > -	_printf "[%s]\n" "${1}"
> > -}
> > -
> >  test_pass()
> >  {
> > -	print_results " OK "
> > +	mptcp_lib_pr_ok
> >  	mptcp_lib_result_pass "${test_name}"
> >  }
> >  
> >  test_skip()
> >  {
> > -	print_results "SKIP"
> > +	mptcp_lib_pr_skip
> >  	mptcp_lib_result_skip "${test_name}"
> >  }
> >  
> >  # $1: msg
> >  test_fail()
> >  {
> > -	print_results "FAIL"
> > +	mptcp_lib_pr_fail "${1:+ ${*}}"
> 
> I guess you can pass "${@}", no?
> 
>   mptcp_lib_pr_fail "${@}"
> 
> >  	ret=1
> >  
> >  	if [ -n "${1}" ]; then
> 
> Here, I guess you don't need the 'if' to print more info, there will
> be
> printed with 'mptcp_lib_pr_fail "${@}"' just before, no?
> 
> Cheers,
> Matt
Re: [PATCH mptcp-next v6 1/9] selftests: mptcp: add mptcp_lib_pr_ok/skip/fail/info
Posted by Matthieu Baerts 1 year, 11 months ago 
Hi Geliang,

On 29/02/2024 10:18, Geliang Tang wrote:
> Hi Matt,
> 
> On Wed, 2024-02-28 at 11:31 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 28/02/2024 08:43, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> To unify the output formats of all test scripts, this patch adds
>>> four more helpers:
>>>
>>> 	mptcp_lib_pr_ok()
>>> 	mptcp_lib_pr_skip()
>>> 	mptcp_lib_pr_fail()
>>> 	mptcp_lib_pr_info()
>>>
>>> to print out [ OK ], [SKIP], [FAIL] and 'INFO:' with colors. Use
>>> them in mptcp_join.sh and userspace_pm.sh first, and then use them
>>> in all scripts in the next commit.

(...)

>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> index 763a2989ca6d..3f5b16fe5220 100644
>>> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> @@ -48,6 +48,29 @@ mptcp_lib_print_err() {
>>>  	mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
>>>  }
>>>  
>>> +mptcp_lib_pr_ok() {
>>> +	mptcp_lib_print_ok "[ OK ]"
>>
>> Here, you will potentially have more info to print (at least from
>> mptcp_join.sh, it can be called with extra args to print):
>>
>>   mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
> 
> I have to use '[ OK ]' here, shellcheck complains about '[ OK ]${1:+
> ${*}}':
> 
> mptcp_lib_pr_ok
>                 ^-------------^ SC2119 (info): Use mptcp_lib_pr_ok "$@"
> if function's $1 should mean script's $1.
Interesting, that's the first time I see that. It looks like you have
this warning, in other files, when mptcp_lib_pr_ok() is called without
parameters, from a function taking some, no?

I guess we can ignore this, explicitly marking this function as "having
optional parameters", using "shellcheck disable=SC2120":

e.g.


> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 4f6bbf5711b17..81ac18841df10 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -48,30 +48,23 @@ mptcp_lib_print_err() {
>         mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
>  }
>  
> +# shellcheck disable=SC2120  # parameters are optional
>  mptcp_lib_pr_ok() {
> -       mptcp_lib_print_ok "[ OK ]"
> +       mptcp_lib_print_ok "[ OK ]${1:+" ${*}"}"
>  }
>  
> +# shellcheck disable=SC2120  # parameters are optional
>  mptcp_lib_pr_skip() {
> -       local msg="[SKIP]"
> -
> -       if [ "${#}" -gt 0 ]; then
> -               msg+="${1:+ ${*}}"
> -       fi
> -       mptcp_lib_print_warn "${msg}"
> +       mptcp_lib_print_warn "[SKIP]${1:+ ${*}}"
>  }
>  
> +# shellcheck disable=SC2120  # parameters are optional
>  mptcp_lib_pr_fail() {
> -       local msg="[FAIL]"
> -
> -       if [ "${#}" -gt 0 ]; then
> -               msg+="${1:+ ${*}}"
> -       fi
> -       mptcp_lib_print_err "${msg}"
> +       mptcp_lib_print_err "[FAIL]${1:+ ${*}}"
>  }
>  
>  mptcp_lib_pr_info() {
> -       mptcp_lib_print_info "INFO:${1:+ ${*}}"
> +       mptcp_lib_print_info "INFO: ${*}"
>  }
>  
>  # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when validating al


WDYT?


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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.