[PATCH 0/7] Kselftest framework SKIP handling

[PATCH 0/7] Kselftest framework SKIP handling

[Shuah Khan (Samsung OSG)]

Kselftest framework currently treats all non-zero return codes from tests
as failures. When tests are skipped with non-zero return code, because of
unmet dependencies and/or unsupported configuration, it reports them as
failed. This will lead to too many false negatives even on the tests that
couldn't be run.

This patch series fixes the problem by adding SKIP handling:

The common RUN_TESTS and EMIT_TESTS functions are changed to test for
SKIP=4 return from tests. This change enables the framework and tests
that return SKIP=4 will report SKIP, other continue to report FAIL/PASS.

KSFT_SKIP has been changed to report Skip code of 4 instead of KSFT_PASS
to clearly differentiate the skipped tests.

I am working on Test changes to report SKIP instead FAIL/PASS when they get
skipped. I will be sending them out later this week.

In addition, I made changes to improve the reporting and cleaned up both
RUN_TESTS and EMIT_TESTS define to remove duplicate test output strings.

Shuah Khan (Samsung OSG) (7):
  selftests: lib.mk: cleanup RUN_TESTS define and make it readable
  selftests: lib.mk: add SKIP handling to RUN_TESTS define
  selftests: lib.mk: move running and printing result to a new function
  selftests: lib.mk: Include test suite name in the RUN_TESTS output
  selftests: lib.mk: add SKIP handling and test suite name to EMIT_TESTS
  selftests: lib.mk: add test execute bit check to EMIT_TESTS
  selftests: kselftest: change KSFT_SKIP=4 instead of KSFT_PASS

 tools/testing/selftests/Makefile    |  3 +-
 tools/testing/selftests/kselftest.h |  2 +-
 tools/testing/selftests/lib.mk      | 55 +++++++++++++++++++++++++++----------
 3 files changed, 44 insertions(+), 16 deletions(-)

-- 
2.14.1

[PATCH 1/7] selftests: lib.mk: cleanup RUN_TESTS define and make it readable

[Shuah Khan (Samsung OSG)]

Refine RUN_TESTS define's output block for summary and non-summary code
to remove duplicate code and make it readable.

cd `dirname $$TEST` > /dev/null; and cd - > /dev/null; are moved
to common code block and indentation fixed.

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
---
 tools/testing/selftests/lib.mk | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index c1b1a4dc6a96..1d2b48f2d481 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -32,11 +32,13 @@ define RUN_TESTS
 			echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
 			echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
 		else					\
