mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
@ 2026-07-14  6:34 QianhengPeng
  2026-09-01  2:44 ` Qianheng Peng
  0 siblings, 1 reply; 8+ messages in thread
From: QianhengPeng @ 2026-07-14  6:34 UTC (permalink / raw)
  To: shuah, broonie, jackmanb, liuhangbin, rbm
  Cc: skhan, linux-kselftest, linux-kernel, pengqh1

Normally the value of kselftest_cmd_args can be read from settings file,
but it will be overridden by KSELFTEST_TEST_KMOD_SH_ARGS which can be
empty if user did not give it a real value.So it should be better to
check if KSELFTEST_TEST_KMOD_SH_ARGS is emty before referring its
value to kselftest_cmd_args.

Signed-off-by: QianhengPeng <pengqh1@chinatelecom.cn>
---
 tools/testing/selftests/kselftest/runner.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 311811d..a99147a 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -103,6 +103,15 @@ run_one()
 		ktap_print_msg "timeout set to $kselftest_timeout" >> "$logfile"
 	fi
 
+	# Exported environment variable overrides the settings file
+	eval kselftest_eval_cmd_args="\$${kselftest_cmd_args_ref:-}"
+	if [ -n "$kselftest_eval_cmd_args" ]; then
+		kselftest_cmd_args=$kselftest_eval_cmd_args
+		ktap_print_msg "overriding cmd_args to $kselftest_cmd_args" >> "$logfile"
+	elif [ -n "$kselftest_cmd_args" ]; then
+		ktap_print_msg "cmd_args set to $kselftest_cmd_args" >> "$logfile"
+	fi
+
 	TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
 	echo "# $TEST_HDR_MSG"
 	if [ ! -e "$TEST" ]; then
@@ -113,7 +122,6 @@ run_one()
 		if [ -x /usr/bin/stdbuf ]; then
 			stdbuf="/usr/bin/stdbuf --output=L "
 		fi
-		eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
 		if [ -x "$TEST" ]; then
 			cmd="$stdbuf ./$BASENAME_TEST $kselftest_cmd_args"
 		elif [ -x "./ksft_runner.sh" ]; then
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
  2026-07-14  6:34 [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable QianhengPeng
@ 2026-09-01  2:44 ` Qianheng Peng
  2026-09-01  9:46   ` Hangbin Liu
  0 siblings, 1 reply; 8+ messages in thread
From: Qianheng Peng @ 2026-09-01  2:44 UTC (permalink / raw)
  To: shuah, broonie, jackmanb, liuhangbin, rbm
  Cc: skhan, linux-kselftest, linux-kernel, pengqh1

On Tue, 14 Jul 2026 14:34:43, Qianheng Peng wrote:
>Normally the value of kselftest_cmd_args can be read from settings file,
>but it will be overridden by KSELFTEST_TEST_KMOD_SH_ARGS which can be
>empty if user did not give it a real value.So it should be better to
>check if KSELFTEST_TEST_KMOD_SH_ARGS is emty before referring its
>value to kselftest_cmd_args.
>
>Signed-off-by: QianhengPeng <pengqh1@chinatelecom.cn>
>---
> tools/testing/selftests/kselftest/runner.sh | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
>index 311811d..a99147a 100644
>--- a/tools/testing/selftests/kselftest/runner.sh
>+++ b/tools/testing/selftests/kselftest/runner.sh
>@@ -103,6 +103,15 @@ run_one()
> 		ktap_print_msg "timeout set to $kselftest_timeout" >> "$logfile"
> 	fi
> 
>+	# Exported environment variable overrides the settings file
>+	eval kselftest_eval_cmd_args="\$${kselftest_cmd_args_ref:-}"
>+	if [ -n "$kselftest_eval_cmd_args" ]; then
>+		kselftest_cmd_args=$kselftest_eval_cmd_args
>+		ktap_print_msg "overriding cmd_args to $kselftest_cmd_args" >> "$logfile"
>+	elif [ -n "$kselftest_cmd_args" ]; then
>+		ktap_print_msg "cmd_args set to $kselftest_cmd_args" >> "$logfile"
>+	fi
>+
> 	TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
> 	echo "# $TEST_HDR_MSG"
> 	if [ ! -e "$TEST" ]; then
>@@ -113,7 +122,6 @@ run_one()
> 		if [ -x /usr/bin/stdbuf ]; then
> 			stdbuf="/usr/bin/stdbuf --output=L "
> 		fi
>-		eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
> 		if [ -x "$TEST" ]; then
> 			cmd="$stdbuf ./$BASENAME_TEST $kselftest_cmd_args"
> 		elif [ -x "./ksft_runner.sh" ]; then
>-- 
>1.8.3.1
>

Gentle ping. This patch has been sent on July and has not got response since then.
The patch still applies cleanly to current mainline.Glad to resend if it's preferable.

Thanks and Regards,
Qianheng

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
  2026-09-01  2:44 ` Qianheng Peng
