linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP
@ 2021-08-27 22:58 Rae Moar
  2021-08-27 22:58 ` [PATCH 1/2] selftests: tool: Add subtest header line and change indentation format in TAP results Rae Moar
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Rae Moar @ 2021-08-27 22:58 UTC (permalink / raw)
  To: brendanhiggins, davidgow, dlatypov, keescook, shuah
  Cc: kunit-dev, linux-kselftest, linux-kernel, Rae Moar

This series of patches updates the format of kselftest TAP results to improve
compatibility with the proposed KTAP specification
(https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/).

Three changes:
- Change from "# " to "  " for indentation of nested tests
- Add subtest header line at start of tests with subtests. Line format
is "# Subtest: [name of test]".
- Remove TAP header in nested tests

Standardizing TAP results would not only allow for clearer documentation and ease of reading but by standardizing the format across different testing frameworks, we could also share the use of tools.

As an example:

This is a truncated version of TAP results from the kselftest ptrace with the new format changes:

TAP version 13
1..1
# selftests: ptrace: get_syscall_info
  # Subtest: selftests: ptrace: get_syscall_info
  1..1
  # Starting 1 tests from 1 test cases.
  #  RUN           global.get_syscall_info ...
  #            OK  global.get_syscall_info
  ok 1 global.get_syscall_info
  # PASSED: 1 / 1 tests passed.
  # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 1 selftests: ptrace: get_syscall_info

With the new patch to update the KUnit parser to improve compatibility with the proposed KTAP specification, (https://lore.kernel.org/linux-kselftest/20210826195505.3066755-1-rmoar@google.com/) the above TAP results would be parsed as the following:

[20:46:09] ============================================================
[20:46:09] ===== selftests: ptrace: get_syscall_info (1 subtest) ======
[20:46:09] [PASSED] global.get_syscall_info
[20:46:09] ======= [PASSED] selftests: ptrace: get_syscall_info =======
[20:46:09] ============================================================
[20:46:09] Testing complete. Passed: 1, Failed: 0, Crashed: 0, Skipped: 0, Errors: 0

Thus, the kunit parser could become a useful tool for kselftest users.

Rae Moar (2):
  selftests: tool: Add subtest header line and change indentation format
    in TAP results
  Revert "selftests: Remove KSFT_TAP_LEVEL"

 tools/testing/selftests/Makefile            | 6 ++++++
 tools/testing/selftests/kselftest/prefix.pl | 2 +-
 tools/testing/selftests/kselftest/runner.sh | 7 ++++---
 3 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.33.0.259.gc128427fd7-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] selftests: tool: Add subtest header line and change indentation format in TAP results
  2021-08-27 22:58 [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Rae Moar
@ 2021-08-27 22:58 ` Rae Moar
  2021-08-28  6:26   ` Kees Cook
  2021-08-27 22:58 ` [PATCH 2/2] Revert "selftests: Remove KSFT_TAP_LEVEL" Rae Moar
  2021-08-28  6:10 ` [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Kees Cook
  2 siblings, 1 reply; 7+ messages in thread
From: Rae Moar @ 2021-08-27 22:58 UTC (permalink / raw)
  To: brendanhiggins, davidgow, dlatypov, keescook, shuah
  Cc: kunit-dev, linux-kselftest, linux-kernel, Rae Moar

This patch is part of a series to alter the format of kselftest TAP
results to improve compatibility with proposed KTAP specification
(https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/).

Two changes:
- Change from "# " to "  " for indentation of nested tests
- Add subtest header line at start of tests with subtests. Line format
is "# Subtest: [name of test]".

An example of the new format:

Old format:

 TAP version 13
 1..1
 # TAP version 13
 # 1..1
 # # Starting 1 tests from 1 test cases.
 # #  RUN           global.get_syscall_info ...
 # #            OK  global.get_syscall_info
 # ok 1 global.get_syscall_info
 # # PASSED: 1 / 1 tests passed.
 # # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
 ok 1 selftests: ptrace: get_syscall_info

New format:

TAP version 13
1..1
  # Subtest: selftests: ptrace: get_syscall_info
  TAP version 13
  1..1
  # Starting 1 tests from 1 test cases.
  #  RUN           global.get_syscall_info ...
  #            OK  global.get_syscall_info
  ok 1 global.get_syscall_info
  # PASSED: 1 / 1 tests passed.
  # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 1 selftests: ptrace: get_syscall_info

Signed-off-by: Rae Moar <rmoar@google.com>
Change-Id: I139774745310ad2cd6dc5d7740254e48d8226241
---
 tools/testing/selftests/kselftest/prefix.pl | 2 +-
 tools/testing/selftests/kselftest/runner.sh | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kselftest/prefix.pl b/tools/testing/selftests/kselftest/prefix.pl
index 12a7f4ca2684..e59374b62603 100755
--- a/tools/testing/selftests/kselftest/prefix.pl
+++ b/tools/testing/selftests/kselftest/prefix.pl
@@ -16,7 +16,7 @@ while (1) {
 	my $bytes = sysread(STDIN, $char, 1);
 	exit 0 if ($bytes == 0);
 	if ($needed) {
-		print "# ";
+		print "  ";
 		$needed = 0;
 	}
 	print $char;
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index cc9c846585f0..9b04aeb26d3a 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -23,7 +23,7 @@ fi
 tap_prefix()
 {
 	if [ ! -x /usr/bin/perl ]; then
-		sed -e 's/^/# /'
+		sed -e 's/^/  /'
 	else
 		"$BASE_DIR"/kselftest/prefix.pl
 	fi
@@ -75,7 +75,8 @@ run_one()
 		echo "not ok $test_num $TEST_HDR_MSG"
 	else
 		cd `dirname $TEST` > /dev/null
-		((((( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) |
+		(echo "  # Subtest: selftests: $DIR: $BASENAME_TEST" &&
+		(((( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) |
 			tap_prefix >&4) 3>&1) |
 			(read xs; exit $xs)) 4>>"$logfile" &&
 		echo "ok $test_num $TEST_HDR_MSG") ||
@@ -83,7 +84,6 @@ run_one()
 		if [ $rc -eq $skip_rc ]; then	\
 			echo "ok $test_num $TEST_HDR_MSG # SKIP"
 		elif [ $rc -eq $timeout_rc ]; then \
-			echo "#"
 			echo "not ok $test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
 		else
 			echo "not ok $test_num $TEST_HDR_MSG # exit=$rc"
-- 
2.33.0.259.gc128427fd7-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] Revert "selftests: Remove KSFT_TAP_LEVEL"
  2021-08-27 22:58 [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Rae Moar
  2021-08-27 22:58 ` [PATCH 1/2] selftests: tool: Add subtest header line and change indentation format in TAP results Rae Moar
