[PATCH selftests] selftests: Use ktap helpers for runner.sh

[PATCH selftests] selftests: Use ktap helpers for runner.sh

[Hangbin Liu]

This has been on my todo list for a long time. We should use ktap
helpers in runner.sh. I saw Brendan did some work in d9e6269e3303
("selftests/run_kselftest.sh: exit with error if tests fail") to make
run_kselftest.sh exit with the correct return value. But we still use a
custom solution in runner.sh. Let's convert all the print messages to use
formal ktap helpers. Here’s what I changed:

  1. Move TAP header from runner.sh to run_kselftest.sh, since run_kselftest.sh
     is the only caller of run_many().
  2. In run_kselftest.sh, call run_many() in main process to count the
     pass/fail numbers.
  3. In run_kselftest.sh, do not generate kselftest_failures_file; just
     use ktap_print_totals to report the result.
  4. In runner.sh run_one(), get the return value and use ktap helpers for
     all pass/fail reporting. This allows counting pass/fail numbers in the
     main process.
  5. In runner.sh run_in_netns(), also return the correct rc, so we can
     count results during wait.

After the change, the printed result looks like:

  not ok 4 4 selftests: clone3: clone3_cap_checkpoint_restore # exit=1
  # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

  ]# echo $?
  1

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 tools/testing/selftests/kselftest/runner.sh | 83 ++++++++++++---------
 tools/testing/selftests/run_kselftest.sh    | 21 ++++--
 2 files changed, 62 insertions(+), 42 deletions(-)

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 3a62039fa621..f3307f2d1f48 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -1,8 +1,8 @@
-#!/bin/sh
+#!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Runs a set of tests in a given subdirectory.
-export skip_rc=4
+. $(dirname "$(readlink -e "${BASH_SOURCE[0]}")")/ktap_helpers.sh
 export timeout_rc=124
 export logfile=/dev/stdout
 export per_test_logging=
@@ -44,17 +44,11 @@ tap_timeout()
 	fi
 }
 