@ 2026-09-01  9:46   ` Hangbin Liu
  2026-09-01 12:07     ` Qianheng Peng
  0 siblings, 1 reply; 8+ messages in thread
From: Hangbin Liu @ 2026-09-01  9:46 UTC (permalink / raw)
  To: Qianheng Peng
  Cc: shuah, broonie, jackmanb, liuhangbin, rbm, skhan,
	linux-kselftest, linux-kernel

Hi Qianheng
On Tue, Sep 01, 2026 at 10:44:52AM +0800, Qianheng Peng wrote:
> On Tue, 14 Jul 2026 14:34:43, Qianheng Peng wrote:
> >Normally the value of kselftest_cmd_args can be read from settings file,
> >but it will be overridden by KSELFTEST_TEST_KMOD_SH_ARGS which can be
> >empty if user did not give it a real value.So it should be better to
> >check if KSELFTEST_TEST_KMOD_SH_ARGS is emty before referring its
> >value to kselftest_cmd_args.

Where does KSELFTEST_TEST_KMOD_SH_ARGS from? kmod testing? If yes,
can we fix it there?

Thanks
Hangbin

> >
> >Signed-off-by: QianhengPeng <pengqh1@chinatelecom.cn>
> >---
> > tools/testing/selftests/kselftest/runner.sh | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> >diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> >index 311811d..a99147a 100644
> >--- a/tools/testing/selftests/kselftest/runner.sh
> >+++ b/tools/testing/selftests/kselftest/runner.sh
> >@@ -103,6 +103,15 @@ run_one()
> > 		ktap_print_msg "timeout set to $kselftest_timeout" >> "$logfile"
> > 	fi
> > 
> >+	# Exported environment variable overrides the settings file
> >+	eval kselftest_eval_cmd_args="\$${kselftest_cmd_args_ref:-}"
> >+	if [ -n "$kselftest_eval_cmd_args" ]; then
> >+		kselftest_cmd_args=$kselftest_eval_cmd_args
> >+		ktap_print_msg "overriding cmd_args to $kselftest_cmd_args" >> "$logfile"
> >+	elif [ -n "$kselftest_cmd_args" ]; then
> >+		ktap_print_msg "cmd_args set to $kselftest_cmd_args" >> "$logfile"
> >+	fi
> >+
> > 	TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
> > 	echo "# $TEST_HDR_MSG"
> > 	if [ ! -e "$TEST" ]; then
> >@@ -113,7 +122,6 @@ run_one()
> > 		if [ -x /usr/bin/stdbuf ]; then
> > 			stdbuf="/usr/bin/stdbuf --output=L "
> > 		fi
> >-		eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
> > 		if [ -x "$TEST" ]; then
> > 			cmd="$stdbuf ./$BASENAME_TEST $kselftest_cmd_args"
> > 		elif [ -x "./ksft_runner.sh" ]; then
> >-- 
> >1.8.3.1
> >
> 
> Gentle ping. This patch has been sent on July and has not got response since then.
> The patch still applies cleanly to current mainline.Glad to resend if it's preferable.
> 
> Thanks and Regards,
> Qianheng

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
  2026-09-01  9:46   ` Hangbin Liu
@ 2026-09-01 12:07     ` Qianheng Peng
  2026-09-02  1:32       ` Hangbin Liu
  0 siblings, 1 reply; 8+ messages in thread
From: Qianheng Peng @ 2026-09-01 12:07 UTC (permalink / raw)
  To: hangbin.liu
  Cc: broonie, jackmanb, linux-kernel, linux-kselftest, liuhangbin,
	pengqh1, rbm, shuah, skhan

Hi Hangbin,
On Tue, 1 Sep 2026 17:46:14, Hangbin Liu wrote:
>Hi Qianheng
>On Tue, Sep 01, 2026 at 10:44:52AM +0800, Qianheng Peng wrote:
>> On Tue, 14 Jul 2026 14:34:43, Qianheng Peng wrote:
>> >Normally the value of kselftest_cmd_args can be read from settings file,
>> >but it will be overridden by KSELFTEST_TEST_KMOD_SH_ARGS which can be
>> >empty if user did not give it a real value.So it should be better to
>> >check if KSELFTEST_TEST_KMOD_SH_ARGS is emty before referring its
>> >value to kselftest_cmd_args.
>
>Where does KSELFTEST_TEST_KMOD_SH_ARGS from? kmod testing? If yes,
>can we fix it there?

Sorry for my confusing commit message and poor English.
In fact the problem is not KSELFTEST_TEST_KMOD_SH_ARGS but empty
kselftest_cmd_args_ref.
If user does not export "KSELFTEST_${BASENAME_SANITIZED}_ARGS" (the
${BASENAME_SANITIZED} here can be anything like KMOD or RTCTEST) explicitly,
kselftest_cmd_args_ref will be empty by default and override kselftest_cmd_args.
So this patch will check whether kselftest_cmd_args_ref is empty before
starting one test.

Thanks,
Qianheng

>
>Thanks
>Hangbin
>
>> >
>> >Signed-off-by: QianhengPeng <pengqh1@chinatelecom.cn>
>> >---
>> > tools/testing/selftests/kselftest/runner.sh | 10 +++++++++-
>> > 1 file changed, 9 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
>> >index 311811d..a99147a 100644
>> >--- a/tools/testing/selftests/kselftest/runner.sh
>> >+++ b/tools/testing/selftests/kselftest/runner.sh
>> >@@ -103,6 +103,15 @@ run_one()
>> > 		ktap_print_msg "timeout set to $kselftest_timeout" >> "$logfile"
>> > 	fi
>> > 
>> >+	# Exported environment variable overrides the settings file
>> >+	eval kselftest_eval_cmd_args="\$${kselftest_cmd_args_ref:-}"
>> >+	if [ -n "$kselftest_eval_cmd_args" ]; then
>> >+		kselftest_cmd_args=$kselftest_eval_cmd_args
>> >+		ktap_print_msg "overriding cmd_args to $kselftest_cmd_args" >> "$logfile"
>> >+	elif [ -n "$kselftest_cmd_args" ]; then
>> >+		ktap_print_msg "cmd_args set to $kselftest_cmd_args" >> "$logfile"
>> >+	fi
>> >+
>> > 	TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
>> > 	echo "# $TEST_HDR_MSG"
>> > 	if [ ! -e "$TEST" ]; then
>> >@@ -113,7 +122,6 @@ run_one()
>> > 		if [ -x /usr/bin/stdbuf ]; then
>> > 			stdbuf="/usr/bin/stdbuf --output=L "
>> > 		fi
>> >-		eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
>> > 		if [ -x "$TEST" ]; then
>> > 			cmd="$stdbuf ./$BASENAME_TEST $kselftest_cmd_args"
>> > 		elif [ -x "./ksft_runner.sh" ]; then
>> >-- 
>> >1.8.3.1
>> >
>> 
>> Gentle ping. This patch has been sent on July and has not got response since then.
>> The patch still applies cleanly to current mainline.Glad to resend if it's preferable.
>> 
>> Thanks and Regards,
>> Qianheng

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
  2026-09-01 12:07     ` Qianheng Peng