-		if [ "X$(summary)" != "X" ]; then		\
-				cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
+			cd `dirname $$TEST` > /dev/null; \
+			if [ "X$(summary)" != "X" ]; then	\
+				(./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";		\
 			else				\
-				cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
+				(./$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";					\
 			fi;				\
+			cd - > /dev/null;		\
 		fi;					\
 	done;
 endef
-- 
2.14.1

[PATCH 2/7] selftests: lib.mk: add SKIP handling to RUN_TESTS define

[Shuah Khan (Samsung OSG)]

RUN_TESTS which is the common function that implements run_tests target,
treats all non-zero return codes from tests as failures. When tests are
skipped with non-zero return code, because of unmet dependencies and/or
unsupported configuration, it reports them as failed. This will lead to
too many false negatives even on the tests that couldn't be run.

RUN_TESTS is changed to test for SKIP=4 return from tests to enable the
framework for individual tests to return special SKIP code.

Tests will be changed as needed to report SKIP instead FAIL/PASS when
they get skipped.

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
---
 tools/testing/selftests/lib.mk | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 1d2b48f2d481..e8f5f8b3abeb 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -22,6 +22,7 @@ all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
 define RUN_TESTS
 	@export KSFT_TAP_LEVEL=`echo 1`;		\
 	test_num=`echo 0`;				\
+	skip=`echo 4`;					\
 	echo "TAP version 13";				\
 	for TEST in $(1); do				\
 		BASENAME_TEST=`basename $$TEST`;	\
@@ -34,9 +35,19 @@ define RUN_TESTS
 		else					\
 			cd `dirname $$TEST` > /dev/null; \
 			if [ "X$(summary)" != "X" ]; then	\
-				(./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";		\
+				(./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && \
+				echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || \
+				(if [ $$? -eq $$skip ]; then	\
+					echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]";				\
+				else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";					\
+				fi;)			\
 			else				\
-				(./$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";					\
+				(./$$BASENAME_TEST &&	\
+				echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") ||						\
+				(if [ $$? -eq $$skip ]; then \
+					echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]"; \
+				else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";				\
+				fi;)		\
 			fi;				\
 			cd - > /dev/null;		\
 		fi;					\
-- 
2.14.1

[PATCH 3/7] selftests: lib.mk: move running and printing result to a new function

[Shuah Khan (Samsung OSG)]

RUN_TESTS function has grown and becoming harder to maintain. Move
the code that runs and tests for returns codes to a new function
and call it from RUN_TESTS.

A new RUN_TEST_PRINT_RESULT is created to simplify RUN_TESTS and make it
easier to add handling for other return codes as needed.

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
---
 tools/testing/selftests/lib.mk | 52 +++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index e8f5f8b3abeb..dd516f92f7c2 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -19,6 +19,33 @@ TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
 all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
 
 .ONESHELL:
+define RUN_TEST_PRINT_RESULT
+	echo "selftests: $$BASENAME_TEST";	\
+	echo "========================================";	\
+	if [ ! -x $$TEST ]; then	\
+		echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
+		echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
+	else					\
+		cd `dirname $$TEST` > /dev/null; \
+		if [ "X$(summary)" != "X" ]; then	\
+			(./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && \
+			echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || \
+			(if [ $$? -eq $$skip ]; then	\
+				echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]";				\
+			else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";					\
+			fi;)			\
+		else				\
+			(./$$BASENAME_TEST &&	\
+			echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") ||						\
+			(if [ $$? -eq $$skip ]; then \
+				echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]"; \
+			else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";				\
+			fi;)		\
+		fi;				\
+		cd - > /dev/null;		\
+	fi;
+endef
+
 define RUN_TESTS
 	@export KSFT_TAP_LEVEL=`echo 1`;		\
 	test_num=`echo 0`;				\
@@ -27,30 +54,7 @@ define RUN_TESTS
 	for TEST in $(1); do				\
 		BASENAME_TEST=`basename $$TEST`;	\
 		test_num=`echo $$test_num+1 | bc`;	\
-		echo "selftests: $$BASENAME_TEST";	\
-		echo "========================================";	\
-		if [ ! -x $$TEST ]; then	\
-			echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
-			echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
-		else					\
-			cd `dirname $$TEST` > /dev/null; \
-			if [ "X$(summary)" != "X" ]; then	\
-				(./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && \
-				echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || \
-				(if [ $$? -eq $$skip ]; then	\
-					echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]";				\
-				else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";					\
-				fi;)			\
-			else				\
-				(./$$BASENAME_TEST &&	\
-				echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") ||						\
-				(if [ $$? -eq $$skip ]; then \
-					echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]"; \
-				else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";				\
-				fi;)		\
-			fi;				\
-			cd - > /dev/null;		\
-		fi;					\
+		$(call RUN_TEST_PRINT_RESULT,$(TEST),$(BASENAME_TEST),$(test_num),$(skip))						\
 	done;
 endef
 
-- 
2.14.1

[PATCH 4/7] selftests: lib.mk: Include test suite name in the RUN_TESTS output

[Shuah Khan (Samsung OSG)]

Currently just the test name is printed in the RUN_TESTS output. For
example, when raw_skew sub-test from timers tests in run, the output
shows just raw_skew. Include main test name when printing sub-test
results.

In addition, remove duplicate strings for printing common information
with a new for the test header information.

Before the change:

selftests: raw_skew
========================================
WARNING: ADJ_OFFSET in progress, this will cause inaccurate results
Estimating clock drift: -20.616(est) -20.586(act)       [OK]
Pass 0 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
1..0
ok 1..7 selftests: raw_skew [PASS]

After the change:

selftests: timers: raw_skew
========================================
WARNING: ADJ_OFFSET in progress, this will cause inaccurate results
Estimating clock drift: -19.794(est) -19.896(act)       [OK]
Pass 0 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
1..0
ok 1..7 selftests: timers: raw_skew [PASS]

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
---
 tools/testing/selftests/lib.mk | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index dd516f92f7c2..eb2be6dabbda 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -20,26 +20,27 @@ all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
 
 .ONESHELL:
 define RUN_TEST_PRINT_RESULT
-	echo "selftests: $$BASENAME_TEST";	\
+	TEST_HDR_MSG="selftests: "`basename $$PWD`:" $$BASENAME_TEST";	\
+	echo $$TEST_HDR_MSG;					\
 	echo "========================================";	\
 	if [ ! -x $$TEST ]; then	\
-		echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
-		echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
+		echo "$$TEST_HDR_MSG: Warning: file $$BASENAME_TEST is not executable, correct this.";\
+		echo "not ok 1..$$test_num $$TEST_HDR_MSG [FAIL]"; \
 	else					\
 		cd `dirname $$TEST` > /dev/null; \
 		if [ "X$(summary)" != "X" ]; then	\
 			(./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && \
-			echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || \
+			echo "ok 1..$$test_num $$TEST_HDR_MSG [PASS]") || \
 			(if [ $$? -eq $$skip ]; then	\
-				echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]";				\
-			else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";					\
+				echo "not ok 1..$$test_num $$TEST_HDR_MSG [SKIP]";				\
+			else echo "not ok 1..$$test_num $$TEST_HDR_MSG [FAIL]";					\
 			fi;)			\
 		else				\
 			(./$$BASENAME_TEST &&	\
-			echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") ||						\
+			echo "ok 1..$$test_num $$TEST_HDR_MSG [PASS]") ||						\
 			(if [ $$? -eq $$skip ]; then \
-				echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [SKIP]"; \
-			else echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]";				\
+				echo "not ok 1..$$test_num $$TEST_HDR_MSG [SKIP]"; \
+			else echo "not ok 1..$$test_num $$TEST_HDR_MSG [FAIL]";				\
 			fi;)		\
 		fi;				\
 		cd - > /dev/null;		\
-- 
2.14.1

[PATCH 5/7] selftests: lib.mk: add SKIP handling and test suite name to EMIT_TESTS

[Shuah Khan (Samsung OSG)]

EMIT_TESTS which is the common function that implements run_tests target,
treats all non-zero return codes from tests as failures. When tests are
skipped with non-zero return code, because of unmet dependencies and/or
unsupported configuration, it reports them as failed. This will lead to
too many false negatives even on the tests that couldn't be run.

EMIT_TESTS is changed to test for SKIP=4 return from tests to enable
the framework for individual tests to return special SKIP code.

Tests will be changed as needed to report SKIP instead FAIL/PASS when
they get skipped.

Currently just the test name is printed in the RUN_TESTS output. For
example, when raw_skew sub-test from timers tests in run, the output
shows just raw_skew. Include main test name when printing sub-test
results.

In addition, remove duplicate strings for printing common information with
a new for the test header information.

With this change run_kelftest.sh output for breakpoints test will be:

TAP version 13
Running tests in breakpoints
========================================
selftests: breakpoints: step_after_suspend_test
not ok 1..1 selftests: breakpoints: step_after_suspend_test [SKIP]
selftests: breakpoints: breakpoint_test
ok 1..2 selftests: breakpoints: breakpoint_test [PASS]

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
---
 tools/testing/selftests/Makefile | 3 ++-
 tools/testing/selftests/lib.mk   | 8 ++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 32aafa92074c..302c7ea8b820 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -134,7 +134,8 @@ 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)
+	echo "export KSFT_TAP_LEVEL=1" >> $(ALL_SCRIPT)
+	echo "export skip=4" >> $(ALL_SCRIPT)
 
 	for TARGET in $(TARGETS); do \
 		BUILD_TARGET=$$BUILD/$$TARGET;	\
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index eb2be6dabbda..0c6012500026 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -94,9 +94,13 @@ else
 endif
 
 define EMIT_TESTS
-	@for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \
+	@test_num=`echo 0`;				\
+	for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \
 		BASENAME_TEST=`basename $$TEST`;	\
-		echo "(./$$BASENAME_TEST >> \$$OUTPUT 2>&1 && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
+		test_num=`echo $$test_num+1 | bc`;	\
+		TEST_HDR_MSG="selftests: "`basename $$PWD`:" $$BASENAME_TEST";	\
+		echo "echo $$TEST_HDR_MSG";	\
+		echo "(./$$BASENAME_TEST >> \$$OUTPUT 2>&1 && echo \"ok 1..$$test_num $$TEST_HDR_MSG [PASS]\") || (if [ \$$? -eq \$$skip ]; then echo \"not ok 1..$$test_num $$TEST_HDR_MSG [SKIP]\"; else echo \"not ok 1..$$test_num $$TEST_HDR_MSG [FAIL]\"; fi;)"; \
 	done;
 endef
 
-- 
2.14.1

[PATCH 6/7] selftests: lib.mk: add test execute bit check to EMIT_TESTS

[Shuah Khan (Samsung OSG)]

Similar to what RUN_TESTS does, change EMIT_TESTS to check for execute
bit and emit code to print warnings if test isn't executable to the
the run_kselftest.sh.

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
---
 tools/testing/selftests/lib.mk | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 0c6012500026..6466294366dc 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -100,7 +100,12 @@ define EMIT_TESTS
 		test_num=`echo $$test_num+1 | bc`;	\
 		TEST_HDR_MSG="selftests: "`basename $$PWD`:" $$BASENAME_TEST";	\
 		echo "echo $$TEST_HDR_MSG";	\
-		echo "(./$$BASENAME_TEST >> \$$OUTPUT 2>&1 && echo \"ok 1..$$test_num $$TEST_HDR_MSG [PASS]\") || (if [ \$$? -eq \$$skip ]; then echo \"not ok 1..$$test_num $$TEST_HDR_MSG [SKIP]\"; else echo \"not ok 1..$$test_num $$TEST_HDR_MSG [FAIL]\"; fi;)"; \
+		if [ ! -x $$TEST ]; then	\
+			echo "echo \"$$TEST_HDR_MSG: Warning: file $$BASENAME_TEST is not executable, correct this.\"";		\
+			echo "echo \"not ok 1..$$test_num $$TEST_HDR_MSG [FAIL]\""; \
+		else
+			echo "(./$$BASENAME_TEST >> \$$OUTPUT 2>&1 && echo \"ok 1..$$test_num $$TEST_HDR_MSG [PASS]\") || (if [ \$$? -eq \$$skip ]; then echo \"not ok 1..$$test_num $$TEST_HDR_MSG [SKIP]\"; else echo \"not ok 1..$$test_num $$TEST_HDR_MSG [FAIL]\"; fi;)"; \
+		fi;		\
 	done;
 endef
 
-- 
2.14.1

[PATCH 7/7] selftests: kselftest: change KSFT_SKIP=4 instead of KSFT_PASS

[Shuah Khan (Samsung OSG)]

KSFT_SKIP points to KSFT_PASS resulting in reporting skipped tests as
Passed, when test programs exit with KSFT_SKIP or call ksft_exit_skip().
If tests are skipped because of unmet dependencies and/or unsupported
configuration, reporting them as passed leads to too many false positives.

Fix it to return a skip code of 4 to clearly differentiate the skipped
tests.

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
---
 tools/testing/selftests/kselftest.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index 1b9d8ecdebce..15e6b75fc3a5 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -20,7 +20,7 @@
 #define KSFT_XFAIL 2
 #define KSFT_XPASS 3
 /* Treat skip as pass */
-#define KSFT_SKIP  KSFT_PASS
+#define KSFT_SKIP  4
 
 /* counters */
 struct ksft_count {
-- 
2.14.1