-report_failure()
-{
-	echo "not ok $*"
-	echo "$*" >> "$kselftest_failures_file"
-}
-
 run_one()
 {
 	DIR="$1"
 	TEST="$2"
-	local test_num="$3"
+	local rc test_num="$3"
 
 	BASENAME_TEST=$(basename $TEST)
 
@@ -102,16 +96,17 @@ run_one()
 	# Command line timeout overrides the settings file
 	if [ -n "$kselftest_override_timeout" ]; then
 		kselftest_timeout="$kselftest_override_timeout"
-		echo "# overriding timeout to $kselftest_timeout" >> "$logfile"
+		ktap_print_msg "overriding timeout to $kselftest_timeout" >> "$logfile"
 	else
-		echo "# timeout set to $kselftest_timeout" >> "$logfile"
+		ktap_print_msg "timeout set to $kselftest_timeout" >> "$logfile"
 	fi
 
 	TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
 	echo "# $TEST_HDR_MSG"
 	if [ ! -e "$TEST" ]; then
-		echo "# Warning: file $TEST is missing!"
-		report_failure "$test_num $TEST_HDR_MSG"
+		ktap_print_msg "Warning: file $TEST is missing!"
+		ktap_test_fail "$test_num $TEST_HDR_MSG"
+		rc=$KSFT_FAIL
 	else
 		if [ -x /usr/bin/stdbuf ]; then
 			stdbuf="/usr/bin/stdbuf --output=L "
@@ -122,33 +117,37 @@ run_one()
 		elif [ -x "./ksft_runner.sh" ]; then
 			cmd="$stdbuf ./ksft_runner.sh ./$BASENAME_TEST"
 		else
-			echo "# Warning: file $TEST is not executable"
+			ktap_print_msg "Warning: file $TEST is not executable"
 
 			if [ $(head -n 1 "$TEST" | cut -c -2) = "#!" ]
 			then
 				interpreter=$(head -n 1 "$TEST" | cut -c 3-)
 				cmd="$stdbuf $interpreter ./$BASENAME_TEST"
 			else
-				report_failure "$test_num $TEST_HDR_MSG"
-				return
+				ktap_test_fail "$test_num $TEST_HDR_MSG"
+				return $KSFT_FAIL
 			fi
 		fi
 		cd `dirname $TEST` > /dev/null
-		((((( tap_timeout "$cmd" 2>&1; echo $? >&3) |
+		(((( tap_timeout "$cmd" 2>&1; echo $? >&3) |
 			tap_prefix >&4) 3>&1) |
-			(read xs; exit $xs)) 4>>"$logfile" &&
-		echo "ok $test_num $TEST_HDR_MSG") ||
-		(rc=$?;	\
-		if [ $rc -eq $skip_rc ]; then	\
-			echo "ok $test_num $TEST_HDR_MSG # SKIP"
-		elif [ $rc -eq $timeout_rc ]; then \
-			echo "#"
-			report_failure "$test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
+			(read xs; exit $xs)) 4>>"$logfile"
+		rc=$?
+		if [ "$rc" -eq "$KSFT_PASS" ]; then
+			ktap_test_pass "$test_num $TEST_HDR_MSG"
+		elif [ "$rc" -eq "$KSFT_SKIP" ]; then
+			ktap_test_skip "$test_num $TEST_HDR_MSG"
+		elif [ "$rc" -eq "$KSFT_XFAIL" ]; then
+			ktap_test_xfail "$test_num $TEST_HDR_MSG"
+		elif [ "$rc" -eq "$timeout_rc" ]; then
+			ktap_test_fail "$test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
 		else
-			report_failure "$test_num $TEST_HDR_MSG # exit=$rc"
-		fi)
+			ktap_test_fail "$test_num $TEST_HDR_MSG # exit=$rc"
+		fi
 		cd - >/dev/null
 	fi
+
+	return $rc
 }
 
 in_netns()
@@ -164,27 +163,34 @@ in_netns()
 
 run_in_netns()
 {
-	local netns=$(mktemp -u ${BASENAME_TEST}-XXXXXX)
 	local tmplog="/tmp/$(mktemp -u ${BASENAME_TEST}-XXXXXX)"
+	local netns=$(mktemp -u ${BASENAME_TEST}-XXXXXX)
+	local rc
+
 	ip netns add $netns
 	if [ $? -ne 0 ]; then
-		echo "# Warning: Create namespace failed for $BASENAME_TEST"
-		echo "not ok $test_num selftests: $DIR: $BASENAME_TEST # Create NS failed"
+		ktap_print_msg "Warning: Create namespace failed for $BASENAME_TEST"
+		ktap_test_fail "$test_num selftests: $DIR: $BASENAME_TEST # Create NS failed"
 	fi
 	ip -n $netns link set lo up
+
 	in_netns $netns &> $tmplog
+	rc=$?
+
 	ip netns del $netns &> /dev/null
+	# Cat the log at once to avoid parallel netns logs.
 	cat $tmplog
 	rm -f $tmplog
+	return $rc
 }
 
 run_many()
 {
-	echo "TAP version 13"
 	DIR="${PWD#${BASE_DIR}/}"
 	test_num=0
-	total=$(echo "$@" | wc -w)
-	echo "1..$total"
+	local rc
+	pids=()
+
 	for TEST in "$@"; do
 		BASENAME_TEST=$(basename $TEST)
 		test_num=$(( test_num + 1 ))
@@ -194,10 +200,19 @@ run_many()
 		fi
 		if [ -n "$RUN_IN_NETNS" ]; then
 			run_in_netns &
+			pids+=($!)
 		else
 			run_one "$DIR" "$TEST" "$test_num"
 		fi
 	done
 
-	wait
+	# Handle the return values when running in netns.
+	for pid in "${pids[@]}"; do
+		wait "$pid"
+		rc=$?
+		[ "$rc" -eq "$KSFT_PASS" ] && KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
+		[ "$rc" -eq "$KSFT_FAIL" ] && KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
+		[ "$rc" -eq "$KSFT_SKIP" ] && KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
+		[ "$rc" -eq "$KSFT_XFAIL" ] && KTAP_CNT_XFAIL=$((KTAP_CNT_XFAIL+1))
+	done
 }
diff --git a/tools/testing/selftests/run_kselftest.sh b/tools/testing/selftests/run_kselftest.sh
index d4be97498b32..10930d09f2a5 100755
--- a/tools/testing/selftests/run_kselftest.sh
+++ b/tools/testing/selftests/run_kselftest.sh
@@ -110,18 +110,23 @@ if [ -n "$TESTS" ]; then
 	available="$(echo "$valid" | sed -e 's/ /\n/g')"
 fi
 
-kselftest_failures_file="$(mktemp --tmpdir kselftest-failures-XXXXXX)"
-export kselftest_failures_file
-
+curdir=$(pwd)
+total=$(echo "$available" | wc -w)
 collections=$(echo "$available" | cut -d: -f1 | sort | uniq)
+
+ktap_print_header
+ktap_set_plan "$total"
+
 for collection in $collections ; do
 	[ -w /dev/kmsg ] && echo "kselftest: Running tests in $collection" >> /dev/kmsg
 	tests=$(echo "$available" | grep "^$collection:" | cut -d: -f2)
-	($dryrun cd "$collection" && $dryrun run_many $tests)
+	$dryrun cd "$collection" && $dryrun run_many $tests
+	$dryrun cd "$curdir"
 done
 
-failures="$(cat "$kselftest_failures_file")"
-rm "$kselftest_failures_file"
-if "$ERROR_ON_FAIL" && [ "$failures" ]; then
-	exit 1
+ktap_print_totals
+if "$ERROR_ON_FAIL" && [ "$KTAP_CNT_FAIL" -ne 0 ]; then
+	exit "$KSFT_FAIL"
+else
+	exit "$KSFT_PASS"
 fi
-- 
2.50.1

Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

[Brendan Jackman]

On Fri Feb 6, 2026 at 8:17 AM UTC, Hangbin Liu wrote:
> This has been on my todo list for a long time. We should use ktap
> helpers in runner.sh. I saw Brendan did some work in d9e6269e3303
> ("selftests/run_kselftest.sh: exit with error if tests fail") to make
> run_kselftest.sh exit with the correct return value. But we still use a
> custom solution in runner.sh. Let's convert all the print messages to use
> formal ktap helpers. Here’s what I changed:
>
>   1. Move TAP header from runner.sh to run_kselftest.sh, since run_kselftest.sh
>      is the only caller of run_many().
>   2. In run_kselftest.sh, call run_many() in main process to count the
>      pass/fail numbers.
>   3. In run_kselftest.sh, do not generate kselftest_failures_file; just
>      use ktap_print_totals to report the result.
>   4. In runner.sh run_one(), get the return value and use ktap helpers for
>      all pass/fail reporting. This allows counting pass/fail numbers in the
>      main process.
>   5. In runner.sh run_in_netns(), also return the correct rc, so we can
>      count results during wait.
>
> After the change, the printed result looks like:
>
>   not ok 4 4 selftests: clone3: clone3_cap_checkpoint_restore # exit=1
>   # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
>
>   ]# echo $?
>   1

Sorry I'm being a bit lazy by not investigating this myself but the
current intended behaviour is for runner.sh to output correct KTAP,
right? Could you describe what behavioural changes this is expected to
bring - does it fix/change the KTAP output? (Or if it's just a cleanup
with no intended change then please note that in the commit message).

I have not reviewed the code properly yet (I just read enough to check
that it still does the thing I care about i.e. return an error code when
something fails). I can do a proper review next week if you add a bit
more context re the question above though.

> -	wait
> +	# Handle the return values when running in netns.
> +	for pid in "${pids[@]}"; do
> +		wait "$pid"
> +		rc=$?
> +		[ "$rc" -eq "$KSFT_PASS" ] && KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
> +		[ "$rc" -eq "$KSFT_FAIL" ] && KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
> +		[ "$rc" -eq "$KSFT_SKIP" ] && KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
> +		[ "$rc" -eq "$KSFT_XFAIL" ] && KTAP_CNT_XFAIL=$((KTAP_CNT_XFAIL+1))

These variables don't seeem to have anything to do with KTAP so I think
they should have a different name prefix. I guess KSFT_*? Or maybe
RUNNER_* or something.

Also I guess we should initialise them to 0 so they are still set if no
tests actually get run?

Also I think it's probably time to start documenting the
interface of runner.sh - sorry I probably should have done this when I
created kselftest_failures_file. Can you add a comment at the top to
show that these variables are part of the "API"?

Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

[Hangbin Liu]

Hi Brendan,
On Fri, Feb 06, 2026 at 10:48:59AM +0000, Brendan Jackman wrote:
> On Fri Feb 6, 2026 at 8:17 AM UTC, Hangbin Liu wrote:
> > This has been on my todo list for a long time. We should use ktap
> > helpers in runner.sh. I saw Brendan did some work in d9e6269e3303
> > ("selftests/run_kselftest.sh: exit with error if tests fail") to make
> > run_kselftest.sh exit with the correct return value. But we still use a
> > custom solution in runner.sh. Let's convert all the print messages to use
> > formal ktap helpers. Here’s what I changed:
> >
> >   1. Move TAP header from runner.sh to run_kselftest.sh, since run_kselftest.sh
> >      is the only caller of run_many().
> >   2. In run_kselftest.sh, call run_many() in main process to count the
> >      pass/fail numbers.
> >   3. In run_kselftest.sh, do not generate kselftest_failures_file; just
> >      use ktap_print_totals to report the result.
> >   4. In runner.sh run_one(), get the return value and use ktap helpers for
> >      all pass/fail reporting. This allows counting pass/fail numbers in the
> >      main process.
> >   5. In runner.sh run_in_netns(), also return the correct rc, so we can
> >      count results during wait.
> >
> > After the change, the printed result looks like:
> >
> >   not ok 4 4 selftests: clone3: clone3_cap_checkpoint_restore # exit=1
> >   # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
> >
> >   ]# echo $?
> >   1
> 
> Sorry I'm being a bit lazy by not investigating this myself but the
> current intended behaviour is for runner.sh to output correct KTAP,
> right? Could you describe what behavioural changes this is expected to

