[PATCH 0/5] Handling for TAP header level

[PATCH 0/5] Handling for TAP header level

[Shuah Khan]

This patch series is a result discussion with Tim Bird about nested TAP
header handling. Based on the discussion, I am introducing a environment
variable to prevent nested TAP headers. These patches improve the run_tests
output and the output from the script generated by emit_tests.
    
This first patch in this series adds environment variable KSFT_TAP_LEVEL
to avoid printing nested TAP headers for each test. lib.mk run_tests
target prints TAP header before invoking the test program or test script.
Tests need a way to suppress TAP headers if it is already printed out.

This new environment variable adds a way for ksft_print_header()
print TAP header only when KSFT_TAP_LEVEL isn't set.
  
The second patch in this series changes lib.mk run_tests target to set
KSFT_TAP_LEVEL before running tests.

The third patch changes Makefile to export KSFT_TAP_LEVEL and adds
TAP and KSFT_TAP_LEVEL handling to emit_tests target.
 
Forth and fifth patches make changes to size and futex tests to
prevent nested TAP headers to take advantage of the framework
change in the first patch.

Shuah Khan (5):
  selftests: kselftest framework: add handling for TAP header level
  selftests: lib.mk set KSFT_TAP_LEVEL to prevent nested TAP headers
  selftests: Makefile set KSFT_TAP_LEVEL to prevent nested TAP headers
  selftests: size call ksft_print_header() to print TAP header
  selftests: futex Makefile add top level TAP header echo to RUN_TESTS

 tools/testing/selftests/Makefile        | 10 +++++++++-
 tools/testing/selftests/futex/Makefile  |  3 +++
 tools/testing/selftests/kselftest.h     |  3 ++-
 tools/testing/selftests/lib.mk          |  1 +
 tools/testing/selftests/size/get_size.c |  4 +++-
 5 files changed, 18 insertions(+), 3 deletions(-)

-- 
2.14.1

[PATCH 1/5] selftests: kselftest framework: add handling for TAP header level

[Shuah Khan]

Introduce environment variable KSFT_TAP_LEVEL to avoid printing
nested TAP headers for each test. lib.mk run_tests target prints
TAP header before invoking the test program or test script. Tests
need a way to suppress TAP headers if it is already printed out.

This new environment variable adds a way for ksft_print_header()
print TAP header only when KSFT_TAP_LEVEL isn't set.

lib.mk run_tests and test program should print TAP header and set
KSFT_TAP_LEVEL to avoid a second TAP header to be printed.

selftests Makefile should export KSFT_TAP_LEVEL and add TAP Header
echo to the run_kselftest.sh script from emit_tests target handling.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/kselftest.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index 1a52b03962a3..1b9d8ecdebce 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -57,7 +57,8 @@ static inline int ksft_get_error_cnt(void) { return ksft_cnt.ksft_error; }
 
 static inline void ksft_print_header(void)
 {
-	printf("TAP version 13\n");
+	if (!(getenv("KSFT_TAP_LEVEL")))
+		printf("TAP version 13\n");
 }
 
 static inline void ksft_print_cnts(void)
-- 
2.14.1

[PATCH 2/5] selftests: lib.mk set KSFT_TAP_LEVEL to prevent nested TAP headers

[Shuah Khan]

Set KSFT_TAP_LEVEL before running tests to prevent nested TAP header
printing from tests.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/lib.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 7de482a0519d..195e9d4739a9 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -20,6 +20,7 @@ all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
 
 .ONESHELL:
 define RUN_TESTS
+	@export KSFT_TAP_LEVEL=`echo 1`;
 	@test_num=`echo 0`;
 	@echo "TAP version 13";
 	@for TEST in $(1); do				\
-- 
2.14.1

[PATCH 3/5] selftests: Makefile set KSFT_TAP_LEVEL to prevent nested TAP headers

[Shuah Khan]