@ 2021-08-27 22:58 ` Rae Moar
  2021-08-28  6:29   ` Kees Cook
  2021-08-28  6:10 ` [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Kees Cook
  2 siblings, 1 reply; 7+ messages in thread
From: Rae Moar @ 2021-08-27 22:58 UTC (permalink / raw)
  To: brendanhiggins, davidgow, dlatypov, keescook, shuah
  Cc: kunit-dev, linux-kselftest, linux-kernel, Rae Moar

This reverts commit f41c322f17ec4aa809222dc352439d80862c175b:
https://lore.kernel.org/linux-kselftest/20190424231236.aWGsEs-_2b6p3DpN3b_4U1xGERmHSv45uBzgjf6RIRk@z/

This patch removes nested TAP headers in kselftets TAP results and is part
of a series to alter the format of kselftest TAP results to improve
compatibility with proposed KTAP specification
(https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/).

Signed-off-by: Rae Moar <rmoar@google.com>
Change-Id: I24e74cacfc49a90a068eb30ee1448c097de5297d
---
 tools/testing/selftests/Makefile            | 6 ++++++
 tools/testing/selftests/kselftest/runner.sh | 1 +
 2 files changed, 7 insertions(+)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index fb010a35d61a..3bbfb83e2252 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -122,6 +122,12 @@ else
 	endif
 endif
 
+# KSFT_TAP_LEVEL is used from KSFT framework to prevent nested TAP header
+# printing from tests. Applicable to run_tests case where run_tests adds
+# TAP header prior running tests and when a test program invokes another
+# with system() call. Export it here to cover override RUN_TESTS defines.
+export KSFT_TAP_LEVEL=`echo 1`
+
 # Prepare for headers install
 top_srcdir ?= ../../..
 include $(top_srcdir)/scripts/subarch.include
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 9b04aeb26d3a..40ce901cb38d 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0
 #
 # Runs a set of tests in a given subdirectory.
+export KSFT_TAP_LEVEL=1
 export skip_rc=4
 export timeout_rc=124
 export logfile=/dev/stdout
-- 
2.33.0.259.gc128427fd7-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP
  2021-08-27 22:58 [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Rae Moar
  2021-08-27 22:58 ` [PATCH 1/2] selftests: tool: Add subtest header line and change indentation format in TAP results Rae Moar
  2021-08-27 22:58 ` [PATCH 2/2] Revert "selftests: Remove KSFT_TAP_LEVEL" Rae Moar
@ 2021-08-28  6:10 ` Kees Cook
  2021-08-31  2:26   ` Rae Moar
  2 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2021-08-28  6:10 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, dlatypov, shuah, kunit-dev,
	linux-kselftest, linux-kernel

On Fri, Aug 27, 2021 at 10:58:10PM +0000, Rae Moar wrote:
> This series of patches updates the format of kselftest TAP results to improve
> compatibility with the proposed KTAP specification
> (https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/).
> 
> Three changes:
> - Change from "# " to "  " for indentation of nested tests

This seems like subtests would need to have a known global nesting
tracking again. That was removed to simplified various aspects of
kselftest.

> - Add subtest header line at start of tests with subtests. Line format
> is "# Subtest: [name of test]".

That would mean subtests are no longer individually parseable by
existing TAP parsers (i.e. when LAVA splits up kselftest output,
suddenly there is no TAP header).

> - Remove TAP header in nested tests

What benefit does this provide? This just makes things harder to machine
parse now.

> Standardizing TAP results would not only allow for clearer documentation and ease of reading but by standardizing the format across different testing frameworks, we could also share the use of tools.
> 
> As an example:
> 
> This is a truncated version of TAP results from the kselftest ptrace with the new format changes:
> 
> TAP version 13
> 1..1
> # selftests: ptrace: get_syscall_info
>   # Subtest: selftests: ptrace: get_syscall_info
>   1..1
>   # Starting 1 tests from 1 test cases.
>   #  RUN           global.get_syscall_info ...
>   #            OK  global.get_syscall_info
>   ok 1 global.get_syscall_info
>   # PASSED: 1 / 1 tests passed.
>   # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
> ok 1 selftests: ptrace: get_syscall_info

But this would mean changing every TAP parser to notice "# Subtest:"
instead of "TAP version ..." Why bother doing that? A parser aware of
subtests just needs to examine indentation level currently.

> 
> With the new patch to update the KUnit parser to improve compatibility with the proposed KTAP specification, (https://lore.kernel.org/linux-kselftest/20210826195505.3066755-1-rmoar@google.com/) the above TAP results would be parsed as the following:
> 
> [20:46:09] ============================================================
> [20:46:09] ===== selftests: ptrace: get_syscall_info (1 subtest) ======
> [20:46:09] [PASSED] global.get_syscall_info
> [20:46:09] ======= [PASSED] selftests: ptrace: get_syscall_info =======
> [20:46:09] ============================================================
> [20:46:09] Testing complete. Passed: 1, Failed: 0, Crashed: 0, Skipped: 0, Errors: 0
> 
> Thus, the kunit parser could become a useful tool for kselftest users.
> 
> Rae Moar (2):
>   selftests: tool: Add subtest header line and change indentation format
>     in TAP results
>   Revert "selftests: Remove KSFT_TAP_LEVEL"
> 
>  tools/testing/selftests/Makefile            | 6 ++++++
>  tools/testing/selftests/kselftest/prefix.pl | 2 +-
>  tools/testing/selftests/kselftest/runner.sh | 7 ++++---
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> -- 
> 2.33.0.259.gc128427fd7-goog
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] selftests: tool: Add subtest header line and change indentation format in TAP results
  2021-08-27 22:58 ` [PATCH 1/2] selftests: tool: Add subtest header line and change indentation format in TAP results Rae Moar