For runner.sh, I just want to make it using the formal helper in
ktap_helpers.sh other than manually echo.

> bring - does it fix/change the KTAP output? (Or if it's just a cleanup
> with no intended change then please note that in the commit message).

It changes the run_kselftest.sh's output, with a total result. e.g.

# Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

> 
> I have not reviewed the code properly yet (I just read enough to check
> that it still does the thing I care about i.e. return an error code when
> something fails). I can do a proper review next week if you add a bit
> more context re the question above though.

Yes, I also care about the return code. This patch doesn't break it.

> 
> > -	wait
> > +	# Handle the return values when running in netns.
> > +	for pid in "${pids[@]}"; do
> > +		wait "$pid"
> > +		rc=$?
> > +		[ "$rc" -eq "$KSFT_PASS" ] && KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
> > +		[ "$rc" -eq "$KSFT_FAIL" ] && KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
> > +		[ "$rc" -eq "$KSFT_SKIP" ] && KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
> > +		[ "$rc" -eq "$KSFT_XFAIL" ] && KTAP_CNT_XFAIL=$((KTAP_CNT_XFAIL+1))
> 
> These variables don't seeem to have anything to do with KTAP so I think
> they should have a different name prefix. I guess KSFT_*? Or maybe
> RUNNER_* or something.
> 
> Also I guess we should initialise them to 0 so they are still set if no
> tests actually get run?
> 
> Also I think it's probably time to start documenting the
> interface of runner.sh - sorry I probably should have done this when I
> created kselftest_failures_file. Can you add a comment at the top to
> show that these variables are part of the "API"?

