[KTAP V2 PATCH] ktap_v2: add skip test result

[KTAP V2 PATCH] ktap_v2: add skip test result

[Rae Moar]

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@google.com>
---

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")
 - "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
 
-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".
+
 ::
 
 	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
-- 
2.40.0.rc1.284.g88254d51c5-goog

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

[Bird, Tim]

> -----Original Message-----
> From: Rae Moar <rmoar@google.com>
> 
> 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.
> 

Is there an implementation patch (RFC or otherwise) that accompanies
this change in the spec?

Also, can you tell me which kselftest modules you expect to use this
new 'skip' result, as opposed to the #SKIP directive?  Are there patches
pending submission that already use this?

Which in-tree and out-of-tree results parsers would be affected?

I know my Fuego kselftest results parser would be affected.

While I recognize the slight improvement in human readability, this 
will cause a fair amount of churn.  And it takes us out of TAP compliance.
Can you quantify the churn a bit?

 -- Tim

> 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@google.com>
> ---
> 
> 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")
>  - "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
> 
> -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".
> +
>  ::
> 
>  	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
> --
> 2.40.0.rc1.284.g88254d51c5-goog

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

[Frank Rowand]

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.

< snip >

General information about the KTAP Specification version 2 process and progress
can be found at:

   https://elinux.org/Test_Results_Format_Notes#KTAP_version_2

This patch has been added to that page.

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

[Frank Rowand]

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@google.com>
> ---

< snip >

Another reason to add the "skip" result is that there was disagreement in previous
discussions as to whether the "#SKIP" directive should be used in an "ok" result
or a "not_ok" result.

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

[Frank Rowand]

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@google.com>
> ---> 
> 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.

>  - "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.

>  
> -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.

> +
>  ::
>  
>  	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

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

[Frank Rowand]

On 3/11/23 11:36, Bird, Tim wrote:
> 
> 
>> -----Original Message-----
>> From: Rae Moar <rmoar@google.com>
>>
>> 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.
>>
> 
> Is there an implementation patch (RFC or otherwise) that accompanies
> this change in the spec?
> 
> Also, can you tell me which kselftest modules you expect to use this
> new 'skip' result, as opposed to the #SKIP directive?  Are there patches
> pending submission that already use this?

All test should eventually be changed to the 'skip' result from the #SKIP
directive.  You asked about kselftest, but I'll comment about kunit because
I used to know more about the kunit infrastructure.  I suspect the kunit
skip related wrapper could be modified to make this change for all kunit
tests.

> 
> Which in-tree and out-of-tree results parsers would be affected?
> 
> I know my Fuego kselftest results parser would be affected.

It is inevitable that all parsers that process KTAP V1 will be affected
by KTAP v2.  It would be desirable for the changes made in V2 allow
the updated parsers to process both V1 compliant data and V2 compliant
data.  After allowing plenty of time for V1 compliant data to disappear,
the V3 Specification could possible obsolete some V1 constructs.  I do
not expect a V3 Specification to appear anytime in the forseeable
future, given the slow pace of creating V2.

> 
> While I recognize the slight improvement in human readability, this 
> will cause a fair amount of churn.  And it takes us out of TAP compliance.
> Can you quantify the churn a bit?

I would say that KTAP V1 is already out of TAP compliance.  And there were
test implementations that moved to use some of the never completed TAP
version 14 format, thus being out of compliance with TAP.

-Frank

> 
>  -- Tim
> 
>> 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@google.com>
>> ---
>>
>> 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")
>>  - "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
>>
>> -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".
>> +
>>  ::
>>
>>  	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
>> --
>> 2.40.0.rc1.284.g88254d51c5-goog
> 

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

[Frank Rowand]

On 3/11/23 21:52, Frank Rowand 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@google.com>
>> ---> 
>> 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.

Since I wrote that paragraph, I have pondered the process of transition from
V1 to V2, to possibly V3.  It seems to be a complex enough issue that I will
start a different email thread to gather thoughts, issues, and possible
directions.

-Frank

> 
> If so, the deprecation plan should be documented.
> 
>>  - "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.
> 
>>  
>> -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.
> 
>> +
>>  ::
>>  
>>  	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
> 

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

[Rae Moar]

On Sat, Mar 11, 2023 at 12:37 PM Bird, Tim <Tim.Bird@sony.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Rae Moar <rmoar@google.com>
> >
> > 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.
> >
>
> Is there an implementation patch (RFC or otherwise) that accompanies
> this change in the spec?

Hi Tim!

Other than the KUnit parser implementation I linked in the commit
message, there is no current implementation patch that accompanies
this proposal. I was very curious to learn what others thought of the
idea of the skip result. An implementation patch should definitely be
created to implement the necessary changes to kselftest output and a
few commonly used parsers.

>
> Also, can you tell me which kselftest modules you expect to use this
> new 'skip' result, as opposed to the #SKIP directive?  Are there patches
> pending submission that already use this?

There are no current patches in my knowledge that are proposing to use
this. This idea partly stems from your suggestion from the KTAP v1
discussions where you proposed an unknown test result type:
https://lore.kernel.org/all/BYAPR13MB25037E7EE38DE8717DC7D254FDCB9@BYAPR13MB2503.namprd13.prod.outlook.com/.
I would be open to this suggestion as an alternative.

>
> Which in-tree and out-of-tree results parsers would be affected?
>
> I know my Fuego kselftest results parser would be affected.

I honestly have much to learn about different results parsers. I
suspect every parser in use would be affected, except for those that
only care about failures and simply grep for "not ok". Their results
might actually be more clear by not including skipped tests. I will
continue to do more research on this and explore this in a potential
implementation patch. I ask everyone to feel free to correct or
enlighten me about the different parsers they use.