@ 2026-09-02  1:32       ` Hangbin Liu
  2026-09-02  3:05         ` Qianheng Peng
  0 siblings, 1 reply; 8+ messages in thread
From: Hangbin Liu @ 2026-09-02  1:32 UTC (permalink / raw)
  To: Qianheng Peng
  Cc: broonie, linux-kernel, linux-kselftest, liuhangbin, rbm, shuah, skhan

On Tue, Sep 01, 2026 at 08:07:22PM +0800, Qianheng Peng wrote:
> Hi Hangbin,
> On Tue, 1 Sep 2026 17:46:14, Hangbin Liu wrote:
> >Hi Qianheng
> >On Tue, Sep 01, 2026 at 10:44:52AM +0800, Qianheng Peng wrote:
> >> On Tue, 14 Jul 2026 14:34:43, Qianheng Peng wrote:
> >> >Normally the value of kselftest_cmd_args can be read from settings file,
> >> >but it will be overridden by KSELFTEST_TEST_KMOD_SH_ARGS which can be
> >> >empty if user did not give it a real value.So it should be better to
> >> >check if KSELFTEST_TEST_KMOD_SH_ARGS is emty before referring its
> >> >value to kselftest_cmd_args.
> >
> >Where does KSELFTEST_TEST_KMOD_SH_ARGS from? kmod testing? If yes,
> >can we fix it there?
> 
> Sorry for my confusing commit message and poor English.
> In fact the problem is not KSELFTEST_TEST_KMOD_SH_ARGS but empty
> kselftest_cmd_args_ref.
> If user does not export "KSELFTEST_${BASENAME_SANITIZED}_ARGS" (the
> ${BASENAME_SANITIZED} here can be anything like KMOD or RTCTEST) explicitly,
> kselftest_cmd_args_ref will be empty by default and override kselftest_cmd_args.
> So this patch will check whether kselftest_cmd_args_ref is empty before
> starting one test.

Oh, I see what you mean now.

	kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"

This is used to retrieve parameters for a *specific* test. When a test does not
require extra parameters, `KSELFTEST_${BASENAME_SANITIZED}_ARGS` is not set.
This has the same effect as when a user exports it but does not assign a value.

Therefore, I do not think we need to add a check for this case. Users should
understand the semantics when exporting the `KSELFTEST_${BASENAME_SANITIZED}_ARGS`
variable.

Thanks
Hangbin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
  2026-09-02  1:32       ` Hangbin Liu
@ 2026-09-02  3:05         ` Qianheng Peng
  2026-09-02  6:22           ` Hangbin Liu
  0 siblings, 1 reply; 8+ messages in thread
From: Qianheng Peng @ 2026-09-02  3:05 UTC (permalink / raw)
  To: hangbin.liu
  Cc: broonie, linux-kernel, linux-kselftest, liuhangbin, pengqh1, rbm,
	shuah, skhan

On 2026-09-02  1:32 UTC, Hangbin Liu wrote:
>On Tue, Sep 01, 2026 at 08:07:22PM +0800, Qianheng Peng wrote:
>> Hi Hangbin,
>> On Tue, 1 Sep 2026 17:46:14, Hangbin Liu wrote:
>> >Hi Qianheng
>> >On Tue, Sep 01, 2026 at 10:44:52AM +0800, Qianheng Peng wrote:
>> >> On Tue, 14 Jul 2026 14:34:43, Qianheng Peng wrote:
>> >> >Normally the value of kselftest_cmd_args can be read from settings file,
>> >> >but it will be overridden by KSELFTEST_TEST_KMOD_SH_ARGS which can be
>> >> >empty if user did not give it a real value.So it should be better to
>> >> >check if KSELFTEST_TEST_KMOD_SH_ARGS is emty before referring its
>> >> >value to kselftest_cmd_args.
>> >
>> >Where does KSELFTEST_TEST_KMOD_SH_ARGS from? kmod testing? If yes,
>> >can we fix it there?
>> 
>> Sorry for my confusing commit message and poor English.
>> In fact the problem is not KSELFTEST_TEST_KMOD_SH_ARGS but empty
>> kselftest_cmd_args_ref.
>> If user does not export "KSELFTEST_${BASENAME_SANITIZED}_ARGS" (the
>> ${BASENAME_SANITIZED} here can be anything like KMOD or RTCTEST) explicitly,
>> kselftest_cmd_args_ref will be empty by default and override kselftest_cmd_args.
>> So this patch will check whether kselftest_cmd_args_ref is empty before
>> starting one test.
>
>Oh, I see what you mean now.
>
>	kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
>
>This is used to retrieve parameters for a *specific* test. When a test does not
>require extra parameters, `KSELFTEST_${BASENAME_SANITIZED}_ARGS` is not set.
>This has the same effect as when a user exports it but does not assign a value.
>
>Therefore, I do not think we need to add a check for this case. Users should
>understand the semantics when exporting the `KSELFTEST_${BASENAME_SANITIZED}_ARGS`
>variable.
>
>Thanks
>Hangbin

Yes, if just only adding a check, that really makes no sense.
But there is a issue for the follow case:
---------------------------------------------------------------------------
User wrote "cmd_args=nice_value" in settings file and want to use the
`nice_value` as arguments for a test program.So the user didn't need to
export something like `KSELFTEST_XXXX_ARGS` because kselftest_cmd_args should
get the `nice_value` as we can see

    eval "kselftest_$field"="$value"

But in current runner.sh, kselftest_cmd_args has always been set up by
`KSELFTEST_XXXX_ARGS` as we can see

    kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
    eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"

As `KSELFTEST_XXXX_ARGS` was empty, the user unexpectedly lost `nice_value`
written in settings file.
---------------------------------------------------------------------------

According to the implement of runner.sh, it may want to give users two choices:
1) using arguments in settings file
2) or using arguments by export `KSELFTEST_${BASENAME_SANITIZED}_ARGS`
But now users has been forced to use `KSELFTEST_${BASENAME_SANITIZED}_ARGS`.

For the case above and making users happy, I did two things in the patch:
1) If `KSELFTEST_${BASENAME_SANITIZED}_ARGS was empty, keep the value from settings file
2) If not, set kselftest_cmd_args by `KSELFTEST_${BASENAME_SANITIZED}_ARGS` 

