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(-)
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
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; \
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
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; \
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
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
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 {
linux-kselftest-mirror@lists.linaro.org
-
Shuah Khan (Samsung OSG)