Re: [PATCH v1] selftests/mm: Log run_vmtests.sh results in TAP format

[Ryan Roberts <ryan.roberts@arm.com>]

On 16/12/2023 02:40, John Hubbard wrote:
> On 12/15/23 18:25, John Hubbard wrote:
>> On 12/15/23 06:28, Ryan Roberts wrote:
>> ...
>>> I've kept all the existing "pretty" output and results summary as is, it just
>>> gets a hash in front of it when TAP is enabled.
>>>
>>> so this:
>>>
>>> -----------------------
>>> running ./hugepage-mmap
>>> -----------------------
>>> Returned address is 0xffff89e00000
>>> First hex is 0
>>> First hex is 3020100
>>> [PASS]
>>> SUMMARY: PASS=1 SKIP=0 FAIL=0
>>>
>>> becomes this:
>>>
>>> TAP version 13
>>> # -----------------------
>>> # running ./hugepage-mmap
>>> # -----------------------
>>> # Returned address is 0xffff89e00000
>>> # First hex is 0
>>> # First hex is 3020100
>>> # [PASS]
>>> ok 1 hugepage-mmap
>>> # SUMMARY: PASS=1 SKIP=0 FAIL=0
>>> 1..1
>>>
>>> If you think the latter is ofensive, then I can do the wrapping as you suggest.
>>
>> I applied this and ran the tests, all while carefully reminding myself
>> to "think like a human". :) And from that perspective, to me, the output
>> is effectively the same: the leading '#' characters do not really change
>> anything, from a readability point of view.
>>
>> So IMHO you're on perfectly solid ground, if you just switch over
>> directly to this format.

Great thanks for taking a look!

>>
>> Tested-by: John Hubbard <jhubbard@nvidia.com>
>>
> 
> I should also point out that some of the subtests already attempt a TAP
> output. So now we end up with TAP-within-TAP output for those programs.

It's actually TAP-in-TAP-in-TAP if you're running from run_kselftest.sh :)

> 
> For example:
>     # -----------------------
>     # running ./madv_populate
>     # -----------------------
>     # TAP version 13
>     # 1..21
>     # # [RUN] test_prot_read
>     # ok 1 MADV_POPULATE_READ with PROT_READ
>     # ok 2 MADV_POPULATE_WRITE with PROT_READ
>     # # [RUN] test_prot_write
>     # ok 3 MADV_POPULATE_READ with PROT_WRITE
>     ...etc...
> 
> Note the double level of leading '#' characters.
> 
> Again, this is still readable enough for humans. But it should probably
> be removed in subsequent patches to the subtests.

I personally don't agree with this. It would be difficult to flatten to a single
TAP instance because the top level doesn't have a clue how many test cases the
child is running. Trying to do this will make things more fragile and less
modular. LAVA can certainly deal with nested test cases and correctly parses
everything to test case names that contain the test name at each level of
nesting. The thing I was trying to solve with this patch was that previously the
top level (run_kselftest.sh) and the bottom level (individual mm test binaries)
were using TAP, but the middle level (run_vmtests.sh) wasn't, and this was
confusing the LAVA parser.

> 
> 
> thanks,