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