Thanks and regards,
Qianheng

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
  2026-09-02  3:05         ` Qianheng Peng
@ 2026-09-02  6:22           ` Hangbin Liu
  2026-09-02  9:24             ` Qianheng Peng
  0 siblings, 1 reply; 8+ messages in thread
From: Hangbin Liu @ 2026-09-02  6:22 UTC (permalink / raw)
  To: Qianheng Peng
  Cc: broonie, linux-kernel, linux-kselftest, liuhangbin, rbm, shuah, skhan

Hi Qianheng,
On Wed, Sep 02, 2026 at 11:05:42AM +0800, Qianheng Peng wrote:
> >Oh, I see what you mean now.
> >
> >	kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
> >
> >This is used to retrieve parameters for a *specific* test. When a test does not
> >require extra parameters, `KSELFTEST_${BASENAME_SANITIZED}_ARGS` is not set.
> >This has the same effect as when a user exports it but does not assign a value.
> >
> >Therefore, I do not think we need to add a check for this case. Users should
> >understand the semantics when exporting the `KSELFTEST_${BASENAME_SANITIZED}_ARGS`
> >variable.
> >
> >Thanks
> >Hangbin
> 
> Yes, if just only adding a check, that really makes no sense.
> But there is a issue for the follow case:
> ---------------------------------------------------------------------------
> User wrote "cmd_args=nice_value" in settings file and want to use the
> `nice_value` as arguments for a test program.So the user didn't need to
> export something like `KSELFTEST_XXXX_ARGS` because kselftest_cmd_args should
> get the `nice_value` as we can see
> 
>     eval "kselftest_$field"="$value"

AFAIK, the setting file is usually used for setting timeout value. As
you can see in runner.sh, it checks kselftest_timeout after reading setting
file.

> But in current runner.sh, kselftest_cmd_args has always been set up by
> `KSELFTEST_XXXX_ARGS` as we can see
> 
>     kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
>     eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
> 
> As `KSELFTEST_XXXX_ARGS` was empty, the user unexpectedly lost `nice_value`
> written in settings file.
> ---------------------------------------------------------------------------
> 
> According to the implement of runner.sh, it may want to give users two choices:
> 1) using arguments in settings file

I’m not sure if this was the original intention. The configuration file
lives under the test directory, which typically holds multiple tests.
What if individual tests require different parameters? How would you
supply distinct arguments in the config file using only a single
`cmd_args` variable?

> 2) or using arguments by export `KSELFTEST_${BASENAME_SANITIZED}_ARGS`
> But now users has been forced to use `KSELFTEST_${BASENAME_SANITIZED}_ARGS`.

In contrast, `KSELFTEST_${BASENAME_SANITIZED}_ARGS` can target each specific
test case individually.

Perhaps I have misunderstood the design. Please feel free to correct me.

Thanks
Hangbin
> 
> For the case above and making users happy, I did two things in the patch:
> 1) If `KSELFTEST_${BASENAME_SANITIZED}_ARGS was empty, keep the value from settings file
> 2) If not, set kselftest_cmd_args by `KSELFTEST_${BASENAME_SANITIZED}_ARGS` 
> 
> Thanks and regards,
> Qianheng

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable
  2026-09-02  6:22           ` Hangbin Liu
@ 2026-09-02  9:24             ` Qianheng Peng
  0 siblings, 0 replies; 8+ messages in thread
