According to the TAP specification, a skipped test must be marked as "ok" and annotated with the SKIP directive, for example

ok 23 # skip Insufficient flogiston pressure. (https://testanything.org/tap-specification.html)

Fix the kselftest infrastructure to match this.

For ksft_exit_skip, it is preferrable to emit a dummy plan line that indicates the whole test was skipped, but this is not always possible because of ksft_exit_skip being used as a "shortcut" by the tests. In that case, print the test counts and a normal "ok" line. The format is now the same independent of whether msg is NULL or not (but it is never NULL in any caller right now).

Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- tools/testing/selftests/kselftest.h | 28 +++++++++++++++------ tools/testing/selftests/kselftest/runner.sh | 2 +- 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 862eee734553..1b0075359734 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -128,7 +128,7 @@ static inline void ksft_test_result_skip(const char *msg, ...) ksft_cnt.ksft_xskip++;

va_start(args, msg); - printf("not ok %d # SKIP ", ksft_test_num()); + printf("ok %d # SKIP ", ksft_test_num()); errno = saved_errno; vprintf(msg, args); va_end(args); @@ -190,18 +190,30 @@ static inline int ksft_exit_xpass(void)

static inline int ksft_exit_skip(const char *msg, ...) { - if (msg) { - int saved_errno = errno; - va_list args; + int saved_errno = errno; + va_list args;

- va_start(args, msg); - printf("not ok %d # SKIP ", 1 + ksft_test_num()); + va_start(args, msg); + + /* + * FIXME: several tests misuse ksft_exit_skip so produce + * something sensible if some tests have already been run + * or a plan has been printed. Those tests should use + * ksft_test_result_skip or ksft_exit_fail_msg instead. + */ + if (ksft_plan || ksft_test_num()) { + ksft_cnt.ksft_xskip++; + printf("ok %d # SKIP ", 1 + ksft_test_num()); + } else { + printf("1..0 # SKIP "); + } + if (msg) { errno = saved_errno; vprintf(msg, args); va_end(args); - } else { - ksft_print_cnts(); } + if (ksft_test_num()) + ksft_print_cnts(); exit(KSFT_SKIP); }

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index 676b3a8b114d..f4815cbcd60f 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -77,7 +77,7 @@ run_one() echo "ok $test_num $TEST_HDR_MSG") || (rc=$?; \ if [ $rc -eq $skip_rc ]; then \ - echo "not ok $test_num $TEST_HDR_MSG # SKIP" + echo "ok $test_num $TEST_HDR_MSG # SKIP" elif [ $rc -eq $timeout_rc ]; then \ echo "#" echo "not ok $test_num $TEST_HDR_MSG # TIMEOUT"

Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip for the individual tests. The call in suspend() is fine, but ksft_set_plan should be after it.

Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- .../breakpoints/step_after_suspend_test.c | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/breakpoints/step_after_suspend_test.c b/tools/testing/selftests/breakpoints/step_after_suspend_test.c index 983ee6182e25..2cf6f10ab7c4 100644 --- a/tools/testing/selftests/breakpoints/step_after_suspend_test.c +++ b/tools/testing/selftests/breakpoints/step_after_suspend_test.c @@ -47,7 +47,7 @@ void child(int cpu) _exit(0); }