These variables are defined in ktap_helpers.sh and used by the ktap helpers.

Thanks
Hangbin

Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

[Brendan Jackman]

On Fri Feb 6, 2026 at 2:36 PM UTC, Hangbin Liu wrote:
> Hi Brendan,
> On Fri, Feb 06, 2026 at 10:48:59AM +0000, Brendan Jackman wrote:
>> On Fri Feb 6, 2026 at 8:17 AM UTC, Hangbin Liu wrote:
>> > This has been on my todo list for a long time. We should use ktap
>> > helpers in runner.sh. I saw Brendan did some work in d9e6269e3303
>> > ("selftests/run_kselftest.sh: exit with error if tests fail") to make
>> > run_kselftest.sh exit with the correct return value. But we still use a
>> > custom solution in runner.sh. Let's convert all the print messages to use
>> > formal ktap helpers. Here’s what I changed:
>> >
>> >   1. Move TAP header from runner.sh to run_kselftest.sh, since run_kselftest.sh
>> >      is the only caller of run_many().
>> >   2. In run_kselftest.sh, call run_many() in main process to count the
>> >      pass/fail numbers.
>> >   3. In run_kselftest.sh, do not generate kselftest_failures_file; just
>> >      use ktap_print_totals to report the result.
>> >   4. In runner.sh run_one(), get the return value and use ktap helpers for
>> >      all pass/fail reporting. This allows counting pass/fail numbers in the
>> >      main process.
>> >   5. In runner.sh run_in_netns(), also return the correct rc, so we can
>> >      count results during wait.
>> >
>> > After the change, the printed result looks like:
>> >
>> >   not ok 4 4 selftests: clone3: clone3_cap_checkpoint_restore # exit=1
>> >   # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
>> >
>> >   ]# echo $?
>> >   1
>> 
>> Sorry I'm being a bit lazy by not investigating this myself but the
>> current intended behaviour is for runner.sh to output correct KTAP,
>> right? Could you describe what behavioural changes this is expected to
>
> For runner.sh, I just want to make it using the formal helper in
> ktap_helpers.sh other than manually echo.
>
>> bring - does it fix/change the KTAP output? (Or if it's just a cleanup
>> with no intended change then please note that in the commit message).
>
> It changes the run_kselftest.sh's output, with a total result. e.g.
>
> # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