Export KSFT_TAP_LEVEL and add TAP Header echo to the run_kselftest.sh
script from emit_tests target handling.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 7442dfb73b7f..a41b4be28b9f 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -66,6 +66,12 @@ ifndef BUILD
   BUILD := $(shell pwd)
 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`
+
 export BUILD
 all:
 	@for TARGET in $(TARGETS); do		\
@@ -125,10 +131,12 @@ ifdef INSTALL_PATH
 	echo "else" >> $(ALL_SCRIPT)
 	echo "  OUTPUT=/dev/stdout" >> $(ALL_SCRIPT)
 	echo "fi" >> $(ALL_SCRIPT)
+	echo "export KSFT_TAP_LEVEL=`echo 1`" >> $(ALL_SCRIPT)
 
 	for TARGET in $(TARGETS); do \
 		BUILD_TARGET=$$BUILD/$$TARGET;	\
-		echo "echo ; echo Running tests in $$TARGET" >> $(ALL_SCRIPT); \
+		echo "echo ; echo TAP version 13" >> $(ALL_SCRIPT);	\
+		echo "echo Running tests in $$TARGET" >> $(ALL_SCRIPT); \
 		echo "echo ========================================" >> $(ALL_SCRIPT); \
 		echo "cd $$TARGET" >> $(ALL_SCRIPT); \
 		make -s --no-print-directory OUTPUT=$$BUILD_TARGET -C $$TARGET emit_tests >> $(ALL_SCRIPT); \
-- 
2.14.1

[PATCH 4/5] selftests: size call ksft_print_header() to print TAP header

[Shuah Khan]

Call ksft_print_header() to print TAP header. This change helps
prevent nested TAP headers when the test is run from run_tests
and from script generated by emit_tests as in both of these cases
KSFT_TAP_LEVEL will be set and ksft_print_header() will suppress
the nested TAP header from size test.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/size/get_size.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/size/get_size.c b/tools/testing/selftests/size/get_size.c
index d4b59ab979a0..1280b09266f0 100644
--- a/tools/testing/selftests/size/get_size.c
+++ b/tools/testing/selftests/size/get_size.c
@@ -23,6 +23,8 @@
 
 #include <sys/sysinfo.h>
 #include <unistd.h>
+#include <stdio.h>
+#include "../kselftest.h"
 
 #define STDOUT_FILENO 1
 
@@ -77,7 +79,7 @@ void _start(void)
 	unsigned long used;
 	static const char *test_name = " get runtime memory use\n";
 
-	print("TAP version 13\n");
+	ksft_print_header();
 	print("# Testing system size.\n");
 
 	ccode = sysinfo(&info);
-- 
2.14.1

[PATCH 5/5] selftests: futex Makefile add top level TAP header echo to RUN_TESTS

[Shuah Khan]

Add top level TAP header echo, testname and separator line to make
the output consistent with the common run_tests target.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/futex/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile
index cea4adcd42b8..3e9a9d7b4de1 100644
--- a/tools/testing/selftests/futex/Makefile
+++ b/tools/testing/selftests/futex/Makefile
@@ -18,6 +18,9 @@ all:
 	done
 
 override define RUN_TESTS
+	@echo "TAP version 13";
+	@echo "selftests: futex";
+	@echo "========================================";
 	@cd $(OUTPUT); ./run.sh
 endef
 
-- 
2.14.1

Re: [PATCH 0/5] Handling for TAP header level

[Ingo Molnar]

* Shuah Khan <shuahkh@osg.samsung.com> wrote:

> This patch series is a result discussion with Tim Bird about nested TAP header 
> handling. Based on the discussion, I am introducing a environment variable to 
> prevent nested TAP headers. These patches improve the run_tests output and the 
> output from the script generated by emit_tests.

Could you please provider a higher level description about what 'nested TAP header 
handling' is about and what it is good for?

Thanks,

	Ingo

Re: [PATCH 5/5] selftests: futex Makefile add top level TAP header echo to RUN_TESTS

[Darren Hart]

On Fri, Feb 23, 2018 at 03:11:40PM -0700, Shuah Khan wrote:
> Add top level TAP header echo, testname and separator line to make
> the output consistent with the common run_tests target.

No objection to the changes, but I'll echo Ingo's request for a more
detailed justification for why these changes are warranted. We've seen
this a few times with recent changes to kselftests, I think too much
might be being assumed regarding the individual test author's tracking
of the core of the kselftests ongoing work.

> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  tools/testing/selftests/futex/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile
> index cea4adcd42b8..3e9a9d7b4de1 100644
> --- a/tools/testing/selftests/futex/Makefile
> +++ b/tools/testing/selftests/futex/Makefile
> @@ -18,6 +18,9 @@ all:
>  	done
>  
>  override define RUN_TESTS
> +	@echo "TAP version 13";
> +	@echo "selftests: futex";
> +	@echo "========================================";
>  	@cd $(OUTPUT); ./run.sh
>  endef
>  
> -- 
> 2.14.1
> 
> 

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH 5/5] selftests: futex Makefile add top level TAP header echo to RUN_TESTS

[Shuah Khan]

On 02/24/2018 02:42 PM, Darren Hart wrote:
> On Fri, Feb 23, 2018 at 03:11:40PM -0700, Shuah Khan wrote:
>> Add top level TAP header echo, testname and separator line to make
>> the output consistent with the common run_tests target.
> 
> No objection to the changes, but I'll echo Ingo's request for a more
> detailed justification for why these changes are warranted. We've seen
> this a few times with recent changes to kselftests, I think too much
> might be being assumed regarding the individual test author's tracking
> of the core of the kselftests ongoing work.
> 

Ingo/Darren,

Sorry for not giving a better justification. I should have kept audience
in mind that test authors might not be up on the framework changes.

Nested TAP header example:

TAP version 13
selftests: step_after_suspend_test
========================================
TAP version 13                                           (#Nested header)
Bail out! open("/sys/power/state") failed (is this test running as root?)
Pass 0 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
1..0
not ok 1..1 selftests:  step_after_suspend_test [FAIL]
selftests: breakpoint_test
========================================
TAP version 13                                            (#Nested header)
Pass 110 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0

<snip>

1..110
ok 1..2 selftests: breakpoint_test [PASS]


Some tests print TAP headers. This is helpful when test is run by itself.
However, when "make kselftest" or "make -C" is used then common run_tests
does its TAP header and nested headers get printed.

Nested headers are a problem for TAP13 parsers. Hence, I decided to go
with a simpler approach to suppress TAP headers using env. variable
that gets checked in the framework and suppresses the header.

selftests: kselftest framework: add handling for TAP header level

This minimizes the churn to actual tests and framework handles the
TAP details.

futex test overrides the generic run_tests, so it needed changes to
add a top level TAP header. Otherwise, you will see nested TAP header
for each of the individual futxex tests as shown in the example below.


TAP version 13
# futex_requeue_pi: Test requeue functionality
#       Arguments: broadcast=0 locked=0 owner=0 timeout=0ns
not ok 1 # error futex-requeue-pi
Pass 0 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 1
1..1

I can capture the nested TAP headers being a problem for parsers in
the changelog. I will have to send v2 anyway, "make -C" still has
the nested headers for fuxtex tests.

My goal is to not have the individual test authors be concerned about
the TAP13 details and handle it in the framework. I am making changes
to framework and test Makefiles for the TAP13 and other common formatting
type features as opposed to individual test code. Test Makefiles that use
lib.mk don't need changes either, futex needs changes mainly because of
overrides.

Hope this help. I will do a better job of explaining in the future.

thanks,
-- Shuah

Re: [PATCH 5/5] selftests: futex Makefile add top level TAP header echo to RUN_TESTS

[Darren Hart]

On Tue, Feb 27, 2018 at 10:25:44AM -0700, Shuah Khan wrote:
> On 02/24/2018 02:42 PM, Darren Hart wrote:
> > On Fri, Feb 23, 2018 at 03:11:40PM -0700, Shuah Khan wrote:
> >> Add top level TAP header echo, testname and separator line to make
> >> the output consistent with the common run_tests target.
> > 
> > No objection to the changes, but I'll echo Ingo's request for a more
> > detailed justification for why these changes are warranted. We've seen
> > this a few times with recent changes to kselftests, I think too much
> > might be being assumed regarding the individual test author's tracking
> > of the core of the kselftests ongoing work.
> > 
> 
> Ingo/Darren,
> 
> Sorry for not giving a better justification. I should have kept audience
> in mind that test authors might not be up on the framework changes.
> 
> Nested TAP header example:
> 
> TAP version 13
> selftests: step_after_suspend_test
> ========================================
> TAP version 13                                           (#Nested header)
> Bail out! open("/sys/power/state") failed (is this test running as root?)
> Pass 0 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
> 1..0
> not ok 1..1 selftests:  step_after_suspend_test [FAIL]
> selftests: breakpoint_test
> ========================================
> TAP version 13                                            (#Nested header)
> Pass 110 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
> 
> <snip>
> 
> 1..110
> ok 1..2 selftests: breakpoint_test [PASS]
> 
> 
> Some tests print TAP headers. This is helpful when test is run by itself.
> However, when "make kselftest" or "make -C" is used then common run_tests
> does its TAP header and nested headers get printed.
> 
> Nested headers are a problem for TAP13 parsers. Hence, I decided to go
> with a simpler approach to suppress TAP headers using env. variable
> that gets checked in the framework and suppresses the header.
> 
> selftests: kselftest framework: add handling for TAP header level
> 
> This minimizes the churn to actual tests and framework handles the
> TAP details.
> 
> futex test overrides the generic run_tests, so it needed changes to
> add a top level TAP header. Otherwise, you will see nested TAP header
> for each of the individual futxex tests as shown in the example below.
> 
> 
> TAP version 13
> # futex_requeue_pi: Test requeue functionality
> #       Arguments: broadcast=0 locked=0 owner=0 timeout=0ns
> not ok 1 # error futex-requeue-pi
> Pass 0 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 1
> 1..1
> 
> I can capture the nested TAP headers being a problem for parsers in
> the changelog. I will have to send v2 anyway, "make -C" still has
> the nested headers for fuxtex tests.
> 
> My goal is to not have the individual test authors be concerned about
> the TAP13 details and handle it in the framework. I am making changes
> to framework and test Makefiles for the TAP13 and other common formatting
> type features as opposed to individual test code. Test Makefiles that use
> lib.mk don't need changes either, futex needs changes mainly because of
> overrides.
> 
> Hope this help. I will do a better job of explaining in the future.

Thank you Shuah, this helps. I think this is also a good motivator to look at
the overrides we are using for the futex tests, and see if we can eliminate
them. As you point out, the fewer overrides we have, the less churn we
experience when the framework changes.

Most of the overrides as I recall were just used to minimize the number of
changes we had to make to what I had already written in the original tests. That
advantage seems to be losing out to impact from changes to the framework.

-- 
Darren Hart
VMware Open Source Technology Center