From: Qianheng Peng @ 2026-09-02  9:24 UTC (permalink / raw)
  To: hangbin.liu
  Cc: broonie, linux-kernel, linux-kselftest, liuhangbin, pengqh1, rbm,
	shuah, skhan

On Wed, 2 Sep 2026 14:22:04 +0800, Hangbin Liu wrote:
>Hi Qianheng,
>On Wed, Sep 02, 2026 at 11:05:42AM +0800, Qianheng Peng wrote:
>> >Oh, I see what you mean now.
>> >
>> >    kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
>> >
>> >This is used to retrieve parameters for a *specific* test. When a test does not
>> >require extra parameters, `KSELFTEST_${BASENAME_SANITIZED}_ARGS` is not set.
>> >This has the same effect as when a user exports it but does not assign a value.
>> >
>> >Therefore, I do not think we need to add a check for this case. Users should
>> >understand the semantics when exporting the `KSELFTEST_${BASENAME_SANITIZED}_ARGS`
>> >variable.
>> >
>> >Thanks
>> >Hangbin
>> 
>> Yes, if just only adding a check, that really makes no sense.
>> But there is a issue for the follow case:
>> ---------------------------------------------------------------------------
>> User wrote "cmd_args=nice_value" in settings file and want to use the
>> `nice_value` as arguments for a test program.So the user didn't need to
>> export something like `KSELFTEST_XXXX_ARGS` because kselftest_cmd_args should
>> get the `nice_value` as we can see
>> 
>>     eval "kselftest_$field"="$value"
>
>AFAIK, the setting file is usually used for setting timeout value. As
>you can see in runner.sh, it checks kselftest_timeout after reading setting
>file.
>
Yes, I see it. Basically I did similar things to kselftest_cmd_args like things that
runner.sh did to kselftest_timeout.
>> But in current runner.sh, kselftest_cmd_args has always been set up by
>> `KSELFTEST_XXXX_ARGS` as we can see
>> 
>>     kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
>>     eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
>> 
>> As `KSELFTEST_XXXX_ARGS` was empty, the user unexpectedly lost `nice_value`
>> written in settings file.
>> ---------------------------------------------------------------------------
>> 
>> According to the implement of runner.sh, it may want to give users two choices:
>> 1) using arguments in settings file
>
>I’m not sure if this was the original intention. The configuration file
>lives under the test directory, which typically holds multiple tests.
>What if individual tests require different parameters? How would you
>supply distinct arguments in the config file using only a single
>`cmd_args` variable?
>
Yeah, settings file can't cover every different parameter(maybe neither timeout).
So `cmd_args` in settings file is only fit for single test which needs special
parameter such as setting `test_range=1,976` for test_kmod.sh of bpf directory,
however other bpf tests can ignore the parameter.
>> 2) or using arguments by export `KSELFTEST_${BASENAME_SANITIZED}_ARGS`
>> But now users has been forced to use `KSELFTEST_${BASENAME_SANITIZED}_ARGS`.
>
>In contrast, `KSELFTEST_${BASENAME_SANITIZED}_ARGS` can target each specific
>test case individually.
>
>Perhaps I have misunderstood the design. Please feel free to correct me.
>
I think you are right. I am also not sure the original intention as I just found
current runner.sh had the feature which can retrieve cmd_args from settings file.
This feature made me think it can be convenient for users who didn't want to
export `KSELFTEST_${BASENAME_SANITIZED}_ARGS` everytime they start one test.

Anyway, glad to recieve more advice whatever next step has more work to do or
nothing to do.

Thanks and regards,
Qianheng

>Thanks
>Hangbin
>> 
>> For the case above and making users happy, I did two things in the patch:
>> 1) If `KSELFTEST_${BASENAME_SANITIZED}_ARGS was empty, keep the value from settings file
>> 2) If not, set kselftest_cmd_args by `KSELFTEST_${BASENAME_SANITIZED}_ARGS` 
>> 
>> Thanks and regards,
>> Qianheng

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-02  9:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-14  6:34 [PATCH] selftests: runner.sh: avoid overriding kselftest_cmd_args with empty variable QianhengPeng
2026-09-01  2:44 ` Qianheng Peng
2026-09-01  9:46   ` Hangbin Liu
2026-09-01 12:07     ` Qianheng Peng
2026-09-02  1:32       ` Hangbin Liu
2026-09-02  3:05         ` Qianheng Peng
2026-09-02  6:22           ` Hangbin Liu
2026-09-02  9:24             ` Qianheng Peng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®