2022-05-09 11:16:56

by Gautam Menghani

[permalink] [raw]
Subject: [PATCH] Improve the readability of test results messages in cgroup

Improve the readability of status messages that indicate whether the
kselftest passed or failed.

Signed-off-by: Gautam Menghani <[email protected]>
---
tools/testing/selftests/kselftest.h | 12 ++++++------
tools/testing/selftests/kselftest/runner.sh | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index b8f248018174..a38f20f376b3 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -137,7 +137,7 @@ static inline void ksft_test_result_pass(const char *msg, ...)
ksft_cnt.ksft_pass++;

va_start(args, msg);
- printf("ok %d ", ksft_test_num());
+ printf("Test %d ok - ", ksft_test_num());
errno = saved_errno;
vprintf(msg, args);
va_end(args);
@@ -151,7 +151,7 @@ static inline void ksft_test_result_fail(const char *msg, ...)
ksft_cnt.ksft_fail++;

va_start(args, msg);
- printf("not ok %d ", ksft_test_num());
+ printf("Test %d not ok - ", ksft_test_num());
errno = saved_errno;
vprintf(msg, args);
va_end(args);
@@ -177,7 +177,7 @@ static inline void ksft_test_result_xfail(const char *msg, ...)
ksft_cnt.ksft_xfail++;

va_start(args, msg);
- printf("ok %d # XFAIL ", ksft_test_num());
+ printf("Test %d ok # XFAIL ", ksft_test_num());
errno = saved_errno;
vprintf(msg, args);
va_end(args);
@@ -191,7 +191,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("Test %d ok # SKIP ", ksft_test_num());
errno = saved_errno;
vprintf(msg, args);
va_end(args);
@@ -206,7 +206,7 @@ static inline void ksft_test_result_error(const char *msg, ...)
ksft_cnt.ksft_error++;

va_start(args, msg);
- printf("not ok %d # error ", ksft_test_num());
+ printf("Test %d not ok # error ", ksft_test_num());
errno = saved_errno;
vprintf(msg, args);
va_end(args);
@@ -287,7 +287,7 @@ static inline int ksft_exit_skip(const char *msg, ...)
*/
if (ksft_plan || ksft_test_num()) {
ksft_cnt.ksft_xskip++;
- printf("ok %d # SKIP ", 1 + ksft_test_num());
+ printf("Test %d ok # SKIP ", 1 + ksft_test_num());
} else {
printf("1..0 # SKIP ");
}
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 294619ade49f..6d95a1e96ea8 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -94,7 +94,7 @@ run_one()
echo "# $TEST_HDR_MSG"
if [ ! -e "$TEST" ]; then
echo "# Warning: file $TEST is missing!"
- echo "not ok $test_num $TEST_HDR_MSG"
+ echo "Test $test_num not ok - $TEST_HDR_MSG"
else
eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
cmd="./$BASENAME_TEST $kselftest_cmd_args"
@@ -106,7 +106,7 @@ run_one()
interpreter=$(head -n 1 "$TEST" | cut -c 3-)
cmd="$interpreter ./$BASENAME_TEST"
else
- echo "not ok $test_num $TEST_HDR_MSG"
+ echo "Test $test_num not ok - $TEST_HDR_MSG"
return
fi
fi
@@ -114,15 +114,15 @@ run_one()
((((( tap_timeout "$cmd" 2>&1; echo $? >&3) |
tap_prefix >&4) 3>&1) |
(read xs; exit $xs)) 4>>"$logfile" &&
- echo "ok $test_num $TEST_HDR_MSG") ||
+ echo "Test $test_num ok - $TEST_HDR_MSG") ||
(rc=$?; \
if [ $rc -eq $skip_rc ]; then \
- echo "ok $test_num $TEST_HDR_MSG # SKIP"
+ echo "Test $test_num ok - $TEST_HDR_MSG # SKIP"
elif [ $rc -eq $timeout_rc ]; then \
echo "#"
- echo "not ok $test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
+ echo "Test $test_num not ok - $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
else
- echo "not ok $test_num $TEST_HDR_MSG # exit=$rc"
+ echo "Test $test_num not ok - $TEST_HDR_MSG # exit=$rc"
fi)
cd - >/dev/null
fi
--
2.25.1



2022-05-09 18:09:27

by Bird, Tim

[permalink] [raw]
Subject: RE: [PATCH] Improve the readability of test results messages in cgroup

