* [RFC 0/7] selftests: kselftest_harness: use common result printing helper
@ 2024-02-16 0:41 Jakub Kicinski
2024-02-16 0:41 ` [RFC 1/7] selftests: kselftest_harness: generate test name once Jakub Kicinski
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
Add a common result printing helper and always include test name
in the result line. Previously when SKIP or XPASS would happen
we printed:
ok 1 # SKIP unknown
without the test name. Now we'll print:
ok 1 global.no_pad # SKIP unknown
This appears to be more inline with:
https://docs.kernel.org/dev-tools/ktap.html
and makes parsing results easier.
First 3 patches rearrange kselftest_harness to use exit code
as an enum rather than separate passed/skip/xfail members.
Rest of the series builds a ksft_test_result_code() helper.
This series is on top of:
https://lore.kernel.org/all/20240216002619.1999225-1-kuba@kernel.org/
Jakub Kicinski (7):
selftests: kselftest_harness: generate test name once
selftests: kselftest_harness: save full exit code in metadata
selftests: kselftest_harness: use exit code to store skip and xfail
selftests: kselftest: add ksft_test_result_code(), handling all exit
codes
selftests: kselftest_harness: print test name for SKIP and XFAIL
selftests: kselftest_harness: let ksft_test_result_code() handle line
termination
selftests: kselftest_harness: let PASS / FAIL provide diagnostic
tools/testing/selftests/kselftest.h | 45 ++++++++++
tools/testing/selftests/kselftest_harness.h | 96 ++++++++++-----------
tools/testing/selftests/net/tls.c | 2 +-
3 files changed, 91 insertions(+), 52 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC 1/7] selftests: kselftest_harness: generate test name once
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
@ 2024-02-16 0:41 ` Jakub Kicinski
2024-02-16 21:24 ` Kees Cook
2024-02-16 0:41 ` [RFC 2/7] selftests: kselftest_harness: save full exit code in metadata Jakub Kicinski
` (6 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
Since we added variant support generating full test case
name takes 4 string arguments. We're about to need it
in another two places. Stop the duplication and print
once into a temporary buffer.
Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/kselftest_harness.h | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 04177813930b..b271cb721b81 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -56,6 +56,7 @@
#include <asm/types.h>
#include <ctype.h>
#include <errno.h>
+#include <limits.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
@@ -1140,6 +1141,8 @@ void __run_test(struct __fixture_metadata *f,
struct __fixture_variant_metadata *variant,
struct __test_metadata *t)
{
+ char test_name[LINE_MAX];
+
/* reset test struct */
t->passed = 1;
t->skip = 0;
@@ -1149,8 +1152,10 @@ void __run_test(struct __fixture_metadata *f,
memset(t->results->reason, 0, sizeof(t->results->reason));
t->results->step = 1;
- ksft_print_msg(" RUN %s%s%s.%s ...\n",
- f->name, variant->name[0] ? "." : "", variant->name, t->name);
+ snprintf(test_name, sizeof(test_name), "%s%s%s.%s",
+ f->name, variant->name[0] ? "." : "", variant->name, t->name);
+
+ ksft_print_msg(" RUN %s ...\n", test_name);
/* Make sure output buffers are flushed before fork */
fflush(stdout);
@@ -1174,8 +1179,8 @@ void __run_test(struct __fixture_metadata *f,
} else {
__wait_for_test(t);
}
- ksft_print_msg(" %4s %s%s%s.%s\n", t->passed ? "OK" : "FAIL",
- f->name, variant->name[0] ? "." : "", variant->name, t->name);
+ ksft_print_msg(" %4s %s\n",
+ t->passed ? "OK" : "FAIL", test_name);
if (t->skip)
ksft_test_result_skip("%s\n", t->results->reason[0] ?
@@ -1184,8 +1189,7 @@ void __run_test(struct __fixture_metadata *f,
ksft_test_result_xfail("%s\n", t->results->reason[0] ?
t->results->reason : "unknown");
else
- ksft_test_result(t->passed, "%s%s%s.%s\n",
- f->name, variant->name[0] ? "." : "", variant->name, t->name);
+ ksft_test_result(t->passed, "%s\n", test_name);
}
static int test_harness_run(int argc, char **argv)
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 2/7] selftests: kselftest_harness: save full exit code in metadata
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
2024-02-16 0:41 ` [RFC 1/7] selftests: kselftest_harness: generate test name once Jakub Kicinski
@ 2024-02-16 0:41 ` Jakub Kicinski
2024-02-16 0:41 ` [RFC 3/7] selftests: kselftest_harness: use exit code to store skip and xfail Jakub Kicinski
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
Instead of tracking passed = 0/1 rename the field to exit_code
and invert the values so that they match the KSFT_* exit codes.
This will allow us to fold SKIP / XFAIL into the same value.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/kselftest_harness.h | 52 ++++++++++++---------
tools/testing/selftests/net/tls.c | 2 +-
2 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index b271cb721b81..70366864ffd9 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -136,7 +136,7 @@
fprintf(TH_LOG_STREAM, "# SKIP %s\n", \
_metadata->results->reason); \
} \
- _metadata->passed = 1; \
+ _metadata->exit_code = KSFT_PASS; \
_metadata->skip = 1; \
_metadata->trigger = 0; \
statement; \
@@ -163,7 +163,7 @@
fprintf(TH_LOG_STREAM, "# XFAIL %s\n", \
_metadata->results->reason); \
} \
- _metadata->passed = 1; \
+ _metadata->exit_code = KSFT_PASS; \
_metadata->xfail = 1; \
_metadata->trigger = 0; \
statement; \
@@ -416,7 +416,7 @@
if (setjmp(_metadata->env) == 0) { \
fixture_name##_setup(_metadata, &self, variant->data); \
/* Let setup failure terminate early. */ \
- if (!_metadata->passed || _metadata->skip) \
+ if (!__test_passed(_metadata) || _metadata->skip) \
return; \
_metadata->setup_completed = true; \
fixture_name##_##test_name(_metadata, &self, variant->data); \
@@ -723,7 +723,7 @@
__bail(_assert, _metadata))
#define __INC_STEP(_metadata) \
- if (_metadata->passed) \
+ if (__test_passed(_metadata)) \
_metadata->results->step++;
#define is_signed_type(var) (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
@@ -769,7 +769,7 @@
break; \
} \
} \
- _metadata->passed = 0; \
+ _metadata->exit_code = KSFT_FAIL; \
/* Ensure the optional handler is triggered */ \
_metadata->trigger = 1; \
} \
@@ -781,7 +781,7 @@
if (_assert) __INC_STEP(_metadata); \
if (!(strcmp(__exp, __seen) _t 0)) { \
__TH_LOG("Expected '%s' %s '%s'.", __exp, #_t, __seen); \
- _metadata->passed = 0; \
+ _metadata->exit_code = KSFT_FAIL; \
_metadata->trigger = 1; \
} \
} while (0); OPTIONAL_HANDLER(_assert)
@@ -860,7 +860,7 @@ struct __test_metadata {
pid_t pid; /* pid of test when being run */
struct __fixture_metadata *fixture;
int termsig;
- int passed;
+ int exit_code;
int skip; /* did SKIP get used? */
int xfail; /* did XFAIL get used? */
int trigger; /* extra handler after the evaluation */
@@ -874,6 +874,12 @@ struct __test_metadata {
struct __test_metadata *prev, *next;
};
+static inline bool __test_passed(struct __test_metadata *metadata)
+{
+ return metadata->exit_code != KSFT_FAIL &&
+ metadata->exit_code <= KSFT_SKIP;
+}
+
/*
* Since constructors are called in reverse order, reverse the test
* list so tests are run in source declaration order.
@@ -941,7 +947,7 @@ void __wait_for_test(struct __test_metadata *t)
int status;
if (sigaction(SIGALRM, &action, &saved_action)) {
- t->passed = 0;
+ t->exit_code = KSFT_FAIL;
fprintf(TH_LOG_STREAM,
"# %s: unable to install SIGALRM handler\n",
t->name);
@@ -953,7 +959,7 @@ void __wait_for_test(struct __test_metadata *t)
waitpid(t->pid, &status, 0);
alarm(0);
if (sigaction(SIGALRM, &saved_action, NULL)) {
- t->passed = 0;
+ t->exit_code = KSFT_FAIL;
fprintf(TH_LOG_STREAM,
"# %s: unable to uninstall SIGALRM handler\n",
t->name);
@@ -962,19 +968,19 @@ void __wait_for_test(struct __test_metadata *t)
__active_test = NULL;
if (t->timed_out) {
- t->passed = 0;
+ t->exit_code = KSFT_FAIL;
fprintf(TH_LOG_STREAM,
"# %s: Test terminated by timeout\n", t->name);
} else if (WIFEXITED(status)) {
if (WEXITSTATUS(status) == KSFT_SKIP) {
/* SKIP */
- t->passed = 1;
+ t->exit_code = KSFT_PASS;
t->skip = 1;
} else if (WEXITSTATUS(status) == KSFT_XFAIL) {
- t->passed = 1;
+ t->exit_code = KSFT_PASS;
t->xfail = 1;
} else if (t->termsig != -1) {
- t->passed = 0;
+ t->exit_code = KSFT_FAIL;
fprintf(TH_LOG_STREAM,
"# %s: Test exited normally instead of by signal (code: %d)\n",
t->name,
@@ -983,11 +989,11 @@ void __wait_for_test(struct __test_metadata *t)
switch (WEXITSTATUS(status)) {
/* Success */
case KSFT_PASS:
- t->passed = 1;
+ t->exit_code = KSFT_PASS;
break;
/* Other failure, assume step report. */
default:
- t->passed = 0;
+ t->exit_code = KSFT_FAIL;
fprintf(TH_LOG_STREAM,
"# %s: Test failed at step #%d\n",
t->name,
@@ -995,13 +1001,13 @@ void __wait_for_test(struct __test_metadata *t)
}
}
} else if (WIFSIGNALED(status)) {
- t->passed = 0;
+ t->exit_code = KSFT_FAIL;
if (WTERMSIG(status) == SIGABRT) {
fprintf(TH_LOG_STREAM,
"# %s: Test terminated by assertion\n",
t->name);
} else if (WTERMSIG(status) == t->termsig) {
- t->passed = 1;
+ t->exit_code = KSFT_PASS;
} else {
fprintf(TH_LOG_STREAM,
"# %s: Test terminated unexpectedly by signal %d\n",
@@ -1144,7 +1150,7 @@ void __run_test(struct __fixture_metadata *f,
char test_name[LINE_MAX];
/* reset test struct */
- t->passed = 1;
+ t->exit_code = KSFT_PASS;
t->skip = 0;
t->xfail = 0;
t->trigger = 0;
@@ -1164,7 +1170,7 @@ void __run_test(struct __fixture_metadata *f,
t->pid = fork();
if (t->pid < 0) {
ksft_print_msg("ERROR SPAWNING TEST CHILD\n");
- t->passed = 0;
+ t->exit_code = KSFT_FAIL;
} else if (t->pid == 0) {
setpgrp();
t->fn(t, variant);
@@ -1172,7 +1178,7 @@ void __run_test(struct __fixture_metadata *f,
_exit(KSFT_SKIP);
if (t->xfail)
_exit(KSFT_XFAIL);
- if (t->passed)
+ if (__test_passed(t))
_exit(KSFT_PASS);
/* Something else happened. */
_exit(KSFT_FAIL);
@@ -1180,7 +1186,7 @@ void __run_test(struct __fixture_metadata *f,
__wait_for_test(t);
}
ksft_print_msg(" %4s %s\n",
- t->passed ? "OK" : "FAIL", test_name);
+ __test_passed(t) ? "OK" : "FAIL", test_name);
if (t->skip)
ksft_test_result_skip("%s\n", t->results->reason[0] ?
@@ -1189,7 +1195,7 @@ void __run_test(struct __fixture_metadata *f,
ksft_test_result_xfail("%s\n", t->results->reason[0] ?
t->results->reason : "unknown");
else
- ksft_test_result(t->passed, "%s\n", test_name);
+ ksft_test_result(__test_passed(t), "%s\n", test_name);
}
static int test_harness_run(int argc, char **argv)
@@ -1237,7 +1243,7 @@ static int test_harness_run(int argc, char **argv)
t->results = results;
__run_test(f, v, t);
t->results = NULL;
- if (t->passed)
+ if (__test_passed(t))
pass_count++;
else
ret = 1;
diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 49c84602707f..046d1ccedcf3 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -1882,7 +1882,7 @@ TEST_F(tls_err, poll_partial_rec_async)
pfd.events = POLLIN;
EXPECT_EQ(poll(&pfd, 1, 20), 1);
- exit(!_metadata->passed);
+ exit(!__test_passed(_metadata));
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 3/7] selftests: kselftest_harness: use exit code to store skip and xfail
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
2024-02-16 0:41 ` [RFC 1/7] selftests: kselftest_harness: generate test name once Jakub Kicinski
2024-02-16 0:41 ` [RFC 2/7] selftests: kselftest_harness: save full exit code in metadata Jakub Kicinski
@ 2024-02-16 0:41 ` Jakub Kicinski
2024-02-16 0:41 ` [RFC 4/7] selftests: kselftest: add ksft_test_result_code(), handling all exit codes Jakub Kicinski
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
We always use skip / xfail with combination of exit_code being 0
(KSFT_PASS). This are just basic KSFT / KTAP semantics.
Store the right KSFT_* code in exit_code directly.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/kselftest_harness.h | 35 ++++++---------------
1 file changed, 9 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 70366864ffd9..6923cd7060fc 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -136,8 +136,7 @@
fprintf(TH_LOG_STREAM, "# SKIP %s\n", \
_metadata->results->reason); \
} \
- _metadata->exit_code = KSFT_PASS; \
- _metadata->skip = 1; \
+ _metadata->exit_code = KSFT_SKIP; \
_metadata->trigger = 0; \
statement; \
} while (0)
@@ -163,8 +162,7 @@
fprintf(TH_LOG_STREAM, "# XFAIL %s\n", \
_metadata->results->reason); \
} \
- _metadata->exit_code = KSFT_PASS; \
- _metadata->xfail = 1; \
+ _metadata->exit_code = KSFT_XFAIL; \
_metadata->trigger = 0; \
statement; \
} while (0)
@@ -416,7 +414,7 @@
if (setjmp(_metadata->env) == 0) { \
fixture_name##_setup(_metadata, &self, variant->data); \
/* Let setup failure terminate early. */ \
- if (!__test_passed(_metadata) || _metadata->skip) \
+ if (_metadata->exit_code) \
return; \
_metadata->setup_completed = true; \
fixture_name##_##test_name(_metadata, &self, variant->data); \
@@ -861,8 +859,6 @@ struct __test_metadata {
struct __fixture_metadata *fixture;
int termsig;
int exit_code;
- int skip; /* did SKIP get used? */
- int xfail; /* did XFAIL get used? */
int trigger; /* extra handler after the evaluation */
int timeout; /* seconds to wait for test timeout */
bool timed_out; /* did this test timeout instead of exiting? */
@@ -972,13 +968,9 @@ void __wait_for_test(struct __test_metadata *t)
fprintf(TH_LOG_STREAM,
"# %s: Test terminated by timeout\n", t->name);
} else if (WIFEXITED(status)) {
- if (WEXITSTATUS(status) == KSFT_SKIP) {
- /* SKIP */
- t->exit_code = KSFT_PASS;
- t->skip = 1;
- } else if (WEXITSTATUS(status) == KSFT_XFAIL) {
- t->exit_code = KSFT_PASS;
- t->xfail = 1;
+ if (WEXITSTATUS(status) == KSFT_SKIP ||
+ WEXITSTATUS(status) == KSFT_XFAIL) {
+ t->exit_code = WEXITSTATUS(status);
} else if (t->termsig != -1) {
t->exit_code = KSFT_FAIL;
fprintf(TH_LOG_STREAM,
@@ -1151,8 +1143,6 @@ void __run_test(struct __fixture_metadata *f,
/* reset test struct */
t->exit_code = KSFT_PASS;
- t->skip = 0;
- t->xfail = 0;
t->trigger = 0;
t->no_print = 0;
memset(t->results->reason, 0, sizeof(t->results->reason));
@@ -1174,24 +1164,17 @@ void __run_test(struct __fixture_metadata *f,
} else if (t->pid == 0) {
setpgrp();
t->fn(t, variant);
- if (t->skip)
- _exit(KSFT_SKIP);
- if (t->xfail)
- _exit(KSFT_XFAIL);
- if (__test_passed(t))
- _exit(KSFT_PASS);
- /* Something else happened. */
- _exit(KSFT_FAIL);
+ _exit(t->exit_code);
} else {
__wait_for_test(t);
}
ksft_print_msg(" %4s %s\n",
__test_passed(t) ? "OK" : "FAIL", test_name);
- if (t->skip)
+ if (t->exit_code == KSFT_SKIP)
ksft_test_result_skip("%s\n", t->results->reason[0] ?
t->results->reason : "unknown");
- else if (t->xfail)
+ else if (t->exit_code == KSFT_XFAIL)
ksft_test_result_xfail("%s\n", t->results->reason[0] ?
t->results->reason : "unknown");
else
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 4/7] selftests: kselftest: add ksft_test_result_code(), handling all exit codes
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
` (2 preceding siblings ...)
2024-02-16 0:41 ` [RFC 3/7] selftests: kselftest_harness: use exit code to store skip and xfail Jakub Kicinski
@ 2024-02-16 0:41 ` Jakub Kicinski
2024-02-16 0:41 ` [RFC 5/7] selftests: kselftest_harness: print test name for SKIP and XFAIL Jakub Kicinski
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
For generic test harness code it's more useful to deal with exit
codes directly, rather than having to switch on them and call
the right ksft_test_result_*() helper. Add such function to kselftest.h.
Note that "directive" and "diagnostic" are what ktap docs call
those parts of the message.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/kselftest.h | 39 +++++++++++++++++++++
tools/testing/selftests/kselftest_harness.h | 14 ++++----
2 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index a781e6311810..12ad7f8dfe3a 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -25,6 +25,7 @@
* ksft_test_result_skip(fmt, ...);
* ksft_test_result_xfail(fmt, ...);
* ksft_test_result_error(fmt, ...);
+ * ksft_test_result_code(exit_code, test_name, fmt, ...);
*
* When all tests are finished, clean up and exit the program with one of:
*
@@ -254,6 +255,44 @@ static inline __printf(1, 2) void ksft_test_result_error(const char *msg, ...)
va_end(args);
}
+static inline __printf(2, 3)
+void ksft_test_result_code(int exit_code, const char *msg, ...)
+{
+ const char *tap_code = "ok";
+ const char *directive = "";
+ int saved_errno = errno;
+ va_list args;
+
+ switch (exit_code) {
+ case KSFT_PASS:
+ ksft_cnt.ksft_pass++;
+ break;
+ case KSFT_XFAIL:
+ directive = " # XFAIL ";
+ ksft_cnt.ksft_xfail++;
+ break;
+ case KSFT_XPASS:
+ directive = " # XPASS ";
+ ksft_cnt.ksft_xpass++;
+ break;
+ case KSFT_SKIP:
+ directive = " # SKIP ";
+ ksft_cnt.ksft_xskip++;
+ break;
+ case KSFT_FAIL:
+ default:
+ tap_code = "not ok";
+ ksft_cnt.ksft_fail++;
+ break;
+ }
+
+ va_start(args, msg);
+ printf("%s %u%s", tap_code, ksft_test_num(), directive);
+ errno = saved_errno;
+ vprintf(msg, args);
+ va_end(args);
+}
+
static inline int ksft_exit_pass(void)
{
ksft_print_cnts();
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 6923cd7060fc..79a3fec0cefa 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -1140,6 +1140,7 @@ void __run_test(struct __fixture_metadata *f,
struct __test_metadata *t)
{
char test_name[LINE_MAX];
+ const char *diagnostic;
/* reset test struct */
t->exit_code = KSFT_PASS;
@@ -1171,12 +1172,13 @@ void __run_test(struct __fixture_metadata *f,
ksft_print_msg(" %4s %s\n",
__test_passed(t) ? "OK" : "FAIL", test_name);
- if (t->exit_code == KSFT_SKIP)
- ksft_test_result_skip("%s\n", t->results->reason[0] ?
- t->results->reason : "unknown");
- else if (t->exit_code == KSFT_XFAIL)
- ksft_test_result_xfail("%s\n", t->results->reason[0] ?
- t->results->reason : "unknown");
+ if (t->results->reason[0])
+ diagnostic = t->results->reason;
+ else
+ diagnostic = "unknown";
+
+ if (t->exit_code == KSFT_SKIP || t->exit_code == KSFT_XFAIL)
+ ksft_test_result_code(t->exit_code, "%s\n", diagnostic);
else
ksft_test_result(__test_passed(t), "%s\n", test_name);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 5/7] selftests: kselftest_harness: print test name for SKIP and XFAIL
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
` (3 preceding siblings ...)
2024-02-16 0:41 ` [RFC 4/7] selftests: kselftest: add ksft_test_result_code(), handling all exit codes Jakub Kicinski
@ 2024-02-16 0:41 ` Jakub Kicinski
2024-02-16 0:41 ` [RFC 6/7] selftests: kselftest_harness: let ksft_test_result_code() handle line termination Jakub Kicinski
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
Jakub points out that for parsers it's rather useful to always
have the test name on the result line. Currently if we SKIP
or XFAIL, we will print:
ok 17 # SKIP SCTP doesn't support IP_BIND_ADDRESS_NO_PORT
^
no test name
Always print the test name.
KTAP format seems to allow or even call for it, per:
https://docs.kernel.org/dev-tools/ktap.html
Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
Link: https://lore.kernel.org/all/87jzn6lnou.fsf@cloudflare.com/
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/kselftest.h | 7 ++++---
tools/testing/selftests/kselftest_harness.h | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index 12ad7f8dfe3a..25e29626566e 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -255,8 +255,9 @@ static inline __printf(1, 2) void ksft_test_result_error(const char *msg, ...)
va_end(args);
}
-static inline __printf(2, 3)
-void ksft_test_result_code(int exit_code, const char *msg, ...)
+static inline __printf(3, 4)
+void ksft_test_result_code(int exit_code, const char *test_name,
+ const char *msg, ...)
{
const char *tap_code = "ok";
const char *directive = "";
@@ -287,7 +288,7 @@ void ksft_test_result_code(int exit_code, const char *msg, ...)
}
va_start(args, msg);
- printf("%s %u%s", tap_code, ksft_test_num(), directive);
+ printf("%s %u %s%s", tap_code, ksft_test_num(), test_name, directive);
errno = saved_errno;
vprintf(msg, args);
va_end(args);
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 79a3fec0cefa..beae50fd5ac3 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -1178,7 +1178,8 @@ void __run_test(struct __fixture_metadata *f,
diagnostic = "unknown";
if (t->exit_code == KSFT_SKIP || t->exit_code == KSFT_XFAIL)
- ksft_test_result_code(t->exit_code, "%s\n", diagnostic);
+ ksft_test_result_code(t->exit_code, test_name,
+ "%s\n", diagnostic);
else
ksft_test_result(__test_passed(t), "%s\n", test_name);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 6/7] selftests: kselftest_harness: let ksft_test_result_code() handle line termination
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
` (4 preceding siblings ...)
2024-02-16 0:41 ` [RFC 5/7] selftests: kselftest_harness: print test name for SKIP and XFAIL Jakub Kicinski
@ 2024-02-16 0:41 ` Jakub Kicinski
2024-02-16 0:41 ` [RFC 7/7] selftests: kselftest_harness: let PASS / FAIL provide diagnostic Jakub Kicinski
2024-02-16 21:32 ` [RFC 0/7] selftests: kselftest_harness: use common result printing helper Kees Cook
7 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
According to the spec we should always print a # if we add
a diagnostic message. Having the caller pass in the new line
as part of diagnostic message makes handling this a bit
counter-intuitive.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/kselftest.h | 5 +++++
tools/testing/selftests/kselftest_harness.h | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index 25e29626566e..541bf192e30e 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -287,10 +287,15 @@ void ksft_test_result_code(int exit_code, const char *test_name,
break;
}
+ /* Docs seem to call for double space if directive is absent */
+ if (!directive[0] && msg[0])
+ directive = " # ";
+
va_start(args, msg);
printf("%s %u %s%s", tap_code, ksft_test_num(), test_name, directive);
errno = saved_errno;
vprintf(msg, args);
+ printf("\n");
va_end(args);
}
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index beae50fd5ac3..5fca75e180b8 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -1179,7 +1179,7 @@ void __run_test(struct __fixture_metadata *f,
if (t->exit_code == KSFT_SKIP || t->exit_code == KSFT_XFAIL)
ksft_test_result_code(t->exit_code, test_name,
- "%s\n", diagnostic);
+ "%s", diagnostic);
else
ksft_test_result(__test_passed(t), "%s\n", test_name);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 7/7] selftests: kselftest_harness: let PASS / FAIL provide diagnostic
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
` (5 preceding siblings ...)
2024-02-16 0:41 ` [RFC 6/7] selftests: kselftest_harness: let ksft_test_result_code() handle line termination Jakub Kicinski
@ 2024-02-16 0:41 ` Jakub Kicinski
2024-02-16 21:32 ` [RFC 0/7] selftests: kselftest_harness: use common result printing helper Kees Cook
7 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-16 0:41 UTC (permalink / raw)
To: jakub; +Cc: shuah, keescook, linux-kselftest, netdev, Jakub Kicinski
Switch to printing KTAP line for PASS / FAIL with ksft_test_result_code(),
this gives us the ability to report diagnostic messages for free.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/kselftest_harness.h | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 5fca75e180b8..202f599c1462 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -1174,14 +1174,12 @@ void __run_test(struct __fixture_metadata *f,
if (t->results->reason[0])
diagnostic = t->results->reason;
+ else if (t->exit_code == KSFT_PASS || t->exit_code == KSFT_FAIL)
+ diagnostic = "";
else
diagnostic = "unknown";
- if (t->exit_code == KSFT_SKIP || t->exit_code == KSFT_XFAIL)
- ksft_test_result_code(t->exit_code, test_name,
- "%s", diagnostic);
- else
- ksft_test_result(__test_passed(t), "%s\n", test_name);
+ ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic);
}
static int test_harness_run(int argc, char **argv)
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC 1/7] selftests: kselftest_harness: generate test name once
2024-02-16 0:41 ` [RFC 1/7] selftests: kselftest_harness: generate test name once Jakub Kicinski
@ 2024-02-16 21:24 ` Kees Cook
0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2024-02-16 21:24 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: jakub, shuah, linux-kselftest, netdev
On Thu, Feb 15, 2024 at 04:41:16PM -0800, Jakub Kicinski wrote:
> Since we added variant support generating full test case
> name takes 4 string arguments. We're about to need it
> in another two places. Stop the duplication and print
> once into a temporary buffer.
>
> Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Clean refactoring -- makes this much more readable.
Acked-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/7] selftests: kselftest_harness: use common result printing helper
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
` (6 preceding siblings ...)
2024-02-16 0:41 ` [RFC 7/7] selftests: kselftest_harness: let PASS / FAIL provide diagnostic Jakub Kicinski
@ 2024-02-16 21:32 ` Kees Cook
2024-02-17 0:31 ` Jakub Kicinski
7 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2024-02-16 21:32 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: jakub, shuah, linux-kselftest, netdev
On Thu, Feb 15, 2024 at 04:41:15PM -0800, Jakub Kicinski wrote:
> First 3 patches rearrange kselftest_harness to use exit code
> as an enum rather than separate passed/skip/xfail members.
One thought I was having here while porting other stuff to use XFAIL was
that in the strictest sense, XFAIL isn't like SKIP, which can be used to
avoid running a test entirely. XFAIL is about the expected outcome,
which means that if we're going to support XFAIL correctly, we need to
distinguish when a test was marked XFAIL but it _didn't_ fail.
The implicit expectation is that a test outcome should be "pass". If
something is marked "xfail", we're saying a successful test is that it
fails. If it _passes_ instead of failing, this is unexpected and should
be reported as well. (i.e. an XPASS -- unexpected pass)
I think if we mix intent with result code, we're going to lose the
ability to make this distinction in the future. (Right now the harness
doesn't do it either -- it treats XFAIL as a special SKIP.)
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/7] selftests: kselftest_harness: use common result printing helper
2024-02-16 21:32 ` [RFC 0/7] selftests: kselftest_harness: use common result printing helper Kees Cook
@ 2024-02-17 0:31 ` Jakub Kicinski
2024-02-17 0:33 ` Jakub Kicinski
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-17 0:31 UTC (permalink / raw)
To: Kees Cook; +Cc: jakub, shuah, linux-kselftest, netdev
On Fri, 16 Feb 2024 13:32:12 -0800 Kees Cook wrote:
> On Thu, Feb 15, 2024 at 04:41:15PM -0800, Jakub Kicinski wrote:
> > First 3 patches rearrange kselftest_harness to use exit code
> > as an enum rather than separate passed/skip/xfail members.
>
> One thought I was having here while porting other stuff to use XFAIL was
> that in the strictest sense, XFAIL isn't like SKIP, which can be used to
> avoid running a test entirely. XFAIL is about the expected outcome,
> which means that if we're going to support XFAIL correctly, we need to
> distinguish when a test was marked XFAIL but it _didn't_ fail.
>
> The implicit expectation is that a test outcome should be "pass". If
> something is marked "xfail", we're saying a successful test is that it
> fails. If it _passes_ instead of failing, this is unexpected and should
> be reported as well. (i.e. an XPASS -- unexpected pass)
>
> I think if we mix intent with result code, we're going to lose the
> ability to make this distinction in the future. (Right now the harness
> doesn't do it either -- it treats XFAIL as a special SKIP.)
Hm.
Let's call "case" the combination of fixture + variant + test.
Currently nothing identifies a single "case" in the harness.
We just recursively walk dimensions.
We can add a new registration list and let user register expected
failures. It should work nicely as long as the exceptions are very
rare. Which is hopefully the case.
Let's see if I can code this up in 30 min. While I do that can you
ELI5 what XPASS is for?! We'll never going to use it, right?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/7] selftests: kselftest_harness: use common result printing helper
2024-02-17 0:31 ` Jakub Kicinski
@ 2024-02-17 0:33 ` Jakub Kicinski
2024-02-17 1:26 ` Jakub Kicinski
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-17 0:33 UTC (permalink / raw)
To: Kees Cook; +Cc: jakub, shuah, linux-kselftest, netdev
On Fri, 16 Feb 2024 16:31:19 -0800 Jakub Kicinski wrote:
> Let's see if I can code this up in 30 min. While I do that can you
> ELI5 what XPASS is for?! We'll never going to use it, right?
Oh, it's UNexpected pass. Okay. So if we have a case on a list of
expected failures and it passes we should throw xpass.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/7] selftests: kselftest_harness: use common result printing helper
2024-02-17 0:33 ` Jakub Kicinski
@ 2024-02-17 1:26 ` Jakub Kicinski
2024-02-17 1:48 ` Kees Cook
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2024-02-17 1:26 UTC (permalink / raw)
To: Kees Cook; +Cc: jakub, shuah, linux-kselftest, netdev
On Fri, 16 Feb 2024 16:33:04 -0800 Jakub Kicinski wrote:
> On Fri, 16 Feb 2024 16:31:19 -0800 Jakub Kicinski wrote:
> > Let's see if I can code this up in 30 min. While I do that can you
> > ELI5 what XPASS is for?! We'll never going to use it, right?
>
> Oh, it's UNexpected pass. Okay. So if we have a case on a list of
> expected failures and it passes we should throw xpass.
I got distracted from this distraction :S
Is this along the lines of what you had in mind?
Both my series need to be rejigged to change the paradigm
but as a PoC on top of them:
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 202f599c1462..399a200a1160 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -826,6 +826,27 @@ struct __fixture_metadata {
.prev = &_fixture_global,
};
+struct __test_xfail {
+ struct __fixture_metadata *fixture;
+ struct __fixture_variant_metadata *variant;
+ struct __test_metadata *test;
+ struct __test_xfail *prev, *next;
+};
+
+#define XFAIL_ADD(fixture_name, variant_name, test_name) \
+ \
+ static struct __test_xfail \
+ _##fixture_name##_##variant_name##_##test_name##_xfail = \
+ { .fixture = &_##fixture_name##_fixture_object, \
+ .variant = &_##fixture_name##_##variant_name##_object, \
+ .test = &_##fixture_name##_##test_name##_object, \
+ };\
+ static void __attribute__((constructor)) \
+ _register_##fixture_name##_##variant_name##_##test_name##_xfail(void) \
+ { \
+ __register_xfail(&_##fixture_name##_##variant_name##_##test_name##_xfail); \
+ }
+
static struct __fixture_metadata *__fixture_list = &_fixture_global;
static int __constructor_order;
@@ -840,6 +861,7 @@ static inline void __register_fixture(struct __fixture_metadata *f)
struct __fixture_variant_metadata {
const char *name;
const void *data;
+ struct __test_xfail *xfails;
struct __fixture_variant_metadata *prev, *next;
};
@@ -890,6 +912,11 @@ static inline void __register_test(struct __test_metadata *t)
__LIST_APPEND(t->fixture->tests, t);
}
+static inline void __register_xfail(struct __test_xfail *xf)
+{
+ __LIST_APPEND(xf->variant->xfails, xf);
+}
+
static inline int __bail(int for_realz, struct __test_metadata *t)
{
/* if this is ASSERT, return immediately. */
@@ -1139,6 +1166,7 @@ void __run_test(struct __fixture_metadata *f,
struct __fixture_variant_metadata *variant,
struct __test_metadata *t)
{
+ struct __test_xfail *xfail;
char test_name[LINE_MAX];
const char *diagnostic;
@@ -1172,6 +1200,14 @@ void __run_test(struct __fixture_metadata *f,
ksft_print_msg(" %4s %s\n",
__test_passed(t) ? "OK" : "FAIL", test_name);
+ /* Check if we're expecting this test to fail */
+ for (xfail = variant->xfails; xfail; xfail = xfail->next)
+ if (xfail->test == t)
+ break;
+ if (xfail)
+ t->exit_code = __test_passed(t) ? KSFT_XPASS : KSFT_XFAIL;
+
+
if (t->results->reason[0])
diagnostic = t->results->reason;
else if (t->exit_code == KSFT_PASS || t->exit_code == KSFT_FAIL)
diff --git a/tools/testing/selftests/net/ip_local_port_range.c b/tools/testing/selftests/net/ip_local_port_range.c
index d4f789f524e5..242ff7de1b12 100644
--- a/tools/testing/selftests/net/ip_local_port_range.c
+++ b/tools/testing/selftests/net/ip_local_port_range.c
@@ -414,6 +414,9 @@ TEST_F(ip_local_port_range, late_bind)
ASSERT_TRUE(!err) TH_LOG("close failed");
}
+XFAIL_ADD(ip_local_port_range, ip4_stcp, late_bind);
+XFAIL_ADD(ip_local_port_range, ip6_stcp, late_bind);
+
TEST_F(ip_local_port_range, get_port_range)
{
__u16 lo, hi;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC 0/7] selftests: kselftest_harness: use common result printing helper
2024-02-17 1:26 ` Jakub Kicinski
@ 2024-02-17 1:48 ` Kees Cook
0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2024-02-17 1:48 UTC (permalink / raw)
To: Jakub Kicinski, Kees Cook; +Cc: jakub, shuah, linux-kselftest, netdev
On February 16, 2024 5:26:21 PM PST, Jakub Kicinski <kuba@kernel.org> wrote:
>On Fri, 16 Feb 2024 16:33:04 -0800 Jakub Kicinski wrote:
>> On Fri, 16 Feb 2024 16:31:19 -0800 Jakub Kicinski wrote:
>> > Let's see if I can code this up in 30 min. While I do that can you
>> > ELI5 what XPASS is for?! We'll never going to use it, right?
>>
>> Oh, it's UNexpected pass. Okay. So if we have a case on a list of
>> expected failures and it passes we should throw xpass.
Right: it's still "ok" but it identifies something worth looking at ("why did this start passing?")
>
>I got distracted from this distraction :S
>Is this along the lines of what you had in mind?
>Both my series need to be rejigged to change the paradigm
>but as a PoC on top of them:
Oh yeah! This looks good. I will give it a spin tomorrow.
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-02-17 1:48 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-16 0:41 [RFC 0/7] selftests: kselftest_harness: use common result printing helper Jakub Kicinski
2024-02-16 0:41 ` [RFC 1/7] selftests: kselftest_harness: generate test name once Jakub Kicinski
2024-02-16 21:24 ` Kees Cook
2024-02-16 0:41 ` [RFC 2/7] selftests: kselftest_harness: save full exit code in metadata Jakub Kicinski
2024-02-16 0:41 ` [RFC 3/7] selftests: kselftest_harness: use exit code to store skip and xfail Jakub Kicinski
2024-02-16 0:41 ` [RFC 4/7] selftests: kselftest: add ksft_test_result_code(), handling all exit codes Jakub Kicinski
2024-02-16 0:41 ` [RFC 5/7] selftests: kselftest_harness: print test name for SKIP and XFAIL Jakub Kicinski
2024-02-16 0:41 ` [RFC 6/7] selftests: kselftest_harness: let ksft_test_result_code() handle line termination Jakub Kicinski
2024-02-16 0:41 ` [RFC 7/7] selftests: kselftest_harness: let PASS / FAIL provide diagnostic Jakub Kicinski
2024-02-16 21:32 ` [RFC 0/7] selftests: kselftest_harness: use common result printing helper Kees Cook
2024-02-17 0:31 ` Jakub Kicinski
2024-02-17 0:33 ` Jakub Kicinski
2024-02-17 1:26 ` Jakub Kicinski
2024-02-17 1:48 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.