>
> While I recognize the slight improvement in human readability, this
> will cause a fair amount of churn.  And it takes us out of TAP compliance.
> Can you quantify the churn a bit?

I do realize this would create quite a bit of churn and if people
think it is not worth the extra effort I would understand that. But
thinking towards the future of KTAP, I suspect we will eventually want
to shift away from using the SKIP directive as it is inherently
confusing to allow multiple result types with the directive, as Frank
mentioned. It might be a question of when we want to make this shift?

I find it difficult to specifically quantify the churn. Looking at a
LKFT build on linaro about 11% of tests were skipped and that was
widespread across different types of tests. So skipped tests are
certainly in widespread use. However, I suspect the actual changes to
the code that creates kselftest output and for each parser would not
be too difficult. But it would require parsers that did not currently
care about skipped tests to decide how to handle the new result.

One thing to note on the created churn: I have noticed a proportion of
kselftests currently implement skipped tests in a way that does not
use the SKIP directive. They use a comment of the format "# [SKIP]"
prior to a test result line with no SKIP directive. Thus, in order to
reach KTAP compliance the way skip tests are handled would need to be
changed in these cases anyways.

Thanks!
Rae


>
>  -- Tim
>
> > 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@google.com>
> > ---
> >
> > 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")
> >  - "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
> >
> > -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".
> > +
> >  ::
> >
> >       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
> > --
> > 2.40.0.rc1.284.g88254d51c5-goog
>

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@gmail.com> 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@google.com>
> > --->
> > 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
>

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

On Tue, Mar 14, 2023 at 06:03:59PM -0400, Rae Moar via groups.io wrote:

> One thing to note on the created churn: I have noticed a proportion of
> kselftests currently implement skipped tests in a way that does not
> use the SKIP directive. They use a comment of the format "# [SKIP]"
> prior to a test result line with no SKIP directive. Thus, in order to
> reach KTAP compliance the way skip tests are handled would need to be
> changed in these cases anyways.

This is the documented way of reporting a skip in KTAP:

   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/ktap.rst#n97

TBH I'm finding it really hard to summon much enthusiasm for changing
this except as part of some other incompatible update - the current
format isn't ideal but deploying a change would be a bunch of hassle for
the existing test automation systems.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Guillaume Tucker]

+ kernelci@lists.linux.dev

Please note: kernelci@groups.io is not used any more, see details
here about the move:

  https://groups.io/g/kernelci

The new KernelCI email archives can be found here:

  https://lore.kernel.org/kernelci/

Thanks,
Guillaume


On 10/03/2023 23: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@google.com>
> ---
> 
> 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")
>  - "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
>  
> -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".
> +
>  ::
>  
>  	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

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

[Frank Rowand]

On 3/15/23 07:53, Mark Brown wrote:
> On Tue, Mar 14, 2023 at 06:03:59PM -0400, Rae Moar via groups.io wrote:
> 
>> One thing to note on the created churn: I have noticed a proportion of
>> kselftests currently implement skipped tests in a way that does not
>> use the SKIP directive. They use a comment of the format "# [SKIP]"
>> prior to a test result line with no SKIP directive. Thus, in order to
>> reach KTAP compliance the way skip tests are handled would need to be
>> changed in these cases anyways.
> 
> This is the documented way of reporting a skip in KTAP:
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/ktap.rst#n97
> 
> TBH I'm finding it really hard to summon much enthusiasm for changing
> this except as part of some other incompatible update - the current
> format isn't ideal but deploying a change would be a bunch of hassle for
> the existing test automation systems.

Yes, there is no need to do a single specification change that results
in incompatibility.  But given the previous discussions there seem to
be plenty of other desired changes that will result in incompatibility.

My desire is to take our time to capture as much of the desired changes
as possible in version 2 of the specification.  And an expectation that
there will not need to be a version 3 of the specification for many years.
I will support not rushing version 2 of the specification so that this
goal can be realized.

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 334 bytes --]

On Wed, Mar 15, 2023 at 04:45:29PM -0500, Frank Rowand wrote:

> Yes, there is no need to do a single specification change that results
> in incompatibility.  But given the previous discussions there seem to
> be plenty of other desired changes that will result in incompatibility.

Do you have a pointer to that previous discussion?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Frank Rowand]

On 3/16/23 06:33, Mark Brown wrote:
> On Wed, Mar 15, 2023 at 04:45:29PM -0500, Frank Rowand wrote:
> 
>> Yes, there is no need to do a single specification change that results
>> in incompatibility.  But given the previous discussions there seem to
>> be plenty of other desired changes that will result in incompatibility.
> 
> Do you have a pointer to that previous discussion?

There are links to a few threads at:

   https://elinux.org/Test_Results_Format_Notes#KTAP_version_1

And I am tracking KTAP Specification version 2 activity in the
next section of that web page (not much yet, but hopefully
becoming more active).

-Frank

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

[Frank Rowand]

On 3/13/23 09:41, Frank Rowand wrote:
> On 3/11/23 21:52, Frank Rowand 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@google.com>
>>> ---> 
>>> 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.
> 
> Since I wrote that paragraph, I have pondered the process of transition from
> V1 to V2, to possibly V3.  It seems to be a complex enough issue that I will
> start a different email thread to gather thoughts, issues, and possible
> directions.

The new thread is now started at:

  https://lore.kernel.org/all/6d4afb49-3cb9-f176-61a2-5bbaab698644@gmail.com/T/#u

-Frank

> 
> -Frank
> 
>>
>> If so, the deprecation plan should be documented.
>>
>>>  - "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.
>>
>>>  
>>> -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.
>>
>>> +
>>>  ::
>>>  
>>>  	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
>>
>