> -----Original Message-----
> From: Gautam Menghani <[email protected]>
>
> Improve the readability of status messages that indicate whether the
> kselftest passed or failed.
>
> Signed-off-by: Gautam Menghani <[email protected]>
> ---
> tools/testing/selftests/kselftest.h | 12 ++++++------
> tools/testing/selftests/kselftest/runner.sh | 12 ++++++------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index b8f248018174..a38f20f376b3 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -137,7 +137,7 @@ static inline void ksft_test_result_pass(const char *msg, ...)
> ksft_cnt.ksft_pass++;
>
> va_start(args, msg);
> - printf("ok %d ", ksft_test_num());
> + printf("Test %d ok - ", ksft_test_num());

The output for each test case is formatted according
to the KTAP protocol, which is documented here:
https://github.com/torvalds/linux/blob/master/Documentation/dev-tools/ktap.rst

The testcase result lines must start with either "ok" or "not ok", and have
specific syntax to be compatible with automated testing tools (both internal
and external to the kernel source).

This change breaks the output format.

Sorry, but NAK from me.
-- Tim

> errno = saved_errno;
> vprintf(msg, args);
> va_end(args);
> @@ -151,7 +151,7 @@ static inline void ksft_test_result_fail(const char *msg, ...)
> ksft_cnt.ksft_fail++;
>
> va_start(args, msg);
> - printf("not ok %d ", ksft_test_num());
> + printf("Test %d not ok - ", ksft_test_num());
> errno = saved_errno;
> vprintf(msg, args);
> va_end(args);
> @@ -177,7 +177,7 @@ static inline void ksft_test_result_xfail(const char *msg, ...)
> ksft_cnt.ksft_xfail++;
>
> va_start(args, msg);
> - printf("ok %d # XFAIL ", ksft_test_num());
> + printf("Test %d ok # XFAIL ", ksft_test_num());
> errno = saved_errno;
> vprintf(msg, args);
> va_end(args);
> @@ -191,7 +191,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("Test %d ok # SKIP ", ksft_test_num());
> errno = saved_errno;
> vprintf(msg, args);
> va_end(args);
> @@ -206,7 +206,7 @@ static inline void ksft_test_result_error(const char *msg, ...)
> ksft_cnt.ksft_error++;
>
> va_start(args, msg);
> - printf("not ok %d # error ", ksft_test_num());
> + printf("Test %d not ok # error ", ksft_test_num());
> errno = saved_errno;
> vprintf(msg, args);
> va_end(args);
> @@ -287,7 +287,7 @@ static inline int ksft_exit_skip(const char *msg, ...)
> */
> if (ksft_plan || ksft_test_num()) {
> ksft_cnt.ksft_xskip++;
> - printf("ok %d # SKIP ", 1 + ksft_test_num());
> + printf("Test %d ok # SKIP ", 1 + ksft_test_num());
> } else {
> printf("1..0 # SKIP ");
> }
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index 294619ade49f..6d95a1e96ea8 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -94,7 +94,7 @@ run_one()
> echo "# $TEST_HDR_MSG"
> if [ ! -e "$TEST" ]; then
> echo "# Warning: file $TEST is missing!"
> - echo "not ok $test_num $TEST_HDR_MSG"
> + echo "Test $test_num not ok - $TEST_HDR_MSG"
> else
> eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
> cmd="./$BASENAME_TEST $kselftest_cmd_args"
> @@ -106,7 +106,7 @@ run_one()
> interpreter=$(head -n 1 "$TEST" | cut -c 3-)
> cmd="$interpreter ./$BASENAME_TEST"
> else
> - echo "not ok $test_num $TEST_HDR_MSG"
> + echo "Test $test_num not ok - $TEST_HDR_MSG"
> return
> fi
> fi
> @@ -114,15 +114,15 @@ run_one()
> ((((( tap_timeout "$cmd" 2>&1; echo $? >&3) |
> tap_prefix >&4) 3>&1) |
> (read xs; exit $xs)) 4>>"$logfile" &&
> - echo "ok $test_num $TEST_HDR_MSG") ||
> + echo "Test $test_num ok - $TEST_HDR_MSG") ||
> (rc=$?; \
> if [ $rc -eq $skip_rc ]; then \
> - echo "ok $test_num $TEST_HDR_MSG # SKIP"
> + echo "Test $test_num ok - $TEST_HDR_MSG # SKIP"
> elif [ $rc -eq $timeout_rc ]; then \
> echo "#"
> - echo "not ok $test_num $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
> + echo "Test $test_num not ok - $TEST_HDR_MSG # TIMEOUT $kselftest_timeout seconds"
> else
> - echo "not ok $test_num $TEST_HDR_MSG # exit=$rc"
> + echo "Test $test_num not ok - $TEST_HDR_MSG # exit=$rc"
> fi)
> cd - >/dev/null
> fi
> --
> 2.25.1