Re: [PATCH] selftests/run_kselftest.sh: Add --error-on-fail flag

[Brendan Jackman <jackmanb@google.com>]

On Fri Oct 10, 2025 at 6:34 AM UTC, Thomas Weißschuh wrote:
> On Tue, Oct 07, 2025 at 11:06:46AM +0000, Brendan Jackman wrote:
>> +report_failure()
>> +{
>> +	echo "not ok $*" >> "$kselftest_failures_file"
>> +	echo "$*" >> "$kselftest_failures_file"
>
> Both of these go into the failures file. The first should go to stdout, no?

Oops, thanks.

>> --- a/tools/testing/selftests/run_kselftest.sh
>> +++ b/tools/testing/selftests/run_kselftest.sh
>> @@ -36,6 +36,7 @@ Usage: $0 [OPTIONS]
>>    -n | --netns			Run each test in namespace
>>    -h | --help			Show this usage info
>>    -o | --override-timeout	Number of seconds after which we timeout
>> +  -e | --error-on-fail	After finishing all tests, exit with code 1 if any failed.
>
> To me it looks like the new behavior could be the default,
> removing the need for an additional option.

That sounds good to me, I will wait a day or two to see if anyone
objects to this before I send v2.

IMO it's important that your testing systems can tell you the difference
between "tests failed, your code is broken" and "I fell over, the
infrastructure is broken". My thinking was that if anyone's using this
in a proper CI loop, they won't actually want --error-on-fail because
then they sort of lose this distinction (nonzero exit code means "I fell
over" while "your tests failed" is communicated through KTAP). But, now
I think about it, this script is running on the kernel under test so
it's not capable of making this distinction anyway. So default SGTM.

>> +kselftest_failures_file=$(mktemp --tmpdir kselftest-failures-XXXXXX)
>
> Quoting?

Oh yep, thanks again.

> I'm not a fan of the implementation details, but also can't come up with
> something better.

Yeah, I feel the same. I think the next best thing is to rewrite this
whole thing in a proper programming language though. 

Thanks for taking a look :)