Conform individual tests to TAP output. One patch conform one test. With this series, all vDSO tests become TAP conformant.
First patch conform the test by using kselftest_harness.h. Other patches are conforming using default kselftest.h helpers.
All tests have been tested multiple times before and after these patches. They are working correctly and outputting TAP messaging to find failures quikly when they happen. --- Changes since v1: - Update cover letter - Update commit message of first patch
Muhammad Usama Anjum (4): kselftests: vdso: vdso_test_clock_getres: conform test to TAP output kselftests: vdso: vdso_test_correctness: conform test to TAP output kselftests: vdso: vdso_test_getcpu: conform test to TAP output kselftests: vdso: vdso_test_gettimeofday: conform test to TAP output
.../selftests/vDSO/vdso_test_clock_getres.c | 68 ++++---- .../selftests/vDSO/vdso_test_correctness.c | 146 +++++++++--------- .../testing/selftests/vDSO/vdso_test_getcpu.c | 16 +- .../selftests/vDSO/vdso_test_gettimeofday.c | 23 +-- 4 files changed, 126 insertions(+), 127 deletions(-)
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. Use kselftest_harness.h to conform to TAP as the number of tests depend on the available options at build time. The kselftest_harness makes the test easy to convert and presents better maintainability.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- Changes since v1: - Update commit message to include that kselftest_harness has been used to conform to TAP and why --- .../selftests/vDSO/vdso_test_clock_getres.c | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c index 38d46a8bf7cba..c1ede40521f05 100644 --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -25,7 +25,7 @@ #include <unistd.h> #include <sys/syscall.h>
-#include "../kselftest.h" +#include "../kselftest_harness.h"
static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) { @@ -54,18 +54,8 @@ const char *vdso_clock_name[12] = { /* * This function calls clock_getres in vdso and by system call * with different values for clock_id. - * - * Example of output: - * - * clock_id: CLOCK_REALTIME [PASS] - * clock_id: CLOCK_BOOTTIME [PASS] - * clock_id: CLOCK_TAI [PASS] - * clock_id: CLOCK_REALTIME_COARSE [PASS] - * clock_id: CLOCK_MONOTONIC [PASS] - * clock_id: CLOCK_MONOTONIC_RAW [PASS] - * clock_id: CLOCK_MONOTONIC_COARSE [PASS] */ -static inline int vdso_test_clock(unsigned int clock_id) +static inline void vdso_test_clock(struct __test_metadata *_metadata, unsigned int clock_id) { struct timespec x, y;
@@ -73,52 +63,60 @@ static inline int vdso_test_clock(unsigned int clock_id) clock_getres(clock_id, &x); syscall_clock_getres(clock_id, &y);
- if ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) { - printf(" [FAIL]\n"); - return KSFT_FAIL; - } - - printf(" [PASS]\n"); - return KSFT_PASS; + ASSERT_EQ(0, ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec))); }
-int main(int argc, char **argv) -{ - int ret = 0; - #if _POSIX_TIMERS > 0
#ifdef CLOCK_REALTIME - ret += vdso_test_clock(CLOCK_REALTIME); +TEST(clock_realtime) +{ + vdso_test_clock(_metadata, CLOCK_REALTIME); +} #endif
#ifdef CLOCK_BOOTTIME - ret += vdso_test_clock(CLOCK_BOOTTIME); +TEST(clock_boottime) +{ + vdso_test_clock(_metadata, CLOCK_BOOTTIME); +} #endif
#ifdef CLOCK_TAI - ret += vdso_test_clock(CLOCK_TAI); +TEST(clock_tai) +{ + vdso_test_clock(_metadata, CLOCK_TAI); +} #endif
#ifdef CLOCK_REALTIME_COARSE - ret += vdso_test_clock(CLOCK_REALTIME_COARSE); +TEST(clock_realtime_coarse) +{ + vdso_test_clock(_metadata, CLOCK_REALTIME_COARSE); +} #endif
#ifdef CLOCK_MONOTONIC - ret += vdso_test_clock(CLOCK_MONOTONIC); +TEST(clock_monotonic) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC); +} #endif
#ifdef CLOCK_MONOTONIC_RAW - ret += vdso_test_clock(CLOCK_MONOTONIC_RAW); +TEST(clock_monotonic_raw) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC_RAW); +} #endif
#ifdef CLOCK_MONOTONIC_COARSE - ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE); +TEST(clock_monotonic_coarse) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC_COARSE); +} #endif
-#endif - if (ret > 0) - return KSFT_FAIL; +#endif /* _POSIX_TIMERS > 0 */
- return KSFT_PASS; -} +TEST_HARNESS_MAIN
On 6/9/24 23:41, Muhammad Usama Anjum wrote:
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. Use kselftest_harness.h to conform to TAP as the number of tests depend on the available options at build time. The kselftest_harness makes the
How does converting to kselftest_harness help with available options ay build time? Can you explain?
I am not seeing any value in converting this test to the harness? I want to see a better justification.
test easy to convert and presents better maintainability.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
Changes since v1:
- Update commit message to include that kselftest_harness has been used to conform to TAP and why
.../selftests/vDSO/vdso_test_clock_getres.c | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c index 38d46a8bf7cba..c1ede40521f05 100644 --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -25,7 +25,7 @@ #include <unistd.h> #include <sys/syscall.h> -#include "../kselftest.h" +#include "../kselftest_harness.h" static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) { @@ -54,18 +54,8 @@ const char *vdso_clock_name[12] = { /*
- This function calls clock_getres in vdso and by system call
- with different values for clock_id.
- Example of output:
- clock_id: CLOCK_REALTIME [PASS]
- clock_id: CLOCK_BOOTTIME [PASS]
- clock_id: CLOCK_TAI [PASS]
- clock_id: CLOCK_REALTIME_COARSE [PASS]
- clock_id: CLOCK_MONOTONIC [PASS]
- clock_id: CLOCK_MONOTONIC_RAW [PASS]
*/
- clock_id: CLOCK_MONOTONIC_COARSE [PASS]
-static inline int vdso_test_clock(unsigned int clock_id) +static inline void vdso_test_clock(struct __test_metadata *_metadata, unsigned int clock_id) { struct timespec x, y; @@ -73,52 +63,60 @@ static inline int vdso_test_clock(unsigned int clock_id) clock_getres(clock_id, &x); syscall_clock_getres(clock_id, &y);
- if ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) {
printf(" [FAIL]\n");return KSFT_FAIL;- }
- printf(" [PASS]\n");
- return KSFT_PASS;
- ASSERT_EQ(0, ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec))); }
-int main(int argc, char **argv) -{
- int ret = 0;
- #if _POSIX_TIMERS > 0
#ifdef CLOCK_REALTIME
- ret += vdso_test_clock(CLOCK_REALTIME);
+TEST(clock_realtime) +{
- vdso_test_clock(_metadata, CLOCK_REALTIME);
+} #endif #ifdef CLOCK_BOOTTIME
- ret += vdso_test_clock(CLOCK_BOOTTIME);
+TEST(clock_boottime) +{
- vdso_test_clock(_metadata, CLOCK_BOOTTIME);
+} #endif #ifdef CLOCK_TAI
- ret += vdso_test_clock(CLOCK_TAI);
+TEST(clock_tai) +{
- vdso_test_clock(_metadata, CLOCK_TAI);
+} #endif #ifdef CLOCK_REALTIME_COARSE
- ret += vdso_test_clock(CLOCK_REALTIME_COARSE);
+TEST(clock_realtime_coarse) +{
- vdso_test_clock(_metadata, CLOCK_REALTIME_COARSE);
+} #endif #ifdef CLOCK_MONOTONIC
- ret += vdso_test_clock(CLOCK_MONOTONIC);
+TEST(clock_monotonic) +{
- vdso_test_clock(_metadata, CLOCK_MONOTONIC);
+} #endif #ifdef CLOCK_MONOTONIC_RAW
- ret += vdso_test_clock(CLOCK_MONOTONIC_RAW);
+TEST(clock_monotonic_raw) +{
- vdso_test_clock(_metadata, CLOCK_MONOTONIC_RAW);
+} #endif #ifdef CLOCK_MONOTONIC_COARSE
- ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE);
+TEST(clock_monotonic_coarse) +{
- vdso_test_clock(_metadata, CLOCK_MONOTONIC_COARSE);
+} #endif -#endif
- if (ret > 0)
return KSFT_FAIL;+#endif /* _POSIX_TIMERS > 0 */
- return KSFT_PASS;
-} +TEST_HARNESS_MAIN
thanks, -- Shuah
On 6/12/24 1:32 AM, Shuah Khan wrote:
On 6/9/24 23:41, Muhammad Usama Anjum wrote:
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. Use kselftest_harness.h to conform to TAP as the number of tests depend on the available options at build time. The kselftest_harness makes the
How does converting to kselftest_harness help with available options ay build time? Can you explain?
I am not seeing any value in converting this test to the harness? I want to see a better justification.
Before: ./vdso_test_clock_getres clock_id: CLOCK_REALTIME [PASS] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [PASS] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS]
Here is the output of the test before this patch. The test output test names and if they are passed or failed. It doesn't output information related to error when it occurs. I wanted to convert it to standard format by using kselftest.h where we can get the error related information as well. But as the number of tests depend on how many of CLOCK_BOOTTIME, CLOCK_TAI etc are defined, static counting is difficult. Test harness is best suited for this. Output:
./vdso_test_clock_getres TAP version 13 1..7 # Starting 7 tests from 1 test cases. # RUN global.clock_realtime ... # OK global.clock_realtime ok 1 global.clock_realtime # RUN global.clock_boottime ... # OK global.clock_boottime ok 2 global.clock_boottime # RUN global.clock_tai ... # OK global.clock_tai ok 3 global.clock_tai # RUN global.clock_realtime_coarse ... # OK global.clock_realtime_coarse ok 4 global.clock_realtime_coarse # RUN global.clock_monotonic ... # OK global.clock_monotonic ok 5 global.clock_monotonic # RUN global.clock_monotonic_raw ... # OK global.clock_monotonic_raw ok 6 global.clock_monotonic_raw # RUN global.clock_monotonic_coarse ... # OK global.clock_monotonic_coarse ok 7 global.clock_monotonic_coarse # PASSED: 7 / 7 tests passed. # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
Not only the code is simplified, the descriptive error is printed on console that what went wrong. Example if a test case fails:
# RUN global.clock_realtime ... # vdso_test_clock_getres.c:66:clock_realtime:Expected 1 (1) == ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) (0) # clock_realtime: Test terminated by assertion # FAIL global.clock_realtime not ok 1 global.clock_realtime
test easy to convert and presents better maintainability.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
Changes since v1:
- Update commit message to include that kselftest_harness has been used
to conform to TAP and why
.../selftests/vDSO/vdso_test_clock_getres.c | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c index 38d46a8bf7cba..c1ede40521f05 100644 --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -25,7 +25,7 @@ #include <unistd.h> #include <sys/syscall.h> -#include "../kselftest.h" +#include "../kselftest_harness.h" static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) { @@ -54,18 +54,8 @@ const char *vdso_clock_name[12] = { /* * This function calls clock_getres in vdso and by system call * with different values for clock_id.
- Example of output:
- clock_id: CLOCK_REALTIME [PASS]
- clock_id: CLOCK_BOOTTIME [PASS]
- clock_id: CLOCK_TAI [PASS]
- clock_id: CLOCK_REALTIME_COARSE [PASS]
- clock_id: CLOCK_MONOTONIC [PASS]
- clock_id: CLOCK_MONOTONIC_RAW [PASS]
- clock_id: CLOCK_MONOTONIC_COARSE [PASS]
*/ -static inline int vdso_test_clock(unsigned int clock_id) +static inline void vdso_test_clock(struct __test_metadata *_metadata, unsigned int clock_id) { struct timespec x, y; @@ -73,52 +63,60 @@ static inline int vdso_test_clock(unsigned int clock_id) clock_getres(clock_id, &x); syscall_clock_getres(clock_id, &y); - if ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) { - printf(" [FAIL]\n"); - return KSFT_FAIL; - }
- printf(" [PASS]\n"); - return KSFT_PASS; + ASSERT_EQ(0, ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec))); } -int main(int argc, char **argv) -{ - int ret = 0;
#if _POSIX_TIMERS > 0 #ifdef CLOCK_REALTIME - ret += vdso_test_clock(CLOCK_REALTIME); +TEST(clock_realtime) +{ + vdso_test_clock(_metadata, CLOCK_REALTIME); +} #endif #ifdef CLOCK_BOOTTIME - ret += vdso_test_clock(CLOCK_BOOTTIME); +TEST(clock_boottime) +{ + vdso_test_clock(_metadata, CLOCK_BOOTTIME); +} #endif #ifdef CLOCK_TAI - ret += vdso_test_clock(CLOCK_TAI); +TEST(clock_tai) +{ + vdso_test_clock(_metadata, CLOCK_TAI); +} #endif #ifdef CLOCK_REALTIME_COARSE - ret += vdso_test_clock(CLOCK_REALTIME_COARSE); +TEST(clock_realtime_coarse) +{ + vdso_test_clock(_metadata, CLOCK_REALTIME_COARSE); +} #endif #ifdef CLOCK_MONOTONIC - ret += vdso_test_clock(CLOCK_MONOTONIC); +TEST(clock_monotonic) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC); +} #endif #ifdef CLOCK_MONOTONIC_RAW - ret += vdso_test_clock(CLOCK_MONOTONIC_RAW); +TEST(clock_monotonic_raw) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC_RAW); +} #endif #ifdef CLOCK_MONOTONIC_COARSE - ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE); +TEST(clock_monotonic_coarse) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC_COARSE); +} #endif -#endif - if (ret > 0) - return KSFT_FAIL; +#endif /* _POSIX_TIMERS > 0 */ - return KSFT_PASS; -} +TEST_HARNESS_MAIN
thanks, -- Shuah
Kind reminder to review
On 6/12/24 1:17 PM, Muhammad Usama Anjum wrote:
On 6/12/24 1:32 AM, Shuah Khan wrote:
On 6/9/24 23:41, Muhammad Usama Anjum wrote:
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. Use kselftest_harness.h to conform to TAP as the number of tests depend on the available options at build time. The kselftest_harness makes the
How does converting to kselftest_harness help with available options ay build time? Can you explain?
I am not seeing any value in converting this test to the harness? I want to see a better justification.
Before: ./vdso_test_clock_getres clock_id: CLOCK_REALTIME [PASS] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [PASS] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS]
Here is the output of the test before this patch. The test output test names and if they are passed or failed. It doesn't output information related to error when it occurs. I wanted to convert it to standard format by using kselftest.h where we can get the error related information as well. But as the number of tests depend on how many of CLOCK_BOOTTIME, CLOCK_TAI etc are defined, static counting is difficult. Test harness is best suited for this. Output:
./vdso_test_clock_getres TAP version 13 1..7 # Starting 7 tests from 1 test cases. # RUN global.clock_realtime ... # OK global.clock_realtime ok 1 global.clock_realtime # RUN global.clock_boottime ... # OK global.clock_boottime ok 2 global.clock_boottime # RUN global.clock_tai ... # OK global.clock_tai ok 3 global.clock_tai # RUN global.clock_realtime_coarse ... # OK global.clock_realtime_coarse ok 4 global.clock_realtime_coarse # RUN global.clock_monotonic ... # OK global.clock_monotonic ok 5 global.clock_monotonic # RUN global.clock_monotonic_raw ... # OK global.clock_monotonic_raw ok 6 global.clock_monotonic_raw # RUN global.clock_monotonic_coarse ... # OK global.clock_monotonic_coarse ok 7 global.clock_monotonic_coarse # PASSED: 7 / 7 tests passed. # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
Not only the code is simplified, the descriptive error is printed on console that what went wrong. Example if a test case fails:
# RUN global.clock_realtime ... # vdso_test_clock_getres.c:66:clock_realtime:Expected 1 (1) == ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) (0) # clock_realtime: Test terminated by assertion # FAIL global.clock_realtime not ok 1 global.clock_realtime
test easy to convert and presents better maintainability.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
Changes since v1:
- Update commit message to include that kselftest_harness has been used
to conform to TAP and why
.../selftests/vDSO/vdso_test_clock_getres.c | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c index 38d46a8bf7cba..c1ede40521f05 100644 --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -25,7 +25,7 @@ #include <unistd.h> #include <sys/syscall.h> -#include "../kselftest.h" +#include "../kselftest_harness.h" static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) { @@ -54,18 +54,8 @@ const char *vdso_clock_name[12] = { /* * This function calls clock_getres in vdso and by system call * with different values for clock_id.
- Example of output:
- clock_id: CLOCK_REALTIME [PASS]
- clock_id: CLOCK_BOOTTIME [PASS]
- clock_id: CLOCK_TAI [PASS]
- clock_id: CLOCK_REALTIME_COARSE [PASS]
- clock_id: CLOCK_MONOTONIC [PASS]
- clock_id: CLOCK_MONOTONIC_RAW [PASS]
- clock_id: CLOCK_MONOTONIC_COARSE [PASS]
*/ -static inline int vdso_test_clock(unsigned int clock_id) +static inline void vdso_test_clock(struct __test_metadata *_metadata, unsigned int clock_id) { struct timespec x, y; @@ -73,52 +63,60 @@ static inline int vdso_test_clock(unsigned int clock_id) clock_getres(clock_id, &x); syscall_clock_getres(clock_id, &y); - if ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) { - printf(" [FAIL]\n"); - return KSFT_FAIL; - }
- printf(" [PASS]\n"); - return KSFT_PASS; + ASSERT_EQ(0, ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec))); } -int main(int argc, char **argv) -{ - int ret = 0;
#if _POSIX_TIMERS > 0 #ifdef CLOCK_REALTIME - ret += vdso_test_clock(CLOCK_REALTIME); +TEST(clock_realtime) +{ + vdso_test_clock(_metadata, CLOCK_REALTIME); +} #endif #ifdef CLOCK_BOOTTIME - ret += vdso_test_clock(CLOCK_BOOTTIME); +TEST(clock_boottime) +{ + vdso_test_clock(_metadata, CLOCK_BOOTTIME); +} #endif #ifdef CLOCK_TAI - ret += vdso_test_clock(CLOCK_TAI); +TEST(clock_tai) +{ + vdso_test_clock(_metadata, CLOCK_TAI); +} #endif #ifdef CLOCK_REALTIME_COARSE - ret += vdso_test_clock(CLOCK_REALTIME_COARSE); +TEST(clock_realtime_coarse) +{ + vdso_test_clock(_metadata, CLOCK_REALTIME_COARSE); +} #endif #ifdef CLOCK_MONOTONIC - ret += vdso_test_clock(CLOCK_MONOTONIC); +TEST(clock_monotonic) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC); +} #endif #ifdef CLOCK_MONOTONIC_RAW - ret += vdso_test_clock(CLOCK_MONOTONIC_RAW); +TEST(clock_monotonic_raw) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC_RAW); +} #endif #ifdef CLOCK_MONOTONIC_COARSE - ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE); +TEST(clock_monotonic_coarse) +{ + vdso_test_clock(_metadata, CLOCK_MONOTONIC_COARSE); +} #endif -#endif - if (ret > 0) - return KSFT_FAIL; +#endif /* _POSIX_TIMERS > 0 */ - return KSFT_PASS; -} +TEST_HARNESS_MAIN
thanks, -- Shuah
On 6/12/24 02:17, Muhammad Usama Anjum wrote:
On 6/12/24 1:32 AM, Shuah Khan wrote:
On 6/9/24 23:41, Muhammad Usama Anjum wrote:
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. Use kselftest_harness.h to conform to TAP as the number of tests depend on the available options at build time. The kselftest_harness makes the
How does converting to kselftest_harness help with available options ay build time? Can you explain?
I am not seeing any value in converting this test to the harness? I want to see a better justification.
Before: ./vdso_test_clock_getres clock_id: CLOCK_REALTIME [PASS] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [PASS] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS]
Here is the output of the test before this patch. The test output test names and if they are passed or failed. It doesn't output information related to error when it occurs. I wanted to convert it to standard format by using kselftest.h where we can get the error related information as well. But as the number of tests depend on how many of CLOCK_BOOTTIME, CLOCK_TAI etc are defined, static counting is difficult. Test harness is best suited for this. Output:
./vdso_test_clock_getres
The reason I don't want to take this patch is if you run the test using the recommended method:
make -C tools/testing/selftests/vDSO/ run_tests you will get the TAP output because lib.mk runtests framework takes care of this.
or
make kselftest TARGETS=vDSO will do the same.
Please don't send TAP conversions for individual runs. You will start seeing duplicate TAP output which will make it unreadable.
Run the test using make -C or make kselftest TARGETS before investing time to concert to TAP. I am not going to take TAP conversions patches if make -C or make kselftest TARGETS shows TAP.
thanks, -- Shuah
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- .../selftests/vDSO/vdso_test_correctness.c | 146 +++++++++--------- 1 file changed, 74 insertions(+), 72 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_correctness.c b/tools/testing/selftests/vDSO/vdso_test_correctness.c index e691a3cf14911..688f83abb28eb 100644 --- a/tools/testing/selftests/vDSO/vdso_test_correctness.c +++ b/tools/testing/selftests/vDSO/vdso_test_correctness.c @@ -46,8 +46,6 @@ struct __kernel_timespec { /* max length of lines in /proc/self/maps - anything longer is skipped here */ #define MAPS_LINE_LEN 128
-int nerrs = 0; - typedef int (*vgettime_t)(clockid_t, struct timespec *);
vgettime_t vdso_clock_gettime; @@ -97,7 +95,7 @@ static void *vsyscall_getcpu(void) fclose(maps);
if (!found) { - printf("Warning: failed to find vsyscall getcpu\n"); + ksft_print_msg("Warning: failed to find vsyscall getcpu\n"); return NULL; } return (void *) (0xffffffffff600800); @@ -115,30 +113,29 @@ static void fill_function_pointers() vdso = dlopen("linux-gate.so.1", RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); if (!vdso) { - printf("[WARN]\tfailed to find vDSO\n"); - return; + ksft_print_msg("failed to find vDSO\n"); + ksft_finished(); }
vdso_getcpu = (getcpu_t)dlsym(vdso, name[4]); if (!vdso_getcpu) - printf("Warning: failed to find getcpu in vDSO\n"); + ksft_print_msg("Warning: failed to find getcpu in vDSO\n");
vgetcpu = (getcpu_t) vsyscall_getcpu();
vdso_clock_gettime = (vgettime_t)dlsym(vdso, name[1]); if (!vdso_clock_gettime) - printf("Warning: failed to find clock_gettime in vDSO\n"); + ksft_print_msg("Warning: failed to find clock_gettime in vDSO\n");
#if defined(VDSO_32BIT) vdso_clock_gettime64 = (vgettime64_t)dlsym(vdso, name[5]); if (!vdso_clock_gettime64) - printf("Warning: failed to find clock_gettime64 in vDSO\n"); + ksft_print_msg("Warning: failed to find clock_gettime64 in vDSO\n"); #endif
vdso_gettimeofday = (vgtod_t)dlsym(vdso, name[0]); if (!vdso_gettimeofday) - printf("Warning: failed to find gettimeofday in vDSO\n"); - + ksft_print_msg("Warning: failed to find gettimeofday in vDSO\n"); }
static long sys_getcpu(unsigned * cpu, unsigned * node, @@ -164,7 +161,7 @@ static inline int sys_gettimeofday(struct timeval *tv, struct timezone *tz)
static void test_getcpu(void) { - printf("[RUN]\tTesting getcpu...\n"); + ksft_print_msg("Testing getcpu...\n");
for (int cpu = 0; ; cpu++) { cpu_set_t cpuset; @@ -199,18 +196,16 @@ static void test_getcpu(void) if (!ret_vsys && (cpu_vsys != cpu || node_vsys != node)) ok = false;
- printf("[%s]\tCPU %u:", ok ? "OK" : "FAIL", cpu); + ksft_print_msg("CPU %u:", ok ? "OK" : "FAIL", cpu); if (!ret_sys) - printf(" syscall: cpu %u, node %u", cpu_sys, node_sys); + ksft_print_msg(" syscall: cpu %u, node %u", cpu_sys, node_sys); if (!ret_vdso) - printf(" vdso: cpu %u, node %u", cpu_vdso, node_vdso); + ksft_print_msg(" vdso: cpu %u, node %u", cpu_vdso, node_vdso); if (!ret_vsys) - printf(" vsyscall: cpu %u, node %u", cpu_vsys, - node_vsys); - printf("\n"); + ksft_print_msg(" vsyscall: cpu %u, node %u", cpu_vsys, node_vsys); + ksft_print_msg("\n");
- if (!ok) - nerrs++; + ksft_test_result(ok, "Succeeded\n"); } }
@@ -259,19 +254,20 @@ static void test_one_clock_gettime(int clock, const char *name) struct timespec start, vdso, end; int vdso_ret, end_ret;
- printf("[RUN]\tTesting clock_gettime for clock %s (%d)...\n", name, clock); + ksft_print_msg("Testing clock_gettime for clock %s (%d)...\n", name, clock);
if (sys_clock_gettime(clock, &start) < 0) { if (errno == EINVAL) { vdso_ret = vdso_clock_gettime(clock, &vdso); if (vdso_ret == -EINVAL) { - printf("[OK]\tNo such clock.\n"); + ksft_test_result_skip("No such clock.\n"); } else { - printf("[FAIL]\tNo such clock, but __vdso_clock_gettime returned %d\n", vdso_ret); - nerrs++; + ksft_test_result_fail("No such clock, but __vdso_clock_gettime returned %d\n", + vdso_ret); } } else { - printf("[WARN]\t clock_gettime(%d) syscall returned error %d\n", clock, errno); + ksft_test_result_skip("clock_gettime(%d) syscall returned error %d\n", + clock, errno); } return; } @@ -280,30 +276,32 @@ static void test_one_clock_gettime(int clock, const char *name) end_ret = sys_clock_gettime(clock, &end);
if (vdso_ret != 0 || end_ret != 0) { - printf("[FAIL]\tvDSO returned %d, syscall errno=%d\n", - vdso_ret, errno); - nerrs++; + ksft_test_result_fail("vDSO returned %d, syscall errno=%d\n", vdso_ret, errno); return; }
- printf("\t%llu.%09ld %llu.%09ld %llu.%09ld\n", - (unsigned long long)start.tv_sec, start.tv_nsec, - (unsigned long long)vdso.tv_sec, vdso.tv_nsec, - (unsigned long long)end.tv_sec, end.tv_nsec); + ksft_print_msg("\t%llu.%09ld %llu.%09ld %llu.%09ld\n", + (unsigned long long)start.tv_sec, start.tv_nsec, + (unsigned long long)vdso.tv_sec, vdso.tv_nsec, + (unsigned long long)end.tv_sec, end.tv_nsec);
if (!ts_leq(&start, &vdso) || !ts_leq(&vdso, &end)) { - printf("[FAIL]\tTimes are out of sequence\n"); - nerrs++; + ksft_test_result_fail("Times are out of sequence\n"); return; }
- printf("[OK]\tTest Passed.\n"); + ksft_test_result_pass("Test Passed.\n"); }
static void test_clock_gettime(void) { if (!vdso_clock_gettime) { - printf("[SKIP]\tNo vDSO, so skipping clock_gettime() tests\n"); + for (int clock = 0; clock < ARRAY_SIZE(clocknames); clock++) + ksft_test_result_skip("No vDSO, so skipping %s\n", clocknames[clock]); + + ksft_test_result_skip("No vDSO, so skipping clock_gettime() tests -1\n"); + ksft_test_result_skip("No vDSO, so skipping clock_gettime() tests min\n"); + ksft_test_result_skip("No vDSO, so skipping clock_gettime() tests max\n"); return; }
@@ -321,19 +319,20 @@ static void test_one_clock_gettime64(int clock, const char *name) struct __kernel_timespec start, vdso, end; int vdso_ret, end_ret;
- printf("[RUN]\tTesting clock_gettime64 for clock %s (%d)...\n", name, clock); + ksft_print_msg("Testing clock_gettime64 for clock %s (%d)...\n", name, clock);
if (sys_clock_gettime64(clock, &start) < 0) { if (errno == EINVAL) { vdso_ret = vdso_clock_gettime64(clock, &vdso); if (vdso_ret == -EINVAL) { - printf("[OK]\tNo such clock.\n"); + ksft_test_result_skip("No such clock.\n"); } else { - printf("[FAIL]\tNo such clock, but __vdso_clock_gettime64 returned %d\n", vdso_ret); - nerrs++; + ksft_test_result_fail("No such clock, but __vdso_clock_gettime64 returned %d\n", + vdso_ret); } } else { - printf("[WARN]\t clock_gettime64(%d) syscall returned error %d\n", clock, errno); + ksft_test_result_skip("clock_gettime64(%d) syscall returned error %d\n", + clock, errno); } return; } @@ -342,30 +341,32 @@ static void test_one_clock_gettime64(int clock, const char *name) end_ret = sys_clock_gettime64(clock, &end);
if (vdso_ret != 0 || end_ret != 0) { - printf("[FAIL]\tvDSO returned %d, syscall errno=%d\n", - vdso_ret, errno); - nerrs++; + ksft_test_result_fail("vDSO returned %d, syscall errno=%d\n", vdso_ret, errno); return; }
- printf("\t%llu.%09lld %llu.%09lld %llu.%09lld\n", - (unsigned long long)start.tv_sec, start.tv_nsec, - (unsigned long long)vdso.tv_sec, vdso.tv_nsec, - (unsigned long long)end.tv_sec, end.tv_nsec); + ksft_print_msg("\t%llu.%09lld %llu.%09lld %llu.%09lld\n", + (unsigned long long)start.tv_sec, start.tv_nsec, + (unsigned long long)vdso.tv_sec, vdso.tv_nsec, + (unsigned long long)end.tv_sec, end.tv_nsec);
if (!ts64_leq(&start, &vdso) || !ts64_leq(&vdso, &end)) { - printf("[FAIL]\tTimes are out of sequence\n"); - nerrs++; + ksft_test_result_fail("Times are out of sequence\n"); return; }
- printf("[OK]\tTest Passed.\n"); + ksft_test_result_pass("Test Passed.\n"); }
static void test_clock_gettime64(void) { if (!vdso_clock_gettime64) { - printf("[SKIP]\tNo vDSO, so skipping clock_gettime64() tests\n"); + for (int clock = 0; clock < ARRAY_SIZE(clocknames); clock++) + ksft_test_result_skip("No vDSO, so skipping %s\n", clocknames[clock]); + + ksft_test_result_skip("No vDSO, so skipping clock_gettime() tests -1\n"); + ksft_test_result_skip("No vDSO, so skipping clock_gettime() tests min\n"); + ksft_test_result_skip("No vDSO, so skipping clock_gettime() tests max\n"); return; }
@@ -384,14 +385,18 @@ static void test_gettimeofday(void) struct timezone sys_tz, vdso_tz; int vdso_ret, end_ret;
- if (!vdso_gettimeofday) + if (!vdso_gettimeofday) { + ksft_test_result_skip("No vDSO, so skipping clock_gettime64() tests\n"); return; + }
- printf("[RUN]\tTesting gettimeofday...\n"); + ksft_print_msg("Testing gettimeofday...\n"); + + /* Make sure that passing NULL for tz doesn't crash. */ + vdso_gettimeofday(&vdso, NULL);
if (sys_gettimeofday(&start, &sys_tz) < 0) { - printf("[FAIL]\tsys_gettimeofday failed (%d)\n", errno); - nerrs++; + ksft_test_result_fail("sys_gettimeofday failed (%d)\n", errno); return; }
@@ -399,39 +404,36 @@ static void test_gettimeofday(void) end_ret = sys_gettimeofday(&end, NULL);
if (vdso_ret != 0 || end_ret != 0) { - printf("[FAIL]\tvDSO returned %d, syscall errno=%d\n", - vdso_ret, errno); - nerrs++; + ksft_test_result_fail("vDSO returned %d, syscall errno=%d\n", vdso_ret, errno); return; }
- printf("\t%llu.%06ld %llu.%06ld %llu.%06ld\n", - (unsigned long long)start.tv_sec, start.tv_usec, - (unsigned long long)vdso.tv_sec, vdso.tv_usec, - (unsigned long long)end.tv_sec, end.tv_usec); + ksft_print_msg("\t%llu.%06ld %llu.%06ld %llu.%06ld\n", + (unsigned long long)start.tv_sec, start.tv_usec, + (unsigned long long)vdso.tv_sec, vdso.tv_usec, + (unsigned long long)end.tv_sec, end.tv_usec);
if (!tv_leq(&start, &vdso) || !tv_leq(&vdso, &end)) { - printf("[FAIL]\tTimes are out of sequence\n"); - nerrs++; + ksft_test_result_fail("Times are out of sequence\n"); + return; }
if (sys_tz.tz_minuteswest == vdso_tz.tz_minuteswest && sys_tz.tz_dsttime == vdso_tz.tz_dsttime) { - printf("[OK]\ttimezones match: minuteswest=%d, dsttime=%d\n", - sys_tz.tz_minuteswest, sys_tz.tz_dsttime); + ksft_test_result_pass("timezones match: minuteswest=%d, dsttime=%d\n", + sys_tz.tz_minuteswest, sys_tz.tz_dsttime); } else { - printf("[FAIL]\ttimezones do not match\n"); - nerrs++; + ksft_test_result_fail("timezones do not match\n"); } - - /* And make sure that passing NULL for tz doesn't crash. */ - vdso_gettimeofday(&vdso, NULL); }
int main(int argc, char **argv) { name = (const char **)&names[VDSO_NAMES];
+ ksft_print_header(); + ksft_set_plan(7 + ARRAY_SIZE(clocknames) * 2 + sysconf(_SC_NPROCESSORS_ONLN)); + fill_function_pointers();
test_clock_gettime(); @@ -444,5 +446,5 @@ int main(int argc, char **argv) */ test_getcpu();
- return nerrs ? 1 : 0; + ksft_finished(); }
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/vDSO/vdso_test_getcpu.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getcpu.c b/tools/testing/selftests/vDSO/vdso_test_getcpu.c index b758f68c6c9c2..d8e247783057a 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getcpu.c +++ b/tools/testing/selftests/vDSO/vdso_test_getcpu.c @@ -28,9 +28,12 @@ int main(int argc, char **argv) getcpu_t get_cpu; long ret;
+ ksft_print_header(); + ksft_set_plan(1); + sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); if (!sysinfo_ehdr) { - printf("AT_SYSINFO_EHDR is not present!\n"); + ksft_print_msg("AT_SYSINFO_EHDR is not present!\n"); return KSFT_SKIP; }
@@ -38,17 +41,12 @@ int main(int argc, char **argv)
get_cpu = (getcpu_t)vdso_sym(version, name[4]); if (!get_cpu) { - printf("Could not find %s\n", name[4]); + ksft_print_msg("Could not find %s\n", name[4]); return KSFT_SKIP; }
ret = get_cpu(&cpu, &node, 0); - if (ret == 0) { - printf("Running on CPU %u node %u\n", cpu, node); - } else { - printf("%s failed\n", name[4]); - return KSFT_FAIL; - } + ksft_test_result(ret == 0, "Running on CPU %u node %u\n", cpu, node);
- return 0; + ksft_finished(); }
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- .../selftests/vDSO/vdso_test_gettimeofday.c | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c index ee4f1ca56a71a..2f42277b29ffe 100644 --- a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c +++ b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c @@ -24,10 +24,13 @@ int main(int argc, char **argv) { const char *version = versions[VDSO_VERSION]; const char **name = (const char **)&names[VDSO_NAMES]; - unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); + + ksft_print_header(); + ksft_set_plan(1); + if (!sysinfo_ehdr) { - printf("AT_SYSINFO_EHDR is not present!\n"); + ksft_print_msg("AT_SYSINFO_EHDR is not present!\n"); return KSFT_SKIP; }
@@ -38,20 +41,18 @@ int main(int argc, char **argv) gtod_t gtod = (gtod_t)vdso_sym(version, name[0]);
if (!gtod) { - printf("Could not find %s\n", name[0]); + ksft_print_msg("Could not find %s\n", name[0]); return KSFT_SKIP; }
struct timeval tv; long ret = gtod(&tv, 0);
- if (ret == 0) { - printf("The time is %lld.%06lld\n", - (long long)tv.tv_sec, (long long)tv.tv_usec); - } else { - printf("%s failed\n", name[0]); - return KSFT_FAIL; - } + if (ret == 0) + ksft_test_result_pass("The time is %lld.%06lld\n", + (long long)tv.tv_sec, (long long)tv.tv_usec); + else + ksft_test_result_fail("%s failed\n", name[0]);
- return 0; + ksft_finished(); }
On 6/9/24 23:41, Muhammad Usama Anjum wrote:
Conform individual tests to TAP output. One patch conform one test. With this series, all vDSO tests become TAP conformant.
First patch conform the test by using kselftest_harness.h. Other patches are conforming using default kselftest.h helpers.
All tests have been tested multiple times before and after these patches. They are working correctly and outputting TAP messaging to find failures quikly when they happen.
Changes since v1:
- Update cover letter
- Update commit message of first patch
Muhammad Usama Anjum (4): kselftests: vdso: vdso_test_clock_getres: conform test to TAP output kselftests: vdso: vdso_test_correctness: conform test to TAP output kselftests: vdso: vdso_test_getcpu: conform test to TAP output kselftests: vdso: vdso_test_gettimeofday: conform test to TAP output
.../selftests/vDSO/vdso_test_clock_getres.c | 68 ++++---- .../selftests/vDSO/vdso_test_correctness.c | 146 +++++++++--------- .../testing/selftests/vDSO/vdso_test_getcpu.c | 16 +- .../selftests/vDSO/vdso_test_gettimeofday.c | 23 +-- 4 files changed, 126 insertions(+), 127 deletions(-)
I see two changes:
- concvering this test to use kselftes_harness. I am not seeing a value to this. - Second converting every single print to TAP. This isn't necessary since the kselftest wrapper prints appropriate summary message based on the KSFT_ code
I want to output before and after to assess the value of this change. TAP conversions make sense if and when they add value.
thanks, -- Shuah
On 6/12/24 1:37 AM, Shuah Khan wrote:
On 6/9/24 23:41, Muhammad Usama Anjum wrote:
Conform individual tests to TAP output. One patch conform one test. With this series, all vDSO tests become TAP conformant.
First patch conform the test by using kselftest_harness.h. Other patches are conforming using default kselftest.h helpers.
All tests have been tested multiple times before and after these patches. They are working correctly and outputting TAP messaging to find failures quikly when they happen.
Changes since v1:
- Update cover letter
- Update commit message of first patch
Muhammad Usama Anjum (4): kselftests: vdso: vdso_test_clock_getres: conform test to TAP output kselftests: vdso: vdso_test_correctness: conform test to TAP output kselftests: vdso: vdso_test_getcpu: conform test to TAP output kselftests: vdso: vdso_test_gettimeofday: conform test to TAP output
.../selftests/vDSO/vdso_test_clock_getres.c | 68 ++++---- .../selftests/vDSO/vdso_test_correctness.c | 146 +++++++++--------- .../testing/selftests/vDSO/vdso_test_getcpu.c | 16 +- .../selftests/vDSO/vdso_test_gettimeofday.c | 23 +-- 4 files changed, 126 insertions(+), 127 deletions(-)
I see two changes:
- concvering this test to use kselftes_harness. I am not seeing a value to
this.
- Second converting every single print to TAP. This isn't necessary since the
kselftest wrapper prints appropriate summary message based on the KSFT_ code
If a test prints standard output, only then CI and regression detection scripts identify if a test (and test case) was passed or failed. Or when it started failing. The attempt here to conform to TAP to get output which is recognizable and can easily be processed.
I want to output before and after to assess the value of this change. TAP conversions make sense if and when they add value.
Sure. Attaching before and after output for easy comparison. Please let me know about your thoughts.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Muhammad Usama Anjum -
Shuah Khan