* [PATCHv2] selftests: Use ktap helpers for runner.sh
@ 2026-02-24 4:10 Hangbin Liu
2026-02-24 11:38 ` Brendan Jackman
0 siblings, 1 reply; 3+ messages in thread
From: Hangbin Liu @ 2026-02-24 4:10 UTC (permalink / raw)
To: linux-kselftest; +Cc: Shuah Khan, Brendan Jackman, netdev, Hangbin Liu
Instead of manually writing ktap messages, we should use the formal
ktap helpers in runner.sh. 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. However, the output
does not include the total results, such as how many tests passed or
failed.
Let’s convert all manually printed messages in runner.sh to use the
formal ktap helpers. Here are 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>
---
v2: Update commit description and comments in code. (Brendan Jackman)
---
tools/testing/selftests/kselftest/runner.sh | 84 ++++++++++++---------
tools/testing/selftests/run_kselftest.sh | 21 ++++--
2 files changed, 63 insertions(+), 42 deletions(-)
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 3a62039fa621..dfd6fedf8911 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,20 @@ run_many()
fi
if [ -n "$RUN_IN_NETNS" ]; then
run_in_netns &
+ pids+=($!)
else
run_one "$DIR" "$TEST" "$test_num"
fi
done
- wait
+ # These variables are outputs of ktap_helpers.sh but since we've
+ # run the test in a subprocess we need to update them manually
+ 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 84d45254675c..3ffd03f1334d 100755
--- a/tools/testing/selftests/run_kselftest.sh
+++ b/tools/testing/selftests/run_kselftest.sh
@@ -121,18 +121,23 @@ if [ -n "$SKIP" ]; then
done
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCHv2] selftests: Use ktap helpers for runner.sh
2026-02-24 4:10 [PATCHv2] selftests: Use ktap helpers for runner.sh Hangbin Liu
@ 2026-02-24 11:38 ` Brendan Jackman
2026-02-24 13:21 ` Hangbin Liu
0 siblings, 1 reply; 3+ messages in thread
From: Brendan Jackman @ 2026-02-24 11:38 UTC (permalink / raw)
To: Hangbin Liu, linux-kselftest; +Cc: Shuah Khan, Brendan Jackman, netdev
On Tue Feb 24, 2026 at 4:10 AM UTC, Hangbin Liu wrote:
> Instead of manually writing ktap messages, we should use the formal
> ktap helpers in runner.sh. 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. However, the output
> does not include the total results, such as how many tests passed or
> failed.
>
> Let’s convert all manually printed messages in runner.sh to use the
> formal ktap helpers. Here are 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>
> ---
>
> v2: Update commit description and comments in code. (Brendan Jackman)
Cool thanks sorry I did not really review the code before but now I'm
taking a closer look.
I don't parse KTAP but I did check that this doesn't break my usecase
[0] so:
Tested-By: Brendan Jackman <jackmanb@google.com>
[0]: https://github.com/bjackman/limmat-kernel-nix/blob/master/ktests.nix
> ---
> tools/testing/selftests/kselftest/runner.sh | 84 ++++++++++++---------
> tools/testing/selftests/run_kselftest.sh | 21 ++++--
> 2 files changed, 63 insertions(+), 42 deletions(-)
>
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index 3a62039fa621..dfd6fedf8911 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)
Nit (no need to respin just for this, but if you're already respinning):
Diff noise here.
> + 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,20 @@ run_many()
> fi
> if [ -n "$RUN_IN_NETNS" ]; then
> run_in_netns &
> + pids+=($!)
> else
> run_one "$DIR" "$TEST" "$test_num"
> fi
> done
>
> - wait
> + # These variables are outputs of ktap_helpers.sh but since we've
> + # run the test in a subprocess we need to update them manually
> + 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
Can we please be consistent about how we do these two big conditional
blocks? Also should they use `case`? Also please add something to not
fail silently if $rc has a garbage value (I guess just log a warning).
FWIW I still think this is very yucky but, it seems switching to the
helper library is a worthwhile cleanup (plus, this patch drops a level
of subshell nesting in the tap_timeout pipeline, which seems like a step
in the right direction!) so I think it would be irrational to block that
cleanup just coz of this yuckiness.
> }
> diff --git a/tools/testing/selftests/run_kselftest.sh b/tools/testing/selftests/run_kselftest.sh
> index 84d45254675c..3ffd03f1334d 100755
> --- a/tools/testing/selftests/run_kselftest.sh
> +++ b/tools/testing/selftests/run_kselftest.sh
> @@ -121,18 +121,23 @@ if [ -n "$SKIP" ]; then
> done
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCHv2] selftests: Use ktap helpers for runner.sh
2026-02-24 11:38 ` Brendan Jackman
@ 2026-02-24 13:21 ` Hangbin Liu
0 siblings, 0 replies; 3+ messages in thread
From: Hangbin Liu @ 2026-02-24 13:21 UTC (permalink / raw)
To: Brendan Jackman; +Cc: linux-kselftest, Shuah Khan, netdev
On Tue, Feb 24, 2026 at 11:38:34AM +0000, Brendan Jackman wrote:
> Cool thanks sorry I did not really review the code before but now I'm
> taking a closer look.
>
> I don't parse KTAP but I did check that this doesn't break my usecase
> [0] so:
>
> Tested-By: Brendan Jackman <jackmanb@google.com>
>
> [0]: https://github.com/bjackman/limmat-kernel-nix/blob/master/ktests.nix
Thanks for the testing.
> > 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)
>
> Nit (no need to respin just for this, but if you're already respinning):
> Diff noise here.
This is to make the variables in reverse Christmas tree based on kernel code
style.
>
> > + 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,20 @@ run_many()
> > fi
> > if [ -n "$RUN_IN_NETNS" ]; then
> > run_in_netns &
> > + pids+=($!)
> > else
> > run_one "$DIR" "$TEST" "$test_num"
> > fi
> > done
> >
> > - wait
> > + # These variables are outputs of ktap_helpers.sh but since we've
> > + # run the test in a subprocess we need to update them manually
> > + 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
>
> Can we please be consistent about how we do these two big conditional
> blocks? Also should they use `case`? Also please add something to not
Ah, yes, we can use case here. Let me update the patch.
> fail silently if $rc has a garbage value (I guess just log a warning).
A log here may break the KTAP output. While run_in_netns() calls run_one()
in the end, which will print the exit value if it's a garbage value. i.e.
ktap_test_fail "$test_num $TEST_HDR_MSG # exit=$rc"
>
> FWIW I still think this is very yucky but, it seems switching to the
> helper library is a worthwhile cleanup (plus, this patch drops a level
> of subshell nesting in the tap_timeout pipeline, which seems like a step
> in the right direction!) so I think it would be irrational to block that
> cleanup just coz of this yuckiness.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-02-24 13:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24 4:10 [PATCHv2] selftests: Use ktap helpers for runner.sh Hangbin Liu
2026-02-24 11:38 ` Brendan Jackman
2026-02-24 13:21 ` Hangbin Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox