diff mbox series

[1/2] selftests: tool: Add subtest header line and change indentation format in TAP results

Message ID 20210827225812.3247919-2-rmoar@google.com
State New
Headers show
Series [1/2] selftests: tool: Add subtest header line and change indentation format in TAP results | expand

Commit Message

Rae Moar Aug. 27, 2021, 10:58 p.m. UTC
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(-)

Comments

Kees Cook Aug. 28, 2021, 6:26 a.m. UTC | #1
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
diff mbox series

Patch

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"