Re: [KTAP V2 PATCH] ktap_v2: add skip test result

[Rae Moar]

On Sat, Mar 11, 2023 at 10:52 PM Frank Rowand <frowand.list@xxxxxxxxx> wrote:
>
> On 3/10/23 16:20, Rae Moar wrote:
> > Add the test result "skip" to KTAP version 2 as an alternative way to
> > indicate a test was skipped.
> >
> > The current spec uses the "#SKIP" directive to indicate that a test was
> > skipped. However, the "#SKIP" directive is not always evident when quickly
> > skimming through KTAP results.
> >
> > The "skip" result would provide an alternative that could make it clearer
> > that a test has not successfully passed because it was skipped.
> >
> > Before:
> >
> >  KTAP version 1
> >  1..1
> >    KTAP version 1
> >    1..2
> >    ok 1 case_1
> >    ok 2 case_2 #SKIP
> >  ok 1 suite
> >
> > After:
> >
> >  KTAP version 2
> >  1..1
> >    KTAP version 2
> >    1..2
> >    ok 1 case_1
> >    skip 2 case_2
> >  ok 1 suite
> >
> > Here is a link to a version of the KUnit parser that is able to parse
> > the skip test result for KTAP version 2. Note this parser is still able
> > to parse the "#SKIP" directive.
> >
> > Link: https://kunit-review.googlesource.com/c/linux/+/5689
> >
> > Signed-off-by: Rae Moar <rmoar@xxxxxxxxxx>
> > --->
> > Note: this patch is based on Frank's ktap_spec_version_2 branch.
> >
> >  Documentation/dev-tools/ktap.rst | 27 ++++++++++++++++++---------
> >  1 file changed, 18 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/ktap.rst b/Documentation/dev-tools/ktap.rst
> > index ff77f4aaa6ef..f48aa00db8f0 100644
> > --- a/Documentation/dev-tools/ktap.rst
> > +++ b/Documentation/dev-tools/ktap.rst
> > @@ -74,7 +74,8 @@ They are required and must have the format:
> >       <result> <number> [<description>][ # [<directive>] [<diagnostic data>]]
> >
> >  The result can be either "ok", which indicates the test case passed,
> > -or "not ok", which indicates that the test case failed.
> > +"not ok", which indicates that the test case failed, or "skip", which indicates
> > +the test case did not run.
> >
> >  <number> represents the number of the test being performed. The first test must
> >  have the number 1 and the number then must increase by 1 for each additional
> > @@ -91,12 +92,13 @@ A directive is a keyword that indicates a different outcome for a test other
> >  than passed and failed. The directive is optional, and consists of a single
> >  keyword preceding the diagnostic data. In the event that a parser encounters
> >  a directive it doesn't support, it should fall back to the "ok" / "not ok"
> > -result.
> > +/ "skip" result.
> >
> >  Currently accepted directives are:
> >
> > -- "SKIP", which indicates a test was skipped (note the result of the test case
> > -  result line can be either "ok" or "not ok" if the SKIP directive is used)
>
> > +- "SKIP", which indicates a test was skipped (note this is an alternative to
> > +  the "skip" result type and if the SKIP directive is used, the
> > +  result can be any type - "ok", "not ok", or "skip")
>
> For the "SKIP" directive, result type of either "ok", or "not ok" reflects the
> current real world usage, which is mixed.  I agree is makes sense to also
> allow the result type of "skip" with the "SKIP directive.
>
> I think it would be good to deprecate the "SKIP" directive, with a scheduled
> removal in the V3 specification - that would allow plenty of time for test
> parsers to process both V1 and V2 data, before removing processing of V1 data.
>
> If so, the deprecation plan should be documented.
>

Hi Frank!

This is a great point. I think it is necessary to add specifications
on how the SKIP directive will be deprecated. I will be taking all of
these suggestions when I make a v2.

Also, just letting you know I am planning on sending out two more KTAP
v2 proposals in the next few days.

Thanks for your insight.
-Rae

> >  - "TODO", which indicates that a test is not expected to pass at the moment,
> >    e.g. because the feature it is testing is known to be broken. While this>    directive is inherited from TAP, its use in the kernel is discouraged.
> > @@ -110,7 +112,7 @@ Currently accepted directives are:
> >
> >  The diagnostic data is a plain-text field which contains any additional details
> >  about why this result was produced. This is typically an error message for ERROR
> > -or failed tests, or a description of missing dependencies for a SKIP result.
> > +or failed tests, or a description of missing dependencies for a skipped test.
> >
> >  The diagnostic data field is optional, and results which have neither a
> >  directive nor any diagnostic data do not need to include the "#" field
> > @@ -130,11 +132,18 @@ The test "test_case_name" failed.
> >
> >  ::
> >
> > -     ok 1 test # SKIP necessary dependency unavailable
> > +     skip 1 test # necessary dependency unavailable
>
> Maybe add a note that the "skip" result method is preferred over the below
> "ok ... # SKIP..." example below.
>

Will add this to v2.

> >
> > -The test "test" was SKIPPED with the diagnostic message "necessary dependency
> > +The test "test" was skipped with the diagnostic message "necessary dependency
> >  unavailable".
> >
> > +::
> > +
> > +     ok 1 test_2 # SKIP this test should not run
> > +
> > +The test "test_2" was skipped with the diagnostic message "this test
> > +should not run".
>
> Maybe add a deprecation note here.
>

WIll add this to v2.

> > +
> >  ::
> >
> >       not ok 1 test # TIMEOUT 30 seconds
> > @@ -225,7 +234,7 @@ An example format with multiple levels of nested testing:
> >           not ok 1 test_1
> >           ok 2 test_2
> >         not ok 1 test_3
> > -       ok 2 test_4 # SKIP
> > +       skip 2 test_4
> >       not ok 1 example_test_1
> >       ok 2 example_test_2
> >
> > @@ -262,7 +271,7 @@ Example KTAP output
> >         ok 1 example_test_1
> >           KTAP version 2
> >           1..2
> > -         ok 1 test_1 # SKIP test_1 skipped
> > +         skip 1 test_1 # test_1 skipped
> >           ok 2 test_2
> >         ok 2 example_test_2
> >           KTAP version 2
> >
> > base-commit: 906f02e42adfbd5ae70d328ee71656ecb602aaf5
>