Re: [KTAP V2 PATCH] ktap_v2: add test metadata

From: Rae Moar <rmoar@google.com>
To: frowand.list@gmail.com, davidgow@google.com,
	skhan@linuxfoundation.org,  keescook@chromium.org,
	Tim.Bird@sony.com, brendanhiggins@google.com
Cc: corbet@lwn.net, guillaume.tucker@collabora.com,
	dlatypov@google.com,  kernelci@lists.linux.dev,
	kunit-dev@googlegroups.com,  linux-kselftest@vger.kernel.org,
	linux-doc@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [KTAP V2 PATCH] ktap_v2: add test metadata
Date: Thu, 20 Jul 2023 17:31:07 -0400	[thread overview]
Message-ID: <CA+GJov6w2GvD8th0t9RW=K1ntHk4dQRuYa4hoDHcmzBDK5YriA@mail.gmail.com> (raw)
In-Reply-To: <20230420205734.1288498-1-rmoar@google.com>

On Thu, Apr 20, 2023 at 4:57 PM Rae Moar <rmoar@google.com> wrote:
>
> Add specification for declaring test metadata to the KTAP v2 spec.
>
> The purpose of test metadata is to allow for the declaration of essential
> testing information in KTAP output. This information includes test
> names, test configuration info, test attributes, and test files.
>
> There have been similar ideas around the idea of test metadata such as test
> prefixes and test name lines. However, I propose this specification as an
> overall fix for these issues.
>
> These test metadata lines are a form of diagnostic lines with the
> format: "# <metadata_type>: <data>". As a type of diagnostic line, test
> metadata lines are compliant with KTAP v1, which will help to not
> interfere too much with current parsers.
>
> Specifically the "# Subtest:" line is derived from the TAP 14 spec:
> https://testanything.org/tap-version-14-specification.html.
>
> The proposed location for test metadata is in the test header, between the
> version line and the test plan line. Note including diagnostic lines in
> the test header is a depature from KTAP v1.
>
> This location provides two main benefits:
>
> First, metadata will be printed prior to when subtests are run. Then if a
> test fails, test metadata can help discern which test is causing the issue
> and potentially why.
>
> Second, this location ensures that the lines will not be accidentally
> parsed as a subtest's diagnostic lines because the lines are bordered by
> the version line and plan line.
>
> Here is an example of test metadata:
>
>  KTAP version 2
>  # Config: CONFIG_TEST=y
>  1..1
>      KTAP version 2
>      # Subtest: test_suite
>      # File: /sys/kernel/...
>      # Attributes: slow
>      # Other: example_test
>      1..2
>      ok 1 test_1
>      ok 2 test_2
>  ok 1 test_suite

Hi everyone!

I have been doing some more thinking on KTAP Metadata as I have been
working on the KUnit Test Attributes patch set
(https://lore.kernel.org/all/20230719222338.259684-1-rmoar@google.com/).
Two additional ideas have come up in the discussion:

1) I wonder if it would be easier to separate "ktap_attributes" into
individual attributes.

The two proposed KUnit attributes currently are speed and module name.
I think it would be easier for parsing and reading if these attributes
had corresponding "ktap_speed" and "ktap_module" categories. Then, in
the future if there are too many attributes to print on separate lines
they could be grouped into a "ktap_attributes" category later.

2) I wonder if we can shift the concept of KTAP metadata to all tests
rather than just suites.

I think it would be very valuable to have a KTAP metadata format that
is flexible to work for both suites and test cases. To transition this
to test cases, I propose we would use the same format we have been
discussing but just printed just before the test result line (David
Gow originally came up with this idea). This would look something like
this:

KTAP version 2
# ktap_config: CONFIG_TEST=y
1..1
    KTAP version 2
    # ktap_test: test_suite
    # ktap_module: example
    1..2
    ok 1 test_1
    # ktap_test: test_2
    # ktap_speed: slow
    # test initializing   // diagnostic data
    ok 2 test_2
ok 1 test_suite

I don't love using the "ktap_test: test_2" line since the test name is
repeated. However, I like that this mirrors the same format used for a
suite and I currently think it is the best way to define the start of
the metadata header.

The test name line could actually be useful by providing context for
any test diagnostic data printed below or if the test crashes while
running.

What do people think of these ideas?

Thanks!
-Rae

>
> Here is a link to a version of the KUnit parser that is able to parse test
> metadata lines for KTAP version 2. Note this includes test metadata
> lines for the main level of KTAP.
>
> Link: https://kunit-review.googlesource.com/c/linux/+/5809
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---
>
> Hi everyone,
>
> I would like to use this proposal similar to an RFC to gather ideas on the
> topic of test metadata. Let me know what you think.
>
> I am also interested in brainstorming a list of recognized metadata types.
> Providing recognized metadata types would be helpful in parsing and
> displaying test metadata in a useful way.
>
> Current ideas:
> - "# Subtest: <test_name>" to indicate test name (name must match
>   corresponding result line)
> - "# Attributes: <attributes list>" to indicate test attributes (list
>   separated by commas)
> - "# File: <file_path>" to indicate file used in testing
>
> Any other ideas?
>
> Note this proposal replaces two of my previous proposals: "ktap_v2: add
> recognized test name line" and "ktap_v2: allow prefix to KTAP lines."
>
> Thanks!
> -Rae
>
> Note: this patch is based on Frank's ktap_spec_version_2 branch.