linux-kernel - Re: [PATCH] kselftest: runner: fix TAP output for skipped tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6461f067-0fea-4419-dd56-2661c44a803a@redhat.com>
Date:   Wed, 10 Jun 2020 20:22:51 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     "Bird, Tim" <Tim.Bird@...y.com>,
        Shuah Khan <skhan@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>
Cc:     "shuah@...nel.org" <shuah@...nel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH] kselftest: runner: fix TAP output for skipped tests

On 10/06/20 19:43, Bird, Tim wrote:
>>>   		(rc=$?;	\
>>>   		if [ $rc -eq $skip_rc ]; then	\
>>> -			echo "not ok $test_num $TEST_HDR_MSG # SKIP"
>>> +			echo "ok $test_num $TEST_HDR_MSG # SKIP"
> 
> This is a pretty big change, and might break upstream CIs that have come to
> rely on kselftest's existing behavior.  I know it's going to break Fuego's parsing
> of results.

Do you have a pointer to this code?

> kselftest has a few conventions that are different from the TAP spec, 
> and a few items it does that are extensions to the TAP spec.

Yes, there are extensions to directives are not a problem and parsers
might raise an error on them.  That can be an issue, but it's a separate
one (and it's easier to ignore it as long as test pass...).

> IMHO, the TAP spec got this one wrong, but I could be convinced
> otherwise.

Here the TAP spec says that a skip starts with "ok" and has a "SKIP"
directive, and anyone can parse it to treat as it as a failure if
desirable.  But doing something else should be treated simply as a
violation of the spec, it's not a matter of "right" or "wrong".

So, if you want to use "not ok ... # SKIP", don't call it TAP.

However, I noticed now that there is another instance of "not ok.*SKIP"
in testing/selftests/kselftest.h (and also one in a comment).  So they
should all be fixed at the same time, and I'm okay with holding this patch.

Paolo

> But I think we should discuss this among CI users of
> kselftest before making the change.
> 
> I started work quite a while ago on an effort to document the
> conventions used by kselftest (particularly where it deviates
> from the TAP spec),  but never submitted it.
> 
> I'm going to submit what I've got as an RFC now, for discussion,
> even though it's not finished.  I'll do that in a separate thread.
> 
> 
>>>   		elif [ $rc -eq $timeout_rc ]; then \
>>>   			echo "#"
>>>   			echo "not ok $test_num $TEST_HDR_MSG # TIMEOUT"
>>>
>>
>> Thanks. I will pull this in for Linux 5.8-rc2
> Shuah - can you hold off on this until we discuss it?
> 
> Thanks,
>  -- Tim
>