@ 2021-08-28  6:26   ` Kees Cook
  0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2021-08-28  6:26 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, dlatypov, shuah, kunit-dev,
	linux-kselftest, linux-kernel

On Fri, Aug 27, 2021 at 10:58:11PM +0000, Rae Moar wrote:
> This patch is part of a series to alter the format of kselftest TAP
> results to improve compatibility with proposed KTAP specification
> (https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/).
> 
> Two changes:
> - Change from "# " to "  " for indentation of nested tests
> - Add subtest header line at start of tests with subtests. Line format
> is "# Subtest: [name of test]".
> 
> An example of the new format:
> 
> Old format:
> 
>  TAP version 13
>  1..1
>  # TAP version 13
>  # 1..1
>  # # Starting 1 tests from 1 test cases.
>  # #  RUN           global.get_syscall_info ...
>  # #            OK  global.get_syscall_info
>  # ok 1 global.get_syscall_info
>  # # PASSED: 1 / 1 tests passed.
>  # # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
>  ok 1 selftests: ptrace: get_syscall_info
> 
> New format:
> 
> TAP version 13
> 1..1
>   # Subtest: selftests: ptrace: get_syscall_info
>   TAP version 13
>   1..1
>   # Starting 1 tests from 1 test cases.
>   #  RUN           global.get_syscall_info ...
>   #            OK  global.get_syscall_info
>   ok 1 global.get_syscall_info
>   # PASSED: 1 / 1 tests passed.
>   # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
> ok 1 selftests: ptrace: get_syscall_info
> 
> Signed-off-by: Rae Moar <rmoar@google.com>
> Change-Id: I139774745310ad2cd6dc5d7740254e48d8226241
> ---
>  tools/testing/selftests/kselftest/prefix.pl | 2 +-
>  tools/testing/selftests/kselftest/runner.sh | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest/prefix.pl b/tools/testing/selftests/kselftest/prefix.pl
> index 12a7f4ca2684..e59374b62603 100755
> --- a/tools/testing/selftests/kselftest/prefix.pl
> +++ b/tools/testing/selftests/kselftest/prefix.pl
> @@ -16,7 +16,7 @@ while (1) {
>  	my $bytes = sysread(STDIN, $char, 1);
>  	exit 0 if ($bytes == 0);
>  	if ($needed) {
> -		print "# ";
> +		print "  ";
>  		$needed = 0;
>  	}
>  	print $char;
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index cc9c846585f0..9b04aeb26d3a 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -23,7 +23,7 @@ fi
>  tap_prefix()
>  {
>  	if [ ! -x /usr/bin/perl ]; then
> -		sed -e 's/^/# /'
> +		sed -e 's/^/  /'
>  	else
>  		"$BASE_DIR"/kselftest/prefix.pl
>  	fi

Some of the existing TAP parsers expect leading spaces to be parsed as
YAML (from the _preceeding_ test result). "#" is understood to be
diagnostics about the currently running test.

Since there's no "---" nor "..." here, and the other parsers were pretty
busted to try to parse YAML without those, I can be convinced that
diagnostics can be marked with a leading " ", but if that's true, why
not just drop all "#" usage? Then we'd have:

TAP version 13
1..1
  # Subtest: selftests: ptrace: get_syscall_info
  TAP version 13
  1..1
    Starting 1 tests from 1 test cases.
     RUN           global.get_syscall_info ...
               OK  global.get_syscall_info
  ok 1 global.get_syscall_info
    PASSED: 1 / 1 tests passed.
    Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 1 selftests: ptrace: get_syscall_info

> @@ -75,7 +75,8 @@ run_one()
>  		echo "not ok $test_num $TEST_HDR_MSG"
>  	else
>  		cd `dirname $TEST` > /dev/null
> -		((((( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) |
> +		(echo "  # Subtest: selftests: $DIR: $BASENAME_TEST" &&
> +		(((( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) |

What's the benefit here? All I see is that now the test and the runner
are once again mixing the responsibility of generating the test output.
The subtest should be able to run strictly independently.

>  			tap_prefix >&4) 3>&1) |
>  			(read xs; exit $xs)) 4>>"$logfile" &&
>  		echo "ok $test_num $TEST_HDR_MSG") ||
> @@ -83,7 +84,6 @@ run_one()
>  		if [ $rc -eq $skip_rc ]; then	\
>  			echo "ok $test_num $TEST_HDR_MSG # SKIP"
>  		elif [ $rc -eq $timeout_rc ]; then \
> -			echo "#"

NAK: this is ensuring a newline before the "not ok" line on a timeout,
where the test output may have been interrupted before printing a
newline. (i.e. unflushed progress report style output)

>  			echo "not ok $test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
>  		else
>  			echo "not ok $test_num $TEST_HDR_MSG # exit=$rc"

-Kees

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] Revert "selftests: Remove KSFT_TAP_LEVEL"
  2021-08-27 22:58 ` [PATCH 2/2] Revert "selftests: Remove KSFT_TAP_LEVEL" Rae Moar
