Re: [PATCH 1/1] Documentation: dev-tools: clarify KTAP specification wording

[Frank Rowand]

On 2/4/22 6:50 PM, David Gow wrote:
> On Sat, Feb 5, 2022 at 8:18 AM Frank Rowand <frowand.list@xxxxxxxxx> wrote:
>>
>> On 2/4/22 5:13 PM, David Gow wrote:
>>> On Sat, Feb 5, 2022 at 4:32 AM <frowand.list@xxxxxxxxx> wrote:
>>>>
>>>> From: Frank Rowand <frank.rowand@xxxxxxxx>
>>>>
>>>> Clarify some confusing phrasing.
>>>
>>> Thanks for this! A few comments below:
>>>
>>>>
>>>> Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx>
>>>> ---
>>>>
>>>> One item that may result in bikeshedding is that I added the spec
>>>> version to the title line.
>>>
>>> This is fine by me.
>>>
>>>>
>>>>  Documentation/dev-tools/ktap.rst | 12 ++++++------
>>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/dev-tools/ktap.rst b/Documentation/dev-tools/ktap.rst
>>>> index 878530cb9c27..3b7a26816930 100644
>>>> --- a/Documentation/dev-tools/ktap.rst
>>>> +++ b/Documentation/dev-tools/ktap.rst
>>>> @@ -1,8 +1,8 @@
>>>>  .. SPDX-License-Identifier: GPL-2.0
>>>>
>>>> -========================================
>>>> -The Kernel Test Anything Protocol (KTAP)
>>>> -========================================
>>>> +===================================================
>>>> +The Kernel Test Anything Protocol (KTAP), version 1
>>>> +===================================================
>>>>
>>>>  TAP, or the Test Anything Protocol is a format for specifying test results used
>>>>  by a number of projects. It's website and specification are found at this `link
>>>> @@ -186,7 +186,7 @@ starting with another KTAP version line and test plan, and end with the overall
>>>>  result. If one of the subtests fail, for example, the parent test should also
>>>>  fail.
>>>>
>>>> -Additionally, all result lines in a subtest should be indented. One level of
>>>> +Additionally, all lines in a subtest should be indented. One level of
>>>
>>> The original reason for this is to accommodate "unknown" lines which
>>> were not generated by the test itself (e.g, a KASAN report or BUG or
>>> something). These are awkward, as sometimes they're a useful thing to
>>> have as part of the test result, and sometimes they're unrelated spam.
>>> (Additionally, I think kselftest will indent these, as it indents the
>>> full results in a separate pass afterwards, but KUnit won't, as the
>>> level of nesting is done during printing.)
>>>
>>> Personally, I'd rather leave this as is, or perhaps call out "unknown"
>>> lines explicitly, e.g:
>>> Additionally, all lines in a subtest (except for 'unknown' lines)
>>> should be indented...
>>
>> Only listing result lines as being indented is not consistent with
>> the "Example KTAP output" section.  The example shows:
>>
>>    Version line           - indented
>>    Plan line              - indented
>>    Test case result lines - indented
>>    Diagnostic lines       - indented
>>    Unknown lines          - not shown in the example
>>
>> So there seem to be at least 4 types of lines that are indented for a
>> nested test.
> 
> Agreed.
> 
>>
>> The TAP standard (I'll use version 14 for my examples) does not allow
>> unknown lines (TAP 14 calls them "Anything else").  It says "is
>> incorrect", and "When the `pragma +strict` is enabled, incorrect test
>> lines SHOULD result in the test set being a failure, ...".  TAP 14
>> calls for the opposite behavior if `pragma -strict` is set.
> 
> Are you reading the same version 14 spec as me?
> 
> https://github.com/TestAnything/Specification/blob/tap-14-specification/specification.md

Thanks for the link.

I wasn't even aware of that repo.  A hint for anyone else that wants to look at the
spec in that repo, it is in a branch (tap-14-specfication).  I was using
https://github.com/isaacs/testanything.github.io.git which has slightly more
recent activity (Sept 6, 2015 vs Jan 19, 2015).

-Frank

> 
> I can find these lines in the version 13 spec, but not TAP14, which
> doesn't mention "Anything else" lines at all...
> 
> Not that it matters... I'll just follow along with version 13.
> 
>>
>> TAP 14 goes on to say "`Test::Harness` silently ignores incorrect lines,
>> but will become more stringent in the futures.
>>
>> It seems to me that KTAP "Unknown lines" are fundamentally different
>> than TAP 14 "Anything else" lines.  Tests that generate KTAP output
>> may print their results to the system console (or log), in which
>> case kernel messages (or for the system log the messages may even
>> come from non-kernel sources) either directly triggered by a test or
>> from a task that is totally unrelated to the test may exist in the KTAP
>> data stream.  So I would agree that "Unknown lines" are not indented.
>> Even if the "Unknown line" is directly triggered by the test.
> 
> I do think that KTAP "unknown lines" and TAP "anything else" lines
> cover similar ground, the big difference being that in KTAP they're
> explicitly permitted, rather than "incorrect".  I guess how similar
> they are is as much a matter of perspective as anything...
> 
> I'd agree that "unknown lines" don't _need_ to be indented, but I
> wouldn't call it an error to indent them if that's something a test
> harness does.
> 
>>
>> But I think the KTAP specification should say that "Diagnostic lines"
>> are emitted by the test (or the test harness), and thus must be
>> indented when related to a nested test.
>>
>> And as you suggest, "Unknown lines" should be explicitly called out
>> as not being part of "lines in a subtest", thus do not need to be
>> indented.
>>
>> Does that sound good?
>>
> 
> Agreed on both counts. Sounds great, thanks!
> 
> Cheers,
> -- David
> 
>>>
>>> Thoughts?
>>>
>>>>  indentation is two spaces: "  ". The indentation should begin at the version
>>>>  line and should end before the parent test's result line.
>>>>
>>>> @@ -225,8 +225,8 @@ Major differences between TAP and KTAP
>>>>  --------------------------------------
>>>>
>>>>  Note the major differences between the TAP and KTAP specification:
>>>> -- yaml and json are not recommended in diagnostic messages
>>>> -- TODO directive not recognized
>>>> +- yaml and json are not recommended in KTAP diagnostic messages
>>>> +- TODO directive not recognized in KTAP
>>>>  - KTAP allows for an arbitrary number of tests to be nested
>>>>
>>>
>>> Looks good here, cheers.
>>>
>>>
>>>>  The TAP14 specification does permit nested tests, but instead of using another
>>>> --
>>>> Frank Rowand <frank.rowand@xxxxxxxx>
>>>>
>>