-bool run_test(int cpu) +int run_test(int cpu) { int status; pid_t pid = fork(); @@ -55,7 +55,7 @@ bool run_test(int cpu)

if (pid < 0) { ksft_print_msg("fork() failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (pid == 0) child(cpu); @@ -63,67 +63,68 @@ bool run_test(int cpu) wpid = waitpid(pid, &status, __WALL); if (wpid != pid) { ksft_print_msg("waitpid() failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (!WIFSTOPPED(status)) { ksft_print_msg("child did not stop: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (WSTOPSIG(status) != SIGSTOP) { ksft_print_msg("child did not stop with SIGSTOP: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; }

if (ptrace(PTRACE_SINGLESTEP, pid, NULL, NULL) < 0) { if (errno == EIO) { - ksft_exit_skip( + ksft_print_msg( "ptrace(PTRACE_SINGLESTEP) not supported on this architecture: %s\n", strerror(errno)); + return KSFT_SKIP; } ksft_print_msg("ptrace(PTRACE_SINGLESTEP) failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; }

wpid = waitpid(pid, &status, __WALL); if (wpid != pid) { ksft_print_msg("waitpid() failed: $s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (WIFEXITED(status)) { ksft_print_msg("child did not single-step: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (!WIFSTOPPED(status)) { ksft_print_msg("child did not stop: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (WSTOPSIG(status) != SIGTRAP) { ksft_print_msg("child did not stop with SIGTRAP: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; }

if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) { ksft_print_msg("ptrace(PTRACE_CONT) failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; }

wpid = waitpid(pid, &status, __WALL); if (wpid != pid) { ksft_print_msg("waitpid() failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (!WIFEXITED(status)) { ksft_print_msg("child did not exit after PTRACE_CONT: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; }

- return true; + return KSFT_PASS; }

void suspend(void) @@ -192,23 +193,29 @@ int main(int argc, char **argv) continue; tests++; } - ksft_set_plan(tests);

if (do_suspend) suspend();

+ ksft_set_plan(tests); for (cpu = 0; cpu < CPU_SETSIZE; cpu++) { - bool test_success; + int test_success;

if (!CPU_ISSET(cpu, &available_cpus)) continue;

test_success = run_test(cpu); - if (test_success) { + switch (test_success) { + case KSFT_PASS: ksft_test_result_pass("CPU %d\n", cpu); - } else { + break; + case KSFT_SKIP: + ksft_test_result_skip("CPU %d\n", cpu); + break; + case KSFT_FAIL: ksft_test_result_fail("CPU %d\n", cpu); succeeded = false; + break; } }

Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip when possible, or just bail out if memory corruption is detected.

Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- tools/testing/selftests/sigaltstack/sas.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c index ad0f8df2ca0a..8934a3766d20 100644 --- a/tools/testing/selftests/sigaltstack/sas.c +++ b/tools/testing/selftests/sigaltstack/sas.c @@ -71,7 +71,7 @@ void my_usr1(int sig, siginfo_t *si, void *u) swapcontext(&sc, &uc); ksft_print_msg("%s\n", p->msg); if (!p->flag) { - ksft_exit_skip("[RUN]\tAborting\n"); + ksft_exit_fail_msg("[RUN]\tAborting\n"); exit(EXIT_FAILURE); } } @@ -144,7 +144,7 @@ int main(void) err = sigaltstack(&stk, NULL); if (err) { if (errno == EINVAL) { - ksft_exit_skip( + ksft_test_result_skip( "[NOTE]\tThe running kernel doesn't support SS_AUTODISARM\n"); /* * If test cases for the !SS_AUTODISARM variant were

On Mon, Jun 22, 2020 at 08:15:45PM -0400, Paolo Bonzini wrote:

Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip instead.

The plan passed to ksft_set_plan was wrong, too, so fix it while at it.

Signed-off-by: Paolo Bonzini pbonzini@redhat.com

Thanks for the patch! Hm, this series misses a bunch of Cces for the maintainers of these files... (Also note that Kees has a/some series with most of us Cced that might conflict with some of these changes. But not sure rn.)

A comment below.

tools/testing/selftests/pidfd/pidfd_test.c | 39 ++++++++++++++++++---- 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 7aff2d3b42c0..380c6314e6a2 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -8,6 +8,7 @@ #include <sched.h> #include <signal.h> #include <stdio.h> +#include <stdbool.h> #include <stdlib.h> #include <string.h> #include <syscall.h> @@ -27,6 +28,8 @@ #define MAX_EVENTS 5 +static bool have_pidfd_send_signal = false;

static pid_t pidfd_clone(int flags, int *pidfd, int (*fn)(void *)) { size_t stack_size = 1024; @@ -56,6 +59,13 @@ static int test_pidfd_send_signal_simple_success(void) int pidfd, ret; const char *test_name = "pidfd_send_signal send SIGUSR1";

• if (!have_pidfd_send_signal) {

• ksft_test_result_skip(
  

• 	"%s test: pidfd_send_signal() syscall not supported\n",
  

• 	test_name);
  

• return 0;
  

• }
• pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC); if (pidfd < 0) ksft_exit_fail_msg(

@@ -86,6 +96,13 @@ static int test_pidfd_send_signal_exited_fail(void) pid_t pid; const char *test_name = "pidfd_send_signal signal exited process";

• if (!have_pidfd_send_signal) {

• ksft_test_result_skip(
  

• 	"%s test: pidfd_send_signal() syscall not supported\n",
  

• 	test_name);
  

• return 0;
  

• }
• pid = fork(); if (pid < 0) ksft_exit_fail_msg("%s test: Failed to create new process\n",

@@ -137,6 +154,13 @@ static int test_pidfd_send_signal_recycled_pid_fail(void) pid_t pid1; const char *test_name = "pidfd_send_signal signal recycled pid";

• if (!have_pidfd_send_signal) {

• ksft_test_result_skip(
  

• 	"%s test: pidfd_send_signal() syscall not supported\n",
  

• 	test_name);
  

• return 0;
  

• }
• ret = unshare(CLONE_NEWPID); if (ret < 0) ksft_exit_fail_msg("%s test: Failed to unshare pid namespace\n",

@@ -325,13 +349,16 @@ static int test_pidfd_send_signal_syscall_support(void) ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0); if (ret < 0) {

• if (errno == ENOSYS)
  

• 	ksft_exit_skip(
  

• if (errno == ENOSYS) {
  

• 	ksft_test_result_skip(
  	"%s test: pidfd_send_signal() syscall not supported\n",
  	test_name);
  

If pidfd_send_signal() is not supported, you're falling through and then you're reporting:

ok 5 # SKIP pidfd_send_signal check for support test: pidfd_send_signal() syscall not supported ok 6 pidfd_send_signal check for support test: pidfd_send_signal() syscall is supported. Tests can be executed

which seems wrong.

• ksft_exit_fail_msg("%s test: Failed to send signal\n",
  

• 		   test_name);
  

• } else {
  

• 	ksft_exit_fail_msg("%s test: Failed to send signal\n",
  

• 			   test_name);
  

• }
  

• } else {

• have_pidfd_send_signal = true;
  

  }

close(pidfd); @@ -521,7 +548,7 @@ static void test_pidfd_poll_leader_exit(int use_waitpid) int main(int argc, char **argv) { ksft_print_header();

• ksft_set_plan(4);

• ksft_set_plan(8);

test_pidfd_poll_exec(0); test_pidfd_poll_exec(1); -- 2.26.2