@ 2021-08-28  6:29   ` Kees Cook
  0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2021-08-28  6:29 UTC (permalink / raw)
  To: Rae Moar
  Cc: brendanhiggins, davidgow, dlatypov, shuah, kunit-dev,
	linux-kselftest, linux-kernel

On Fri, Aug 27, 2021 at 10:58:12PM +0000, Rae Moar wrote:
> This reverts commit f41c322f17ec4aa809222dc352439d80862c175b:
> https://lore.kernel.org/linux-kselftest/20190424231236.aWGsEs-_2b6p3DpN3b_4U1xGERmHSv45uBzgjf6RIRk@z/
> 
> This patch removes nested TAP headers in kselftets TAP results and is part
> of a series to alter the format of kselftest TAP results to improve
> compatibility with proposed KTAP specification
> (https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/).

That would have been nice to have been CCed on. :)

-Kees

> 
> Signed-off-by: Rae Moar <rmoar@google.com>
> Change-Id: I24e74cacfc49a90a068eb30ee1448c097de5297d
> ---
>  tools/testing/selftests/Makefile            | 6 ++++++
>  tools/testing/selftests/kselftest/runner.sh | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index fb010a35d61a..3bbfb83e2252 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -122,6 +122,12 @@ else
>  	endif
>  endif
>  
> +# KSFT_TAP_LEVEL is used from KSFT framework to prevent nested TAP header
> +# printing from tests. Applicable to run_tests case where run_tests adds
> +# TAP header prior running tests and when a test program invokes another
> +# with system() call. Export it here to cover override RUN_TESTS defines.
> +export KSFT_TAP_LEVEL=`echo 1`
> +
>  # Prepare for headers install
>  top_srcdir ?= ../../..
>  include $(top_srcdir)/scripts/subarch.include
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index 9b04aeb26d3a..40ce901cb38d 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -2,6 +2,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  #
>  # Runs a set of tests in a given subdirectory.
> +export KSFT_TAP_LEVEL=1
>  export skip_rc=4
>  export timeout_rc=124
>  export logfile=/dev/stdout
> -- 
> 2.33.0.259.gc128427fd7-goog
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP
  2021-08-28  6:10 ` [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Kees Cook
@ 2021-08-31  2:26   ` Rae Moar
  0 siblings, 0 replies; 7+ messages in thread
From: Rae Moar @ 2021-08-31  2:26 UTC (permalink / raw)
  To: Kees Cook
  Cc: brendanhiggins, davidgow, dlatypov, shuah, kunit-dev,
	linux-kselftest, linux-kernel

These changes to the format of TAP results for kselftests are
based on this proposed specification of KTAP:
https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/

However, there has been considerable discussion on this proposal recently.
It is great to see this discourse on the specification KTAP. However,
the changes to the format of TAP results for kselftests in this series of
patches should be paused until further agreement on the exact specification
of KTAP.

- Rae

On Sat, Aug 28, 2021 at 2:10 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Aug 27, 2021 at 10:58:10PM +0000, Rae Moar wrote:
> > This series of patches updates the format of kselftest TAP results to improve
> > compatibility with the proposed KTAP specification
> > (https://lore.kernel.org/linux-kselftest/CA+GJov6tdjvY9x12JsJT14qn6c7NViJxqaJk+r-K1YJzPggFDQ@mail.gmail.com/).
> >
> > Three changes:
> > - Change from "# " to "  " for indentation of nested tests
>
> This seems like subtests would need to have a known global nesting
> tracking again. That was removed to simplified various aspects of
> kselftest.
>
> > - Add subtest header line at start of tests with subtests. Line format
> > is "# Subtest: [name of test]".
>
> That would mean subtests are no longer individually parseable by
> existing TAP parsers (i.e. when LAVA splits up kselftest output,
> suddenly there is no TAP header).
>
> > - Remove TAP header in nested tests
>
> What benefit does this provide? This just makes things harder to machine
> parse now.
>
> > Standardizing TAP results would not only allow for clearer documentation and ease of reading but by standardizing the format across different testing frameworks, we could also share the use of tools.
> >
> > As an example:
> >
> > This is a truncated version of TAP results from the kselftest ptrace with the new format changes:
> >
> > TAP version 13
> > 1..1
> > # selftests: ptrace: get_syscall_info
> >   # Subtest: selftests: ptrace: get_syscall_info
> >   1..1
> >   # Starting 1 tests from 1 test cases.
> >   #  RUN           global.get_syscall_info ...
> >   #            OK  global.get_syscall_info
> >   ok 1 global.get_syscall_info
> >   # PASSED: 1 / 1 tests passed.
> >   # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
> > ok 1 selftests: ptrace: get_syscall_info
>
> But this would mean changing every TAP parser to notice "# Subtest:"
> instead of "TAP version ..." Why bother doing that? A parser aware of
> subtests just needs to examine indentation level currently.
>
> >
> > With the new patch to update the KUnit parser to improve compatibility with the proposed KTAP specification, (https://lore.kernel.org/linux-kselftest/20210826195505.3066755-1-rmoar@google.com/) the above TAP results would be parsed as the following:
> >
> > [20:46:09] ============================================================
> > [20:46:09] ===== selftests: ptrace: get_syscall_info (1 subtest) ======
> > [20:46:09] [PASSED] global.get_syscall_info
> > [20:46:09] ======= [PASSED] selftests: ptrace: get_syscall_info =======
> > [20:46:09] ============================================================
> > [20:46:09] Testing complete. Passed: 1, Failed: 0, Crashed: 0, Skipped: 0, Errors: 0
> >
> > Thus, the kunit parser could become a useful tool for kselftest users.
> >
> > Rae Moar (2):
> >   selftests: tool: Add subtest header line and change indentation format
> >     in TAP results
> >   Revert "selftests: Remove KSFT_TAP_LEVEL"
> >
> >  tools/testing/selftests/Makefile            | 6 ++++++
> >  tools/testing/selftests/kselftest/prefix.pl | 2 +-
> >  tools/testing/selftests/kselftest/runner.sh | 7 ++++---
> >  3 files changed, 11 insertions(+), 4 deletions(-)
> >
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
>
> --
> Kees Cook

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-31  2:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 22:58 [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Rae Moar
2021-08-27 22:58 ` [PATCH 1/2] selftests: tool: Add subtest header line and change indentation format in TAP results Rae Moar
2021-08-28  6:26   ` Kees Cook
2021-08-27 22:58 ` [PATCH 2/2] Revert "selftests: Remove KSFT_TAP_LEVEL" Rae Moar
2021-08-28  6:29   ` Kees Cook
2021-08-28  6:10 ` [PATCH 0/2] selftests: tool: update format of kselftest TAP results to improve compatibility with KTAP Kees Cook
2021-08-31  2:26   ` Rae Moar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).