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).