Cool thanks, then please just clarify the commit message to say this.

>> I have not reviewed the code properly yet (I just read enough to check
>> that it still does the thing I care about i.e. return an error code when
>> something fails). I can do a proper review next week if you add a bit
>> more context re the question above though.
>
> Yes, I also care about the return code. This patch doesn't break it.
>
>> 
>> > -	wait
>> > +	# Handle the return values when running in netns.
>> > +	for pid in "${pids[@]}"; do
>> > +		wait "$pid"
>> > +		rc=$?
>> > +		[ "$rc" -eq "$KSFT_PASS" ] && KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
>> > +		[ "$rc" -eq "$KSFT_FAIL" ] && KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
>> > +		[ "$rc" -eq "$KSFT_SKIP" ] && KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
>> > +		[ "$rc" -eq "$KSFT_XFAIL" ] && KTAP_CNT_XFAIL=$((KTAP_CNT_XFAIL+1))
>> 
>> These variables don't seeem to have anything to do with KTAP so I think
>> they should have a different name prefix. I guess KSFT_*? Or maybe
>> RUNNER_* or something.
>> 
>> Also I guess we should initialise them to 0 so they are still set if no
>> tests actually get run?
>> 
>> Also I think it's probably time to start documenting the
>> interface of runner.sh - sorry I probably should have done this when I
>> created kselftest_failures_file. Can you add a comment at the top to
>> show that these variables are part of the "API"?
>
> These variables are defined in ktap_helpers.sh and used by the ktap helpers.

Hm, I see. Then it seems pretty hacky to mutate them directly here, and
is it correct to always increment them by 1?

I think the "correct" thing to do here is to parse the KTAP from the
child process's stdout, but I assume that's not actually practical with
the current tools we have available. (I could be wrong though, let me
know if that's the case).

Or maybe we can dump the values of the variables at the end of the child
and then add them into the parent process' values... 

Or, if we just keep the current approach I think just having some
commentary to explain why we are doing this would help. I.e. "these
variables are outputs of ktap_helpers.sh but since we've run the test in
a subprocess we need to update them manually" (the current comment talks
about the netns which I think is confusing, I don't think the netns is
relevant at all).

Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

[Hangbin Liu]

On Mon, Feb 23, 2026 at 12:40:03PM +0000, Brendan Jackman wrote:
> On Fri Feb 6, 2026 at 2:36 PM UTC, Hangbin Liu wrote:
> > Hi Brendan,
> > On Fri, Feb 06, 2026 at 10:48:59AM +0000, Brendan Jackman wrote:
> >> On Fri Feb 6, 2026 at 8:17 AM UTC, Hangbin Liu wrote:
> >> > This has been on my todo list for a long time. We should use ktap
> >> > helpers in runner.sh. I saw Brendan did some work in d9e6269e3303
> >> > ("selftests/run_kselftest.sh: exit with error if tests fail") to make
> >> > run_kselftest.sh exit with the correct return value. But we still use a
> >> > custom solution in runner.sh. Let's convert all the print messages to use
> >> > formal ktap helpers. Here’s what I changed:
> >> >
> >> >   1. Move TAP header from runner.sh to run_kselftest.sh, since run_kselftest.sh
> >> >      is the only caller of run_many().
> >> >   2. In run_kselftest.sh, call run_many() in main process to count the
> >> >      pass/fail numbers.
> >> >   3. In run_kselftest.sh, do not generate kselftest_failures_file; just
> >> >      use ktap_print_totals to report the result.
> >> >   4. In runner.sh run_one(), get the return value and use ktap helpers for
> >> >      all pass/fail reporting. This allows counting pass/fail numbers in the
> >> >      main process.
> >> >   5. In runner.sh run_in_netns(), also return the correct rc, so we can
> >> >      count results during wait.
> >> >
> >> > After the change, the printed result looks like:
> >> >
> >> >   not ok 4 4 selftests: clone3: clone3_cap_checkpoint_restore # exit=1
> >> >   # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
> >> >
> >> >   ]# echo $?
> >> >   1
> >> 
> > It changes the run_kselftest.sh's output, with a total result. e.g.
> >
> > # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
> 
> Cool thanks, then please just clarify the commit message to say this.

