This refactors the selftest Makefiles to extract the test running logic to be reused between "run_tests" and "emit_tests", while also fixing up the test output to be TAP version 13 compliant: - added "plan" line - fixed result line syntax - moved all test output to be "# "-prefixed as TAP "diagnostic" lines
The prefixing code includes a fallback mode for limited execution environments.
Additionally, the plan lines are fixed for all callers of kselftest.h.
-Kees
v2: - fix external make variable "summary=1" through-out series (shuah) - fix plan line output for all kselftest.h users
Kees Cook (8): selftests: Extract single-test shell logic from lib.mk selftests: Use runner.sh for emit targets selftests: Extract logic for multiple test runs selftests: Add plan line and fix result line syntax selftests: Distinguish between missing and non-executable selftests: Move test output to diagnostic lines selftests: Remove KSFT_TAP_LEVEL selftests: Add test plan API to kselftest.h and adjust callers
tools/testing/selftests/.gitignore | 1 - tools/testing/selftests/Makefile | 24 ++---- .../selftests/breakpoints/breakpoint_test.c | 15 +++- .../breakpoints/breakpoint_test_arm64.c | 3 +- .../breakpoints/step_after_suspend_test.c | 8 ++ .../selftests/capabilities/test_execve.c | 6 +- .../futex/functional/futex_requeue_pi.c | 1 + .../futex_requeue_pi_mismatched_ops.c | 1 + .../futex_requeue_pi_signal_restart.c | 1 + .../futex_wait_private_mapped_file.c | 1 + .../futex/functional/futex_wait_timeout.c | 1 + .../futex_wait_uninitialized_heap.c | 1 + .../futex/functional/futex_wait_wouldblock.c | 1 + tools/testing/selftests/kselftest.h | 17 +++- tools/testing/selftests/kselftest/prefix.pl | 23 +++++ tools/testing/selftests/kselftest/runner.sh | 86 +++++++++++++++++++ tools/testing/selftests/lib.mk | 64 +++----------- .../selftests/membarrier/membarrier_test.c | 1 + tools/testing/selftests/pidfd/pidfd_test.c | 1 + tools/testing/selftests/sigaltstack/sas.c | 1 + tools/testing/selftests/sync/sync_test.c | 1 + 21 files changed, 178 insertions(+), 80 deletions(-) create mode 100755 tools/testing/selftests/kselftest/prefix.pl create mode 100644 tools/testing/selftests/kselftest/runner.sh
In order to improve the reusability of the kselftest test running logic, this extracts the single-test logic from lib.mk into kselftest/runner.sh which lib.mk can call directly. No changes in output.
As part of the change, this moves the "summary" Makefile logic around to set a new "logfile" output. This will be used again in the future "emit_tests" target as well.
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/.gitignore | 1 - tools/testing/selftests/kselftest/runner.sh | 32 ++++++++++++++++++ tools/testing/selftests/lib.mk | 37 ++++----------------- 3 files changed, 39 insertions(+), 31 deletions(-) create mode 100644 tools/testing/selftests/kselftest/runner.sh
diff --git a/tools/testing/selftests/.gitignore b/tools/testing/selftests/.gitignore index 91750352459d..8059ce834247 100644 --- a/tools/testing/selftests/.gitignore +++ b/tools/testing/selftests/.gitignore @@ -1,4 +1,3 @@ -kselftest gpiogpio-event-mon gpiogpio-hammer gpioinclude/ diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh new file mode 100644 index 000000000000..e1117d703887 --- /dev/null +++ b/tools/testing/selftests/kselftest/runner.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Runs a set of tests in a given subdirectory. +export skip_rc=4 +export logfile=/dev/stdout + +run_one() +{ + TEST="$1" + NUM="$2" + + BASENAME_TEST=$(basename $TEST) + + TEST_HDR_MSG="selftests: "`basename $PWD`:" $BASENAME_TEST" + echo "$TEST_HDR_MSG" + echo "========================================" + if [ ! -x "$TEST" ]; then + echo "$TEST_HDR_MSG: Warning: file $TEST is not executable, correct this." + echo "not ok 1..$test_num $TEST_HDR_MSG [FAIL]" + else + cd `dirname $TEST` > /dev/null + (./$BASENAME_TEST >> "$logfile" 2>&1 && + echo "ok 1..$test_num $TEST_HDR_MSG [PASS]") || + (if [ $? -eq $skip_rc ]; then \ + echo "not ok 1..$test_num $TEST_HDR_MSG [SKIP]" + else + echo "not ok 1..$test_num $TEST_HDR_MSG [FAIL]" + fi) + cd - >/dev/null + fi +} diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 5979fdc4f36c..9d2b3c303bfa 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -14,6 +14,7 @@ ifeq (0,$(MAKELEVEL)) endif endif endif +selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST))))
# The following are built by lib.mk common compile rules. # TEST_CUSTOM_PROGS should be used by tests that require @@ -65,43 +66,19 @@ all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) endif
.ONESHELL: -define RUN_TEST_PRINT_RESULT - TEST_HDR_MSG="selftests: "`basename $$PWD`:" $$BASENAME_TEST"; \ - echo $$TEST_HDR_MSG; \ - echo "========================================"; \ - if [ ! -x $$TEST ]; then \ - 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 $$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;) \ - else \ - (./$$BASENAME_TEST && \ - 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; \ - cd - > /dev/null; \ - fi; -endef - define RUN_TESTS @export KSFT_TAP_LEVEL=`echo 1`; \ test_num=`echo 0`; \ - skip=`echo 4`; \ + . $(selfdir)/kselftest/runner.sh; \ echo "TAP version 13"; \ for TEST in $(1); do \ BASENAME_TEST=`basename $$TEST`; \ test_num=`echo $$test_num+1 | bc`; \ - $(call RUN_TEST_PRINT_RESULT,$(TEST),$(BASENAME_TEST),$(test_num),$(skip)) \ + if [ "X$(summary)" != "X" ]; then \ + logfile="/tmp/$$BASENAME_TEST"; \ + cat /dev/null > "$$logfile"; \ + fi; \ + run_one "$$BASENAME_TEST" "$$test_num"; \ done; endef
This reuses the new runner.sh for the emit targets instead of manually running each test via run_kselftest.sh.
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/Makefile | 11 +++++------ tools/testing/selftests/lib.mk | 15 ++------------- 2 files changed, 7 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index f2ebf8cf4686..c5f9f736cdbd 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -176,7 +176,8 @@ ALL_SCRIPT := $(INSTALL_PATH)/run_kselftest.sh install: ifdef INSTALL_PATH @# Ask all targets to install their files - mkdir -p $(INSTALL_PATH) + mkdir -p $(INSTALL_PATH)/kselftest + install -m 744 kselftest/runner.sh $(INSTALL_PATH)/kselftest/ @for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ make OUTPUT=$$BUILD_TARGET -C $$TARGET INSTALL_PATH=$(INSTALL_PATH)/$$TARGET install; \ @@ -186,15 +187,13 @@ ifdef INSTALL_PATH echo "#!/bin/sh" > $(ALL_SCRIPT) echo "BASE_DIR=$$(realpath $$(dirname $$0))" >> $(ALL_SCRIPT) echo "cd $$BASE_DIR" >> $(ALL_SCRIPT) + echo ". ./kselftest/runner.sh" >> $(ALL_SCRIPT) echo "ROOT=$$PWD" >> $(ALL_SCRIPT) echo "if [ "$$1" = "--summary" ]; then" >> $(ALL_SCRIPT) - echo " OUTPUT=$$BASE_DIR/output.log" >> $(ALL_SCRIPT) - echo " cat /dev/null > $$OUTPUT" >> $(ALL_SCRIPT) - echo "else" >> $(ALL_SCRIPT) - echo " OUTPUT=/dev/stdout" >> $(ALL_SCRIPT) + echo " logfile=$$BASE_DIR/output.log" >> $(ALL_SCRIPT) + echo " cat /dev/null > $$logfile" >> $(ALL_SCRIPT) echo "fi" >> $(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 9d2b3c303bfa..6b2d026a94ea 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -116,24 +116,13 @@ else $(error Error: set INSTALL_PATH to use install) endif
-define EMIT_TESTS +emit_tests: @test_num=`echo 0`; \ for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \ BASENAME_TEST=`basename $$TEST`; \ test_num=`echo $$test_num+1 | bc`; \ - TEST_HDR_MSG="selftests: "`basename $$PWD`:" $$BASENAME_TEST"; \ - echo "echo $$TEST_HDR_MSG"; \ - 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; \ + echo "run_one "$$BASENAME_TEST" "$$test_num""; \ done; -endef - -emit_tests: - $(EMIT_TESTS)
# define if isn't already. It is undefined in make O= case. ifeq ($(RM),)
This moves the logic for running multiple tests into a single "run_many" function of runner.sh. Both "run_tests" and "emit_tests" are modified to use it. Summary handling is now controlled by the "per_test_logging" shell flag.
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/Makefile | 6 ++--- tools/testing/selftests/kselftest/runner.sh | 25 ++++++++++++++++++--- tools/testing/selftests/lib.mk | 25 +++++++-------------- 3 files changed, 32 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index c5f9f736cdbd..4ac1d1c7ce5b 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -193,16 +193,14 @@ ifdef INSTALL_PATH echo " logfile=$$BASE_DIR/output.log" >> $(ALL_SCRIPT) echo " cat /dev/null > $$logfile" >> $(ALL_SCRIPT) echo "fi" >> $(ALL_SCRIPT) - echo "export KSFT_TAP_LEVEL=1" >> $(ALL_SCRIPT)
for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ - echo "echo ; echo TAP version 13" >> $(ALL_SCRIPT); \ - echo "echo Running tests in $$TARGET" >> $(ALL_SCRIPT); \ - echo "echo ========================================" >> $(ALL_SCRIPT); \ echo "[ -w /dev/kmsg ] && echo "kselftest: Running tests in $$TARGET" >> /dev/kmsg" >> $(ALL_SCRIPT); \ echo "cd $$TARGET" >> $(ALL_SCRIPT); \ + echo -n "run_many" >> $(ALL_SCRIPT); \ make -s --no-print-directory OUTPUT=$$BUILD_TARGET -C $$TARGET emit_tests >> $(ALL_SCRIPT); \ + echo "" >> $(ALL_SCRIPT); \ echo "cd $$ROOT" >> $(ALL_SCRIPT); \ done;
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index e1117d703887..f12b0a631273 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -2,17 +2,20 @@ # 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 logfile=/dev/stdout +export per_test_logging=
run_one() { - TEST="$1" - NUM="$2" + DIR="$1" + TEST="$2" + NUM="$3"
BASENAME_TEST=$(basename $TEST)
- TEST_HDR_MSG="selftests: "`basename $PWD`:" $BASENAME_TEST" + TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST" echo "$TEST_HDR_MSG" echo "========================================" if [ ! -x "$TEST" ]; then @@ -30,3 +33,19 @@ run_one() cd - >/dev/null fi } + +run_many() +{ + echo "TAP version 13" + DIR=$(basename "$PWD") + test_num=0 + for TEST in "$@"; do + BASENAME_TEST=$(basename $TEST) + test_num=$(( test_num + 1 )) + if [ -n "$per_test_logging" ]; then + logfile="/tmp/$BASENAME_TEST" + cat /dev/null > "$logfile" + fi + run_one "$DIR" "$TEST" "$test_num" + done +} diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 6b2d026a94ea..28b8ffedfdf1 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -67,19 +67,11 @@ endif
.ONESHELL: define RUN_TESTS - @export KSFT_TAP_LEVEL=`echo 1`; \ - test_num=`echo 0`; \ - . $(selfdir)/kselftest/runner.sh; \ - echo "TAP version 13"; \ - for TEST in $(1); do \ - BASENAME_TEST=`basename $$TEST`; \ - test_num=`echo $$test_num+1 | bc`; \ - if [ "X$(summary)" != "X" ]; then \ - logfile="/tmp/$$BASENAME_TEST"; \ - cat /dev/null > "$$logfile"; \ - fi; \ - run_one "$$BASENAME_TEST" "$$test_num"; \ - done; + @. $(selfdir)/kselftest/runner.sh; \ + if [ "X$(summary)" != "X" ]; then \ + per_test_logging=1; \ + fi; \ + run_many $(1) endef
run_tests: all @@ -117,12 +109,11 @@ else endif
emit_tests: - @test_num=`echo 0`; \ for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \ BASENAME_TEST=`basename $$TEST`; \ - test_num=`echo $$test_num+1 | bc`; \ - echo "run_one "$$BASENAME_TEST" "$$test_num""; \ - done; + echo " \"; \ + echo -n " "$$BASENAME_TEST""; \ + done; \
# define if isn't already. It is undefined in make O= case. ifeq ($(RM),)
The TAP version 13 spec requires a "plan" line, which has been missing. Since we always know how many tests we're going to run, emit the count on the plan line. This also fixes the result lines to remove the "1.." prefix which is against spec, and to mark skips with the correct "# SKIP" suffix.
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/kselftest.h | 4 ++-- tools/testing/selftests/kselftest/runner.sh | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 47e1d995c182..9f4147a6fdbc 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -111,7 +111,7 @@ static inline void ksft_test_result_skip(const char *msg, ...) ksft_cnt.ksft_xskip++;
va_start(args, msg); - printf("ok %d # skip ", ksft_test_num()); + printf("not ok %d # SKIP ", ksft_test_num()); vprintf(msg, args); va_end(args); } @@ -172,7 +172,7 @@ static inline int ksft_exit_skip(const char *msg, ...) va_list args;
va_start(args, msg); - printf("1..%d # Skipped: ", ksft_test_num()); + printf("not ok %d # SKIP ", ksft_test_num()); vprintf(msg, args); va_end(args); } else { diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index f12b0a631273..e0621974e01e 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -20,15 +20,15 @@ run_one() echo "========================================" if [ ! -x "$TEST" ]; then echo "$TEST_HDR_MSG: Warning: file $TEST is not executable, correct this." - echo "not ok 1..$test_num $TEST_HDR_MSG [FAIL]" + echo "not ok $test_num $TEST_HDR_MSG" else cd `dirname $TEST` > /dev/null (./$BASENAME_TEST >> "$logfile" 2>&1 && - echo "ok 1..$test_num $TEST_HDR_MSG [PASS]") || + echo "ok $test_num $TEST_HDR_MSG") || (if [ $? -eq $skip_rc ]; then \ - echo "not ok 1..$test_num $TEST_HDR_MSG [SKIP]" + echo "not ok $test_num $TEST_HDR_MSG # SKIP" else - echo "not ok 1..$test_num $TEST_HDR_MSG [FAIL]" + echo "not ok $test_num $TEST_HDR_MSG" fi) cd - >/dev/null fi @@ -39,6 +39,8 @@ run_many() echo "TAP version 13" DIR=$(basename "$PWD") test_num=0 + total=$(echo "$@" | wc -w) + echo "1..$total" for TEST in "$@"; do BASENAME_TEST=$(basename $TEST) test_num=$(( test_num + 1 ))
If a test was missing (e.g. wrong architecture, etc), the test runner would incorrectly claim the test was non-executable. This adds an existence check to report correctly.
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/kselftest/runner.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index e0621974e01e..a66fb64e61e9 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -19,7 +19,12 @@ run_one() echo "$TEST_HDR_MSG" echo "========================================" if [ ! -x "$TEST" ]; then - echo "$TEST_HDR_MSG: Warning: file $TEST is not executable, correct this." + echo -n "$TEST_HDR_MSG: Warning: file $TEST is " + if [ ! -e "$TEST" ]; then + echo "missing!" + else + echo "not executable, correct this." + fi echo "not ok $test_num $TEST_HDR_MSG" else cd `dirname $TEST` > /dev/null
This changes the selftest output so that each test's output is prefixed with "# " as a TAP "diagnostic line".
This creates a bit of a kernel-specific TAP dialect where the diagnostics precede the results. The TAP spec isn't entirely clear about this, though, so I think it's the correct solution so as to keep interactive runs making sense. If the output _followed_ the result line in the spec-suggested YAML form, each test would dump all of its output at once instead of as it went, making debugging harder.
This does, however, solve the recursive TAP output problem, as sub-tests will simply be prefixed by "# ". Parsing sub-tests becomes a simple problem of just removing the first two characters of a given top-level test's diagnostic output, and parsing the results.
Note that the shell construct needed to both get an exit code from the first command in a pipe and still filter the pipe (to add the "# " prefix) uses a POSIX solution rather than the bash "pipefail" option which is not supported by dash.
Since some test environments may have a very minimal set of utilities available, the new prefixing code will fall back to doing line-at-a-time prefixing if perl and/or stdbuf are not available.
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/kselftest.h | 2 +- tools/testing/selftests/kselftest/prefix.pl | 23 +++++++++++++ tools/testing/selftests/kselftest/runner.sh | 37 ++++++++++++++++++--- tools/testing/selftests/lib.mk | 3 +- 5 files changed, 60 insertions(+), 6 deletions(-) create mode 100755 tools/testing/selftests/kselftest/prefix.pl
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 4ac1d1c7ce5b..64699f59b95f 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -178,6 +178,7 @@ ifdef INSTALL_PATH @# Ask all targets to install their files mkdir -p $(INSTALL_PATH)/kselftest install -m 744 kselftest/runner.sh $(INSTALL_PATH)/kselftest/ + install -m 744 kselftest/prefix.pl $(INSTALL_PATH)/kselftest/ @for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ make OUTPUT=$$BUILD_TARGET -C $$TARGET INSTALL_PATH=$(INSTALL_PATH)/$$TARGET install; \ diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 9f4147a6fdbc..7f078e79a9fa 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -63,7 +63,7 @@ static inline void ksft_print_header(void)
static inline void ksft_print_cnts(void) { - printf("Pass %d Fail %d Xfail %d Xpass %d Skip %d Error %d\n", + printf("# Pass %d Fail %d Xfail %d Xpass %d Skip %d Error %d\n", ksft_cnt.ksft_pass, ksft_cnt.ksft_fail, ksft_cnt.ksft_xfail, ksft_cnt.ksft_xpass, ksft_cnt.ksft_xskip, ksft_cnt.ksft_error); diff --git a/tools/testing/selftests/kselftest/prefix.pl b/tools/testing/selftests/kselftest/prefix.pl new file mode 100755 index 000000000000..ec7e48118183 --- /dev/null +++ b/tools/testing/selftests/kselftest/prefix.pl @@ -0,0 +1,23 @@ +#!/usr/bin/perl +# SPDX-License-Identifier: GPL-2.0 +# Prefix all lines with "# ", unbuffered. Command being piped in may need +# to have unbuffering forced with "stdbuf -i0 -o0 -e0 $cmd". +use strict; + +binmode STDIN; +binmode STDOUT; + +STDOUT->autoflush(1); + +my $needed = 1; +while (1) { + my $char; + my $bytes = sysread(STDIN, $char, 1); + exit 0 if ($bytes == 0); + if ($needed) { + print "# "; + $needed = 0; + } + print $char; + $needed = 1 if ($char eq "\n"); +} diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index a66fb64e61e9..b9f74e5a2ee5 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -7,6 +7,34 @@ export skip_rc=4 export logfile=/dev/stdout export per_test_logging=
+# There isn't a shell-agnostic way to find the path of a sourced file, +# so we must rely on BASE_DIR being set to find other tools. +if [ -z "$BASE_DIR" ]; then + echo "Error: BASE_DIR must be set before sourcing." >&2 + exit 1 +fi + +# If Perl is unavailable, we must fall back to line-at-a-time prefixing +# with sed instead of unbuffered output. +tap_prefix() +{ + if [ ! -x /usr/bin/perl ]; then + sed -e 's/^/# /' + else + "$BASE_DIR"/kselftest/prefix.pl + fi +} + +# If stdbuf is unavailable, we must fall back to line-at-a-time piping. +tap_unbuffer() +{ + if ! which stdbuf >/dev/null ; then + "$@" + else + stdbuf -i0 -o0 -e0 "$@" + fi +} + run_one() { DIR="$1" @@ -16,10 +44,9 @@ run_one() BASENAME_TEST=$(basename $TEST)
TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST" - echo "$TEST_HDR_MSG" - echo "========================================" + echo "# $TEST_HDR_MSG" if [ ! -x "$TEST" ]; then - echo -n "$TEST_HDR_MSG: Warning: file $TEST is " + echo -n "# Warning: file $TEST is " if [ ! -e "$TEST" ]; then echo "missing!" else @@ -28,7 +55,9 @@ run_one() echo "not ok $test_num $TEST_HDR_MSG" else cd `dirname $TEST` > /dev/null - (./$BASENAME_TEST >> "$logfile" 2>&1 && + (((((tap_unbuffer ./$BASENAME_TEST 2>&1; echo $? >&3) | + tap_prefix >&4) 3>&1) | + (read xs; exit $xs)) 4>>"$logfile" && echo "ok $test_num $TEST_HDR_MSG") || (if [ $? -eq $skip_rc ]; then \ echo "not ok $test_num $TEST_HDR_MSG # SKIP" diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 28b8ffedfdf1..098dd0065fb1 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -67,7 +67,8 @@ endif
.ONESHELL: define RUN_TESTS - @. $(selfdir)/kselftest/runner.sh; \ + @BASE_DIR="$(selfdir)"; \ + . $(selfdir)/kselftest/runner.sh; \ if [ "X$(summary)" != "X" ]; then \ per_test_logging=1; \ fi; \
Since sub-testing can now be detected by indentation level, this removes KSFT_TAP_LEVEL so that subtests report their TAP header for later parsing.
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/Makefile | 6 ------ tools/testing/selftests/kselftest/runner.sh | 1 - 2 files changed, 7 deletions(-)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 64699f59b95f..9f05448e5e4b 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -86,12 +86,6 @@ 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 b9f74e5a2ee5..eff3ee303d0d 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -2,7 +2,6 @@ # 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 logfile=/dev/stdout export per_test_logging=
On 4/24/19 5:12 PM, Kees Cook wrote:
Since sub-testing can now be detected by indentation level, this removes KSFT_TAP_LEVEL so that subtests report their TAP header for later parsing.
Signed-off-by: Kees Cook keescook@chromium.org
tools/testing/selftests/Makefile | 6 ------ tools/testing/selftests/kselftest/runner.sh | 1 - 2 files changed, 7 deletions(-)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 64699f59b95f..9f05448e5e4b 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -86,12 +86,6 @@ 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 b9f74e5a2ee5..eff3ee303d0d 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -2,7 +2,6 @@ # 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 logfile=/dev/stdout export per_test_logging=
Does this take into ksft_print_header() getenv logic to avoid printing TAP headers from tests when they fork? e.g timers tests do that a lot.
thanks, -- Shuah
On Thu, Apr 25, 2019 at 9:36 AM shuah shuah@kernel.org wrote:
On 4/24/19 5:12 PM, Kees Cook wrote:
Since sub-testing can now be detected by indentation level, this removes KSFT_TAP_LEVEL so that subtests report their TAP header for later parsing.
Does this take into ksft_print_header() getenv logic to avoid printing TAP headers from tests when they fork? e.g timers tests do that a lot.
I didn't change the ksft_print_header() code, in case you want it back in the future. But nothing sets that variable any more in my series:
$ git grep KSFT_TAP_LEVEL tools/testing/selftests/kselftest.h: if (!(getenv("KSFT_TAP_LEVEL")))
I don't see the timers tests using print_header() at all, actually...
$ cd tools/testing/kselftest/timers $ git grep print_header | wc -l 0
On 4/25/19 10:56 AM, Kees Cook wrote:
On Thu, Apr 25, 2019 at 9:36 AM shuah shuah@kernel.org wrote:
On 4/24/19 5:12 PM, Kees Cook wrote:
Since sub-testing can now be detected by indentation level, this removes KSFT_TAP_LEVEL so that subtests report their TAP header for later parsing.
Does this take into ksft_print_header() getenv logic to avoid printing TAP headers from tests when they fork? e.g timers tests do that a lot.
I didn't change the ksft_print_header() code, in case you want it back in the future. But nothing sets that variable any more in my series:
Right. I want to see the impact of not setting this. I added this for two reasons, one is to prevent nesting which goes away with your refactor. I know there is another reason which I can't recall.
$ git grep KSFT_TAP_LEVEL tools/testing/selftests/kselftest.h: if (!(getenv("KSFT_TAP_LEVEL")))
I don't see the timers tests using print_header() at all, actually...
$ cd tools/testing/kselftest/timers $ git grep print_header | wc -l 0
I don't see it in there either. I must be thinking about another test.
btw I like the changes. I just want to make sure it gets some road testing first. :)
thanks, -- Shuah
The test plan for TAP needs to be declared immediately after the header. This adds the test plan API to kselftest.h and updates all callers to declare their expected test counts.
Signed-off-by: Kees Cook keescook@chromium.org --- .../selftests/breakpoints/breakpoint_test.c | 15 ++++++++++++--- .../selftests/breakpoints/breakpoint_test_arm64.c | 3 ++- .../breakpoints/step_after_suspend_test.c | 8 ++++++++ .../testing/selftests/capabilities/test_execve.c | 6 ++++-- .../selftests/futex/functional/futex_requeue_pi.c | 1 + .../functional/futex_requeue_pi_mismatched_ops.c | 1 + .../functional/futex_requeue_pi_signal_restart.c | 1 + .../functional/futex_wait_private_mapped_file.c | 1 + .../futex/functional/futex_wait_timeout.c | 1 + .../functional/futex_wait_uninitialized_heap.c | 1 + .../futex/functional/futex_wait_wouldblock.c | 1 + tools/testing/selftests/kselftest.h | 13 +++++++++++-- .../selftests/membarrier/membarrier_test.c | 1 + tools/testing/selftests/pidfd/pidfd_test.c | 1 + tools/testing/selftests/sigaltstack/sas.c | 1 + tools/testing/selftests/sync/sync_test.c | 1 + 16 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/breakpoints/breakpoint_test.c b/tools/testing/selftests/breakpoints/breakpoint_test.c index 901b85ea6a59..8f3655e59020 100644 --- a/tools/testing/selftests/breakpoints/breakpoint_test.c +++ b/tools/testing/selftests/breakpoints/breakpoint_test.c @@ -21,6 +21,8 @@
#include "../kselftest.h"
+#define COUNT_ISN_BPS 4 +#define COUNT_WPS 4
/* Breakpoint access modes */ enum { @@ -220,7 +222,7 @@ static void trigger_tests(void) if (!local && !global) continue;
- for (i = 0; i < 4; i++) { + for (i = 0; i < COUNT_ISN_BPS; i++) { dummy_funcs[i](); check_trapped(); } @@ -292,7 +294,7 @@ static void launch_instruction_breakpoints(char *buf, int local, int global) { int i;
- for (i = 0; i < 4; i++) { + for (i = 0; i < COUNT_ISN_BPS; i++) { set_breakpoint_addr(dummy_funcs[i], i); toggle_breakpoint(i, BP_X, 1, local, global, 1); ptrace(PTRACE_CONT, child_pid, NULL, 0); @@ -314,7 +316,7 @@ static void launch_watchpoints(char *buf, int mode, int len, else mode_str = "read";
- for (i = 0; i < 4; i++) { + for (i = 0; i < COUNT_WPS; i++) { set_breakpoint_addr(&dummy_var[i], i); toggle_breakpoint(i, mode, len, local, global, 1); ptrace(PTRACE_CONT, child_pid, NULL, 0); @@ -330,8 +332,15 @@ static void launch_watchpoints(char *buf, int mode, int len, static void launch_tests(void) { char buf[1024]; + unsigned int tests = 0; int len, local, global, i;
+ tests += 3 * COUNT_ISN_BPS; + tests += sizeof(long) / 2 * 3 * COUNT_WPS; + tests += sizeof(long) / 2 * 3 * COUNT_WPS; + tests += 2; + ksft_set_plan(tests); + /* Instruction breakpoints */ for (local = 0; local < 2; local++) { for (global = 0; global < 2; global++) { diff --git a/tools/testing/selftests/breakpoints/breakpoint_test_arm64.c b/tools/testing/selftests/breakpoints/breakpoint_test_arm64.c index 2d95e5adde72..ab59d814341a 100644 --- a/tools/testing/selftests/breakpoints/breakpoint_test_arm64.c +++ b/tools/testing/selftests/breakpoints/breakpoint_test_arm64.c @@ -118,7 +118,7 @@ static bool set_watchpoint(pid_t pid, int size, int wp) return false; }
-static bool run_test(int wr_size, int wp_size, int wr, int wp) +static bool arun_test(int wr_size, int wp_size, int wr, int wp) { int status; siginfo_t siginfo; @@ -214,6 +214,7 @@ int main(int argc, char **argv) bool result;
ksft_print_header(); + ksft_set_plan(213);
act.sa_handler = sigalrm; sigemptyset(&act.sa_mask); diff --git a/tools/testing/selftests/breakpoints/step_after_suspend_test.c b/tools/testing/selftests/breakpoints/step_after_suspend_test.c index f82dcc1f8841..cf868b5e00f7 100644 --- a/tools/testing/selftests/breakpoints/step_after_suspend_test.c +++ b/tools/testing/selftests/breakpoints/step_after_suspend_test.c @@ -173,6 +173,7 @@ int main(int argc, char **argv) int opt; bool do_suspend = true; bool succeeded = true; + unsigned int tests = 0; cpu_set_t available_cpus; int err; int cpu; @@ -191,6 +192,13 @@ int main(int argc, char **argv) } }
+ for (cpu = 0; cpu < CPU_SETSIZE; cpu++) { + if (!CPU_ISSET(cpu, &available_cpus)) + continue; + tests++; + } + ksft_set_plan(tests); + if (do_suspend) suspend();
diff --git a/tools/testing/selftests/capabilities/test_execve.c b/tools/testing/selftests/capabilities/test_execve.c index 3ab39a61b95b..df0ef02b4036 100644 --- a/tools/testing/selftests/capabilities/test_execve.c +++ b/tools/testing/selftests/capabilities/test_execve.c @@ -430,8 +430,6 @@ int main(int argc, char **argv) { char *tmp1, *tmp2, *our_path;
- ksft_print_header(); - /* Find our path */ tmp1 = strdup(argv[0]); if (!tmp1) @@ -445,6 +443,8 @@ int main(int argc, char **argv) mpid = getpid();
if (fork_wait()) { + ksft_print_header(); + ksft_set_plan(12); ksft_print_msg("[RUN]\t+++ Tests with uid == 0 +++\n"); return do_tests(0, our_path); } @@ -452,6 +452,8 @@ int main(int argc, char **argv) ksft_print_msg("==================================================\n");
if (fork_wait()) { + ksft_print_header(); + ksft_set_plan(9); ksft_print_msg("[RUN]\t+++ Tests with uid != 0 +++\n"); return do_tests(1, our_path); } diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi.c b/tools/testing/selftests/futex/functional/futex_requeue_pi.c index 54cd5c414e82..8d20957f7586 100644 --- a/tools/testing/selftests/futex/functional/futex_requeue_pi.c +++ b/tools/testing/selftests/futex/functional/futex_requeue_pi.c @@ -395,6 +395,7 @@ int main(int argc, char *argv[]) }
ksft_print_header(); + ksft_set_plan(1); ksft_print_msg("%s: Test requeue functionality\n", basename(argv[0])); ksft_print_msg( "\tArguments: broadcast=%d locked=%d owner=%d timeout=%ldns\n", diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c b/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c index 08187a16507f..742624c59ba7 100644 --- a/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c +++ b/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c @@ -79,6 +79,7 @@ int main(int argc, char *argv[]) }
ksft_print_header(); + ksft_set_plan(1); ksft_print_msg("%s: Detect mismatched requeue_pi operations\n", basename(argv[0]));
diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c b/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c index f0542a344d95..a0f5934707ff 100644 --- a/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c +++ b/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c @@ -144,6 +144,7 @@ int main(int argc, char *argv[]) }
ksft_print_header(); + ksft_set_plan(1); ksft_print_msg("%s: Test signal handling during requeue_pi\n", basename(argv[0])); ksft_print_msg("\tArguments: <none>\n"); diff --git a/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c b/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c index 6216de828093..a458d42ff86e 100644 --- a/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c +++ b/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c @@ -98,6 +98,7 @@ int main(int argc, char **argv) }
ksft_print_header(); + ksft_set_plan(1); ksft_print_msg( "%s: Test the futex value of private file mappings in FUTEX_WAIT\n", basename(argv[0])); diff --git a/tools/testing/selftests/futex/functional/futex_wait_timeout.c b/tools/testing/selftests/futex/functional/futex_wait_timeout.c index bab3dfe1787f..04b95478059c 100644 --- a/tools/testing/selftests/futex/functional/futex_wait_timeout.c +++ b/tools/testing/selftests/futex/functional/futex_wait_timeout.c @@ -69,6 +69,7 @@ int main(int argc, char *argv[]) }
ksft_print_header(); + ksft_set_plan(1); ksft_print_msg("%s: Block on a futex and wait for timeout\n", basename(argv[0])); ksft_print_msg("\tArguments: timeout=%ldns\n", timeout_ns); diff --git a/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c b/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c index 26975322545b..3a1d12a14921 100644 --- a/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c +++ b/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c @@ -100,6 +100,7 @@ int main(int argc, char **argv) }
ksft_print_header(); + ksft_set_plan(1); ksft_print_msg("%s: Test the uninitialized futex value in FUTEX_WAIT\n", basename(argv[0]));
diff --git a/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c b/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c index da15a63269b4..a34a6bbc30ce 100644 --- a/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c +++ b/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c @@ -65,6 +65,7 @@ int main(int argc, char *argv[]) }
ksft_print_header(); + ksft_set_plan(1); ksft_print_msg("%s: Test the unexpected futex value in FUTEX_WAIT\n", basename(argv[0]));
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 7f078e79a9fa..ec15c4f6af55 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -33,6 +33,7 @@ struct ksft_count { };
static struct ksft_count ksft_cnt; +static unsigned int ksft_plan;
static inline int ksft_test_num(void) { @@ -61,13 +62,21 @@ static inline void ksft_print_header(void) printf("TAP version 13\n"); }
+static inline void ksft_set_plan(unsigned int plan) +{ + ksft_plan = plan; + printf("1..%d\n", ksft_plan); +} + static inline void ksft_print_cnts(void) { + if (ksft_plan != ksft_test_num()) + printf("# Planned tests != run tests (%u != %u)\n", + ksft_plan, ksft_test_num()); printf("# Pass %d Fail %d Xfail %d Xpass %d Skip %d Error %d\n", ksft_cnt.ksft_pass, ksft_cnt.ksft_fail, ksft_cnt.ksft_xfail, ksft_cnt.ksft_xpass, ksft_cnt.ksft_xskip, ksft_cnt.ksft_error); - printf("1..%d\n", ksft_test_num()); }
static inline void ksft_print_msg(const char *msg, ...) @@ -172,7 +181,7 @@ static inline int ksft_exit_skip(const char *msg, ...) va_list args;
va_start(args, msg); - printf("not ok %d # SKIP ", ksft_test_num()); + printf("not ok %d # SKIP ", 1 + ksft_test_num()); vprintf(msg, args); va_end(args); } else { diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index 6793f8ecc8e7..70b4ddbf126b 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -304,6 +304,7 @@ static int test_membarrier_query(void) int main(int argc, char **argv) { ksft_print_header(); + ksft_set_plan(13);
test_membarrier_query(); test_membarrier(); diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index d59378a93782..5bae1792e3d6 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -371,6 +371,7 @@ static int test_pidfd_send_signal_syscall_support(void) int main(int argc, char **argv) { ksft_print_header(); + ksft_set_plan(4);
test_pidfd_send_signal_syscall_support(); test_pidfd_send_signal_simple_success(); diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c index 228c2ae47687..ad0f8df2ca0a 100644 --- a/tools/testing/selftests/sigaltstack/sas.c +++ b/tools/testing/selftests/sigaltstack/sas.c @@ -109,6 +109,7 @@ int main(void) int err;
ksft_print_header(); + ksft_set_plan(3);
sigemptyset(&act.sa_mask); act.sa_flags = SA_ONSTACK | SA_SIGINFO; diff --git a/tools/testing/selftests/sync/sync_test.c b/tools/testing/selftests/sync/sync_test.c index 7f7938263c5c..3824b66f41a0 100644 --- a/tools/testing/selftests/sync/sync_test.c +++ b/tools/testing/selftests/sync/sync_test.c @@ -86,6 +86,7 @@ int main(void) int err;
ksft_print_header(); + ksft_set_plan(3 + 7);
sync_api_supported();
On 4/24/19 5:12 PM, wrote:
This refactors the selftest Makefiles to extract the test running logic to be reused between "run_tests" and "emit_tests", while also fixing up the test output to be TAP version 13 compliant:
- added "plan" line
- fixed result line syntax
- moved all test output to be "# "-prefixed as TAP "diagnostic" lines
The prefixing code includes a fallback mode for limited execution environments.
Additionally, the plan lines are fixed for all callers of kselftest.h.
-Kees
Kees,
Just about to apply these to a topic branch to do testing and ran into checkpatch errors:
WARNING: line over 80 characters - a few WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #141: FILE: tools/testing/selftests/kselftest/runner.sh:2:
Can fix them and resend - SPDX one is my main concern.
The plan is to apply these to linux-kselftest ksft-tap-refactor topic first. I don't want to rush these until we do some testing.
thanks, -- Shuah
On Thu, Apr 25, 2019 at 9:52 AM shuah shuah@kernel.org wrote:
On 4/24/19 5:12 PM, wrote:
This refactors the selftest Makefiles to extract the test running logic to be reused between "run_tests" and "emit_tests", while also fixing up the test output to be TAP version 13 compliant:
- added "plan" line
- fixed result line syntax
- moved all test output to be "# "-prefixed as TAP "diagnostic" lines
The prefixing code includes a fallback mode for limited execution environments.
Additionally, the plan lines are fixed for all callers of kselftest.h.
-Kees
Kees,
Just about to apply these to a topic branch to do testing and ran into checkpatch errors:
WARNING: line over 80 characters - a few
I only saw one, which is on a string which kernel coding style says to leave unsplit:
WARNING: line over 80 characters #55: FILE: tools/testing/selftests/kselftest/runner.sh:19: + echo "$TEST_HDR_MSG: Warning: file $TEST is not executable, correct this."
WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #141: FILE: tools/testing/selftests/kselftest/runner.sh:2:
WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #37: FILE: tools/testing/selftests/kselftest/runner.sh:2: # SPDX-License-Identifier: GPL-2.0
This is a shell script. It can't be on line 1:
$ head -n3 tools/testing/selftests/kselftest/runner.sh #!/bin/sh # SPDX-License-Identifier: GPL-2.0 #
That looks like a bug in checkpatch not resetting the expected line or something.
Can fix them and resend - SPDX one is my main concern.
These appear to be false positives; I don't think I need to fix them? Let me know what you think.
The plan is to apply these to linux-kselftest ksft-tap-refactor topic first. I don't want to rush these until we do some testing.
Absolutely. :)
Thanks!
On Thu, Apr 25, 2019 at 10:05 AM Kees Cook keescook@chromium.org wrote:
WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #37: FILE: tools/testing/selftests/kselftest/runner.sh:2: # SPDX-License-Identifier: GPL-2.0
This is a shell script. It can't be on line 1:
$ head -n3 tools/testing/selftests/kselftest/runner.sh #!/bin/sh # SPDX-License-Identifier: GPL-2.0 #
That looks like a bug in checkpatch not resetting the expected line or something.
It doesn't like patch 3 and doesn't notice that diff offset starts at line 2:
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index e1117d703887..f12b0a631273 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -2,17 +2,20 @@ # SPDX-License-Identifier: GPL-2.0 # # Runs a set of tests in a given subdirectory. +export KSFT_TAP_LEVEL=1
Joe, looks like the problem is here:
if ($realline == $checklicenseline) { realline == 2, checklicenseline == 1 so this is skipped, including the "#!/" check to move checklicenseline to 2. then: if ($realline != $checklicenseline && $rawline =~ /\bSPDX-License-Identifier:/ && realline == 2, checklicenseline == 1 throws warning.
Seems like checklicenseline should be unconditionally set to 2 for ".sh" files? I don't see a way to fix this for just missing the #!/ line from a context diff, though...
On 4/25/19 11:05 AM, Kees Cook wrote:
On Thu, Apr 25, 2019 at 9:52 AM shuah shuah@kernel.org wrote:
On 4/24/19 5:12 PM, wrote:
This refactors the selftest Makefiles to extract the test running logic to be reused between "run_tests" and "emit_tests", while also fixing up the test output to be TAP version 13 compliant:
- added "plan" line
- fixed result line syntax
- moved all test output to be "# "-prefixed as TAP "diagnostic" lines
The prefixing code includes a fallback mode for limited execution environments.
Additionally, the plan lines are fixed for all callers of kselftest.h.
-Kees
Kees,
Just about to apply these to a topic branch to do testing and ran into checkpatch errors:
WARNING: line over 80 characters - a few
I only saw one, which is on a string which kernel coding style says to leave unsplit:
WARNING: line over 80 characters #55: FILE: tools/testing/selftests/kselftest/runner.sh:19:
echo "$TEST_HDR_MSG: Warning: file $TEST is notexecutable, correct this."
WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #141: FILE: tools/testing/selftests/kselftest/runner.sh:2:
WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #37: FILE: tools/testing/selftests/kselftest/runner.sh:2: # SPDX-License-Identifier: GPL-2.0
This is a shell script. It can't be on line 1:
$ head -n3 tools/testing/selftests/kselftest/runner.sh #!/bin/sh # SPDX-License-Identifier: GPL-2.0 #
That looks like a bug in checkpatch not resetting the expected line or something.
Can fix them and resend - SPDX one is my main concern.
These appear to be false positives; I don't think I need to fix them? Let me know what you think.
The plan is to apply these to linux-kselftest ksft-tap-refactor topic first. I don't want to rush these until we do some testing.
Absolutely. :)
Thanks!
Kees,
Pushed all 8 patches to https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest ksft-tap-refactor topic branch
I will have time to test tomorrow.
thanks, -- Shuah
On Thu, Apr 25, 2019 at 1:39 PM shuah shuah@kernel.org wrote:
Pushed all 8 patches to https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest ksft-tap-refactor topic branch
I will have time to test tomorrow.
Awesome! I'm excited. :)
Next I want to update kselftest_harness.h to do header/plan printing...
linux-kselftest-mirror@lists.linaro.org
-
Kees Cook -
shuah