Date:   Wed, 26 Apr 2023 12:18:02 -0400
From:   Rae Moar <rmoar@...gle.com>
To:     Frank Rowand <frowand.list@...il.com>
Cc:     davidgow@...gle.com, skhan@...uxfoundation.org,
        keescook@...omium.org, Tim.Bird@...y.com,
        brendanhiggins@...gle.com, corbet@....net,
        guillaume.tucker@...labora.com, dlatypov@...gle.com,
        kernelci@...ts.linux.dev, kunit-dev@...glegroups.com,
        linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [KTAP V2 PATCH] ktap_v2: add test metadata

On Tue, Apr 25, 2023 at 4:55 PM Frank Rowand <frowand.list@...il.com> wrote:
>
> On 4/20/23 15:57, Rae Moar 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.
>
> This seems like a cleaner approach.
>
> >
> > 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.
>
> I like that.
>
> >
> > 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
> >
> > 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@...gle.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)
>
> I would prefer "Test" to "Subtest" because the type should be allowed for the
> top level test, as well as for subtest levels.

Hi Frank!

Yes, I can see the reasoning to switch to "Test". Although this is a
departure from current behavior, it would be clearer. I am happy to
make this change.

>
> > - "# Attributes: <attributes list>" to indicate test attributes (list
> >   separated by commas)
> > - "# File: <file_path>" to indicate file used in testing
> >
> > Any other ideas?
>
> (Already used in an example above...)
>
> - "# Config: <config_option list> to indicate kernel configuration options
>   (list separated by commas)
>
>     config_option format:
>       Option XXX is enabled: CONFIG_XXX=y
>       Option XXX is not enabled: CONFIG_XXX=n
>       Option XXX is text: CONFIG_XXX="a text string"
>

I like this addition of the "Config" metadata. I also like all of
these format options, including the text string option. Although, I
would be interested in adding "Option XXX is loadable as a module:
CONFIG_XXX=m" to the format list.

> Linux .config format is "#CONFIG_XXX is not set",
> but this would be harder to parse in a list.
>
> A text config option also complicates parsing of a list.  Maybe there
> should not be a list, instead have a separate "# Config:" line for
> each config option.

I'm not sure how to deal with multiple config options. I am split
between either using a list or multiple "Config" lines. I would be
happy with either approach. Maybe a list would be slightly better,
since it is slightly closer to the defined behavior for the attributes
metadata line.

>
> I would like to bifurcate the name space of metadata types, to names
> specified in the standard vs names not in the standard that can be
> used on an experimental or for future use in existing tests.
>
> I can think of at least two ways to implement this:
>
> (1) types that are in the specification all begin with a specific prefix,
> such as "ktap_" (bike shedding on naming welcomed), so the examples woudld be
>
>   # ktap_test:
>   # ktap_attributes:
>   # ktap_file:
>   # ktap_config:
>
> (2) types that are _not_ in the specification all begin with a specific prefix,
> such as "custom_" (bike shedding on naming welcomed).
>

This is an interesting proposal. I like this idea of using a prefix. I
would be happy to add this. I like "ktap_" and "custom_".

Thanks!
-Rae

> >
> > 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.
> >
> >  Documentation/dev-tools/ktap.rst | 51 ++++++++++++++++++++++++++++++--
> >  1 file changed, 48 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/ktap.rst b/Documentation/dev-tools/ktap.rst
> > index ff77f4aaa6ef..a2d0a196c115 100644
> > --- a/Documentation/dev-tools/ktap.rst
> > +++ b/Documentation/dev-tools/ktap.rst
> > @@ -17,7 +17,9 @@ KTAP test results describe a series of tests (which may be nested: i.e., test
> >  can have subtests), each of which can contain both diagnostic data -- e.g., log
> >  lines -- and a final result. The test structure and results are
> >  machine-readable, whereas the diagnostic data is unstructured and is there to
> > -aid human debugging.
> > +aid human debugging. One exception to this is test metadata lines - a type
> > +of diagnostic lines. Test metadata is located between the version line and
> > +plan line of a test and can be machine-readable.
> >
> >  KTAP output is built from four different types of lines:
> >  - Version lines
> > @@ -28,8 +30,7 @@ KTAP output is built from four different types of lines:
> >  In general, valid KTAP output should also form valid TAP output, but some
> >  information, in particular nested test results, may be lost. Also note that
> >  there is a stagnant draft specification for TAP14, KTAP diverges from this in
> > -a couple of places (notably the "Subtest" header), which are described where
> > -relevant later in this document.
> > +a couple of places, which are described where relevant later in this document.
> >
> >  Version lines
> >  -------------
> > @@ -166,6 +167,45 @@ even if they do not start with a "#": this is to capture any other useful
> >  kernel output which may help debug the test. It is nevertheless recommended
> >  that tests always prefix any diagnostic output they have with a "#" character.
> >
> > +Test metadata lines
> > +-------------------
> > +
> > +Test metadata lines are a type of diagnostic lines used to the declare the
> > +name of a test and other helpful testing information in the test header.
> > +These lines are often helpful for parsing and for providing context during
> > +crashes.
> > +
> > +Test metadata lines must follow the format: "# <metadata_type>: <data>".
> > +These lines must be located between the version line and the plan line
> > +within a test header.
> > +
> > +There are a few currently recognized metadata types:
> > +- "# 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
> > +
> > +As a rule, the "# Subtest:" line is generally first to declare the test
> > +name. Note that metadata lines do not necessarily need to use a
> > +recognized metadata type.
> > +
> > +An example of using metadata lines:
> > +
> > +::
> > +
> > +        KTAP version 2
> > +        1..1
> > +        # File: /sys/kernel/...
> > +          KTAP version 2
> > +          # Subtest: example
> > +          # Attributes: slow, example_test
> > +          1..1
> > +          ok 1 test_1
> > +        # example passed
> > +        ok 1 example
> > +
> > +
> >  Unknown lines
> >  -------------
> >
> > @@ -206,6 +246,7 @@ An example of a test with two nested subtests:
> >       KTAP version 2
> >       1..1
> >         KTAP version 2
> > +       # Subtest: example
> >         1..2
> >         ok 1 test_1
> >         not ok 2 test_2
> > @@ -219,6 +260,7 @@ An example format with multiple levels of nested testing:
> >       KTAP version 2
> >       1..2
> >         KTAP version 2
> > +       # Subtest: example_test_1
> >         1..2
> >           KTAP version 2
> >           1..2
> > @@ -254,6 +296,7 @@ Example KTAP output
> >       KTAP version 2
> >       1..1
> >         KTAP version 2
> > +       # Subtest: main_test
> >         1..3
> >           KTAP version 2
> >           1..1
> > @@ -261,11 +304,13 @@ Example KTAP output
> >           ok 1 test_1
> >         ok 1 example_test_1
> >           KTAP version 2
> > +            # Attributes: slow
> >           1..2
> >           ok 1 test_1 # SKIP test_1 skipped
> >           ok 2 test_2
> >         ok 2 example_test_2
> >           KTAP version 2
> > +         # Subtest: example_test_3
> >           1..3
> >           ok 1 test_1
> >           # test_2: FAIL
> >
> > base-commit: 906f02e42adfbd5ae70d328ee71656ecb602aaf5
>

Powered by blists - more mailing lists