Yes, I have added this with 3. use ktap_print_totals to report the result.

> 
> >> I have not reviewed the code properly yet (I just read enough to check
> >> that it still does the thing I care about i.e. return an error code when
> >> something fails). I can do a proper review next week if you add a bit
> >> more context re the question above though.
> >
> > Yes, I also care about the return code. This patch doesn't break it.
> >
> >> 
> >> > -	wait
> >> > +	# Handle the return values when running in netns.
> >> > +	for pid in "${pids[@]}"; do
> >> > +		wait "$pid"
> >> > +		rc=$?
> >> > +		[ "$rc" -eq "$KSFT_PASS" ] && KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
> >> > +		[ "$rc" -eq "$KSFT_FAIL" ] && KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
> >> > +		[ "$rc" -eq "$KSFT_SKIP" ] && KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
> >> > +		[ "$rc" -eq "$KSFT_XFAIL" ] && KTAP_CNT_XFAIL=$((KTAP_CNT_XFAIL+1))
> >> 
> > These variables are defined in ktap_helpers.sh and used by the ktap helpers.
> 
> Hm, I see. Then it seems pretty hacky to mutate them directly here, and
> is it correct to always increment them by 1?
> 
> I think the "correct" thing to do here is to parse the KTAP from the
> child process's stdout, but I assume that's not actually practical with
> the current tools we have available. (I could be wrong though, let me
> know if that's the case).
> 
> Or maybe we can dump the values of the variables at the end of the child
> and then add them into the parent process' values... 

Based on code

        for TEST in "$@"; do
                if [ -n "$RUN_IN_NETNS" ]; then
                        run_in_netns &
                        pids+=($!)
                else
                        run_one "$DIR" "$TEST" "$test_num"
                fi
        done

        # Handle the return values when running in netns.
        for pid in "${pids[@]}"; do
		...
	done

This hack only affects when we run tests in netns.

If we dump the values for each child process. It do make the result more
accurate. But the process is complex (maybe force dump the result to a file and
analyse it? But some tests doesn't have ktap helper and only return $rc), and
some tests may also have sub-process that use KTAP numbers but not count in
main process.

Since the purpose is to count the test pass/failed number. I think it's enough
to only count the return value when run tests in netns.

> 
> Or, if we just keep the current approach I think just having some
> commentary to explain why we are doing this would help. I.e. "these
> variables are outputs of ktap_helpers.sh but since we've run the test in
> a subprocess we need to update them manually" (the current comment talks
> about the netns which I think is confusing, I don't think the netns is
> relevant at all).

Thanks, I will update the commit description.

Regards
Hangbin

Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

[Hangbin Liu]

Hi Shuah, Brendan,

Any feedback for this patch?

Thanks
Hangbin
On Fri, Feb 06, 2026 at 08:17:04AM +0000, Hangbin Liu wrote:
> This has been on my todo list for a long time. We should use ktap
> helpers in runner.sh. I saw Brendan did some work in d9e6269e3303
> ("selftests/run_kselftest.sh: exit with error if tests fail") to make
> run_kselftest.sh exit with the correct return value. But we still use a
> custom solution in runner.sh. Let's convert all the print messages to use
> formal ktap helpers. Here’s what I changed:
> 
>   1. Move TAP header from runner.sh to run_kselftest.sh, since run_kselftest.sh
>      is the only caller of run_many().
>   2. In run_kselftest.sh, call run_many() in main process to count the
>      pass/fail numbers.
>   3. In run_kselftest.sh, do not generate kselftest_failures_file; just
>      use ktap_print_totals to report the result.
>   4. In runner.sh run_one(), get the return value and use ktap helpers for
>      all pass/fail reporting. This allows counting pass/fail numbers in the
>      main process.
>   5. In runner.sh run_in_netns(), also return the correct rc, so we can
>      count results during wait.
> 
> After the change, the printed result looks like:
> 
>   not ok 4 4 selftests: clone3: clone3_cap_checkpoint_restore # exit=1
>   # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
> 
>   ]# echo $?
>   1
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  tools/testing/selftests/kselftest/runner.sh | 83 ++++++++++++---------
>  tools/testing/selftests/run_kselftest.sh    | 21 ++++--
>  2 files changed, 62 insertions(+), 42 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index 3a62039fa621..f3307f2d1f48 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -1,8 +1,8 @@
> -#!/bin/sh
> +#!/bin/bash
>  # SPDX-License-Identifier: GPL-2.0
>  #
>  # Runs a set of tests in a given subdirectory.
> -export skip_rc=4
> +. $(dirname "$(readlink -e "${BASH_SOURCE[0]}")")/ktap_helpers.sh
>  export timeout_rc=124
>  export logfile=/dev/stdout
>  export per_test_logging=
> @@ -44,17 +44,11 @@ tap_timeout()
>  	fi
>  }
>  
> -report_failure()
> -{
> -	echo "not ok $*"
> -	echo "$*" >> "$kselftest_failures_file"
> -}
> -
>  run_one()
>  {
>  	DIR="$1"
>  	TEST="$2"
> -	local test_num="$3"
> +	local rc test_num="$3"
>  
>  	BASENAME_TEST=$(basename $TEST)
>  
> @@ -102,16 +96,17 @@ run_one()
>  	# Command line timeout overrides the settings file
>  	if [ -n "$kselftest_override_timeout" ]; then
>  		kselftest_timeout="$kselftest_override_timeout"
> -		echo "# overriding timeout to $kselftest_timeout" >> "$logfile"
> +		ktap_print_msg "overriding timeout to $kselftest_timeout" >> "$logfile"
>  	else
> -		echo "# timeout set to $kselftest_timeout" >> "$logfile"
> +		ktap_print_msg "timeout set to $kselftest_timeout" >> "$logfile"
>  	fi
>  
>  	TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
>  	echo "# $TEST_HDR_MSG"
>  	if [ ! -e "$TEST" ]; then
> -		echo "# Warning: file $TEST is missing!"
> -		report_failure "$test_num $TEST_HDR_MSG"
> +		ktap_print_msg "Warning: file $TEST is missing!"
> +		ktap_test_fail "$test_num $TEST_HDR_MSG"
> +		rc=$KSFT_FAIL
>  	else
>  		if [ -x /usr/bin/stdbuf ]; then
>  			stdbuf="/usr/bin/stdbuf --output=L "
> @@ -122,33 +117,37 @@ run_one()
>  		elif [ -x "./ksft_runner.sh" ]; then
>  			cmd="$stdbuf ./ksft_runner.sh ./$BASENAME_TEST"
>  		else
> -			echo "# Warning: file $TEST is not executable"
> +			ktap_print_msg "Warning: file $TEST is not executable"
>  
>  			if [ $(head -n 1 "$TEST" | cut -c -2) = "#!" ]
>  			then
>  				interpreter=$(head -n 1 "$TEST" | cut -c 3-)
>  				cmd="$stdbuf $interpreter ./$BASENAME_TEST"
>  			else
> -				report_failure "$test_num $TEST_HDR_MSG"
> -				return
> +				ktap_test_fail "$test_num $TEST_HDR_MSG"
> +				return $KSFT_FAIL
>  			fi
>  		fi
>  		cd `dirname $TEST` > /dev/null
> -		((((( tap_timeout "$cmd" 2>&1; echo $? >&3) |
> +		(((( tap_timeout "$cmd" 2>&1; echo $? >&3) |
>  			tap_prefix >&4) 3>&1) |
> -			(read xs; exit $xs)) 4>>"$logfile" &&
> -		echo "ok $test_num $TEST_HDR_MSG") ||
> -		(rc=$?;	\
> -		if [ $rc -eq $skip_rc ]; then	\
> -			echo "ok $test_num $TEST_HDR_MSG # SKIP"
> -		elif [ $rc -eq $timeout_rc ]; then \
> -			echo "#"
> -			report_failure "$test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
> +			(read xs; exit $xs)) 4>>"$logfile"
> +		rc=$?
> +		if [ "$rc" -eq "$KSFT_PASS" ]; then
> +			ktap_test_pass "$test_num $TEST_HDR_MSG"
> +		elif [ "$rc" -eq "$KSFT_SKIP" ]; then
> +			ktap_test_skip "$test_num $TEST_HDR_MSG"
> +		elif [ "$rc" -eq "$KSFT_XFAIL" ]; then
> +			ktap_test_xfail "$test_num $TEST_HDR_MSG"
> +		elif [ "$rc" -eq "$timeout_rc" ]; then
> +			ktap_test_fail "$test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
>  		else
> -			report_failure "$test_num $TEST_HDR_MSG # exit=$rc"
> -		fi)
> +			ktap_test_fail "$test_num $TEST_HDR_MSG # exit=$rc"
> +		fi
>  		cd - >/dev/null
>  	fi
> +
> +	return $rc
>  }
>  
>  in_netns()
> @@ -164,27 +163,34 @@ in_netns()
>  
>  run_in_netns()
>  {
> -	local netns=$(mktemp -u ${BASENAME_TEST}-XXXXXX)
>  	local tmplog="/tmp/$(mktemp -u ${BASENAME_TEST}-XXXXXX)"
> +	local netns=$(mktemp -u ${BASENAME_TEST}-XXXXXX)
> +	local rc
> +
>  	ip netns add $netns
>  	if [ $? -ne 0 ]; then
> -		echo "# Warning: Create namespace failed for $BASENAME_TEST"
> -		echo "not ok $test_num selftests: $DIR: $BASENAME_TEST # Create NS failed"
> +		ktap_print_msg "Warning: Create namespace failed for $BASENAME_TEST"
> +		ktap_test_fail "$test_num selftests: $DIR: $BASENAME_TEST # Create NS failed"
>  	fi
>  	ip -n $netns link set lo up
> +
>  	in_netns $netns &> $tmplog
> +	rc=$?
> +
>  	ip netns del $netns &> /dev/null
> +	# Cat the log at once to avoid parallel netns logs.
>  	cat $tmplog
>  	rm -f $tmplog
> +	return $rc
>  }
>  
>  run_many()
>  {
> -	echo "TAP version 13"
>  	DIR="${PWD#${BASE_DIR}/}"
>  	test_num=0
> -	total=$(echo "$@" | wc -w)
> -	echo "1..$total"
> +	local rc
> +	pids=()
> +
>  	for TEST in "$@"; do
>  		BASENAME_TEST=$(basename $TEST)
>  		test_num=$(( test_num + 1 ))
> @@ -194,10 +200,19 @@ run_many()
>  		fi
>  		if [ -n "$RUN_IN_NETNS" ]; then
>  			run_in_netns &
> +			pids+=($!)
>  		else
>  			run_one "$DIR" "$TEST" "$test_num"
>  		fi
>  	done
>  
> -	wait
> +	# Handle the return values when running in netns.
> +	for pid in "${pids[@]}"; do
> +		wait "$pid"
> +		rc=$?
> +		[ "$rc" -eq "$KSFT_PASS" ] && KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
> +		[ "$rc" -eq "$KSFT_FAIL" ] && KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
> +		[ "$rc" -eq "$KSFT_SKIP" ] && KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
> +		[ "$rc" -eq "$KSFT_XFAIL" ] && KTAP_CNT_XFAIL=$((KTAP_CNT_XFAIL+1))
> +	done
>  }
> diff --git a/tools/testing/selftests/run_kselftest.sh b/tools/testing/selftests/run_kselftest.sh
> index d4be97498b32..10930d09f2a5 100755
> --- a/tools/testing/selftests/run_kselftest.sh
> +++ b/tools/testing/selftests/run_kselftest.sh
> @@ -110,18 +110,23 @@ if [ -n "$TESTS" ]; then
>  	available="$(echo "$valid" | sed -e 's/ /\n/g')"
>  fi
>  
> -kselftest_failures_file="$(mktemp --tmpdir kselftest-failures-XXXXXX)"
> -export kselftest_failures_file
> -
> +curdir=$(pwd)
> +total=$(echo "$available" | wc -w)
>  collections=$(echo "$available" | cut -d: -f1 | sort | uniq)
> +
> +ktap_print_header
> +ktap_set_plan "$total"
> +
>  for collection in $collections ; do
>  	[ -w /dev/kmsg ] && echo "kselftest: Running tests in $collection" >> /dev/kmsg
>  	tests=$(echo "$available" | grep "^$collection:" | cut -d: -f2)
> -	($dryrun cd "$collection" && $dryrun run_many $tests)
> +	$dryrun cd "$collection" && $dryrun run_many $tests
> +	$dryrun cd "$curdir"
>  done
>  
> -failures="$(cat "$kselftest_failures_file")"
> -rm "$kselftest_failures_file"
> -if "$ERROR_ON_FAIL" && [ "$failures" ]; then
> -	exit 1
> +ktap_print_totals
> +if "$ERROR_ON_FAIL" && [ "$KTAP_CNT_FAIL" -ne 0 ]; then
> +	exit "$KSFT_FAIL"
> +else
> +	exit "$KSFT_PASS"
>  fi
> -- 
> 2.50.1
>