Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output
@ 2024-06-10  5:41 Muhammad Usama Anjum
  2024-06-10  5:41 ` [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test " Muhammad Usama Anjum
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-06-10  5:41 UTC (permalink / raw)
  To: Shuah Khan, Vincenzo Frascino, Muhammad Usama Anjum, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel

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(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test to TAP output
  2024-06-10  5:41 [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output Muhammad Usama Anjum
@ 2024-06-10  5:41 ` Muhammad Usama Anjum
  2024-06-11 20:32   ` Shuah Khan
  2024-06-10  5:41 ` [PATCH v2 2/4] kselftests: vdso: vdso_test_correctness: " Muhammad Usama Anjum
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-06-10  5:41 UTC (permalink / raw)
  To: Shuah Khan, Vincenzo Frascino, Muhammad Usama Anjum, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel

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
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] kselftests: vdso: vdso_test_correctness: conform test to TAP output
  2024-06-10  5:41 [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output Muhammad Usama Anjum
  2024-06-10  5:41 ` [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test " Muhammad Usama Anjum
@ 2024-06-10  5:41 ` Muhammad Usama Anjum
  2024-06-10  5:41 ` [PATCH v2 3/4] kselftests: vdso: vdso_test_getcpu: " Muhammad Usama Anjum
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-06-10  5:41 UTC (permalink / raw)
  To: Shuah Khan, Vincenzo Frascino, Muhammad Usama Anjum, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel

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();
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] kselftests: vdso: vdso_test_getcpu: conform test to TAP output
  2024-06-10  5:41 [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output Muhammad Usama Anjum
  2024-06-10  5:41 ` [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test " Muhammad Usama Anjum
  2024-06-10  5:41 ` [PATCH v2 2/4] kselftests: vdso: vdso_test_correctness: " Muhammad Usama Anjum
@ 2024-06-10  5:41 ` Muhammad Usama Anjum
  2024-06-10  5:41 ` [PATCH v2 4/4] kselftests: vdso: vdso_test_gettimeofday: " Muhammad Usama Anjum
  2024-06-11 20:37 ` [PATCH v2 0/4] kselftests: vdso: conform tests " Shuah Khan
  4 siblings, 0 replies; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-06-10  5:41 UTC (permalink / raw)
  To: Shuah Khan, Vincenzo Frascino, Muhammad Usama Anjum, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel

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();
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] kselftests: vdso: vdso_test_gettimeofday: conform test to TAP output
  2024-06-10  5:41 [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output Muhammad Usama Anjum
                   ` (2 preceding siblings ...)
  2024-06-10  5:41 ` [PATCH v2 3/4] kselftests: vdso: vdso_test_getcpu: " Muhammad Usama Anjum
@ 2024-06-10  5:41 ` Muhammad Usama Anjum
  2024-06-11 20:37 ` [PATCH v2 0/4] kselftests: vdso: conform tests " Shuah Khan
  4 siblings, 0 replies; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-06-10  5:41 UTC (permalink / raw)
  To: Shuah Khan, Vincenzo Frascino, Muhammad Usama Anjum, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel

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();
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test to TAP output
  2024-06-10  5:41 ` [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test " Muhammad Usama Anjum
@ 2024-06-11 20:32   ` Shuah Khan
  2024-06-12  8:17     ` Muhammad Usama Anjum
  0 siblings, 1 reply; 11+ messages in thread
From: Shuah Khan @ 2024-06-11 20:32 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Vincenzo Frascino, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel, Shuah Khan

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output
  2024-06-10  5:41 [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output Muhammad Usama Anjum
                   ` (3 preceding siblings ...)
  2024-06-10  5:41 ` [PATCH v2 4/4] kselftests: vdso: vdso_test_gettimeofday: " Muhammad Usama Anjum
@ 2024-06-11 20:37 ` Shuah Khan
  2024-06-12  8:22   ` Muhammad Usama Anjum
  4 siblings, 1 reply; 11+ messages in thread
From: Shuah Khan @ 2024-06-11 20:37 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Vincenzo Frascino, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel, Shuah Khan

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test to TAP output
  2024-06-11 20:32   ` Shuah Khan
@ 2024-06-12  8:17     ` Muhammad Usama Anjum
  2024-07-02 10:16       ` Muhammad Usama Anjum
  2024-07-09 23:15       ` Shuah Khan
  0 siblings, 2 replies; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-06-12  8:17 UTC (permalink / raw)
  To: Shuah Khan, Shuah Khan, Vincenzo Frascino, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: Muhammad Usama Anjum, kernel

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
> 
> 

-- 
BR,
Muhammad Usama Anjum

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output
  2024-06-11 20:37 ` [PATCH v2 0/4] kselftests: vdso: conform tests " Shuah Khan
@ 2024-06-12  8:22   ` Muhammad Usama Anjum
  0 siblings, 0 replies; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-06-12  8:22 UTC (permalink / raw)
  To: Shuah Khan, Shuah Khan, Vincenzo Frascino, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: Muhammad Usama Anjum, kernel

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]

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
> 

-- 
BR,
Muhammad Usama Anjum

[-- Attachment #2: after --]
[-- Type: text/plain, Size: 8286 bytes --]

TAP version 13
1..6
# timeout set to 45
# selftests: vDSO: vdso_test_gettimeofday
# TAP version 13
# 1..1
# ok 1 The time is 1718178343.212990
# # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 1 selftests: vDSO: vdso_test_gettimeofday
# timeout set to 45
# selftests: vDSO: vdso_test_getcpu
# TAP version 13
# 1..1
# ok 1 Running on CPU 12 node 0
# # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 2 selftests: vDSO: vdso_test_getcpu
# timeout set to 45
# selftests: vDSO: vdso_test_abi
# TAP version 13
# 1..16
# # [vDSO kselftest] VDSO_VERSION: LINUX_2.6
# # The time is 1718178343.302971
# ok 1 __vdso_gettimeofday
# # clock_id: CLOCK_REALTIME
# # The time is 1718178343.302984142
# ok 2 __vdso_clock_gettime CLOCK_REALTIME
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 3 __vdso_clock_getres CLOCK_REALTIME
# # clock_id: CLOCK_BOOTTIME
# # The time is 1373292.867443353
# ok 4 __vdso_clock_gettime CLOCK_BOOTTIME
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 5 __vdso_clock_getres CLOCK_BOOTTIME
# # clock_id: CLOCK_TAI
# # The time is 1718178343.303018993
# ok 6 __vdso_clock_gettime CLOCK_TAI
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 7 __vdso_clock_getres CLOCK_TAI
# # clock_id: CLOCK_REALTIME_COARSE
# # The time is 1718178343.299307907
# ok 8 __vdso_clock_gettime CLOCK_REALTIME_COARSE
# # The vdso resolution is 0 4000000
# # The syscall resolution is 0 4000000
# ok 9 __vdso_clock_getres CLOCK_REALTIME_COARSE
# # clock_id: CLOCK_MONOTONIC
# # The time is 399550.224742318
# ok 10 __vdso_clock_gettime CLOCK_MONOTONIC
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 11 __vdso_clock_getres CLOCK_MONOTONIC
# # clock_id: CLOCK_MONOTONIC_RAW
# # The time is 399550.182218615
# ok 12 __vdso_clock_gettime CLOCK_MONOTONIC_RAW
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 13 __vdso_clock_getres CLOCK_MONOTONIC_RAW
# # clock_id: CLOCK_MONOTONIC_COARSE
# # The time is 399550.221001200
# ok 14 __vdso_clock_gettime CLOCK_MONOTONIC_COARSE
# # The vdso resolution is 0 4000000
# # The syscall resolution is 0 4000000
# ok 15 __vdso_clock_getres CLOCK_MONOTONIC_COARSE
# # The time in hours since January 1, 1970 is 477271
# ok 16 __vdso_time
# # Totals: pass:16 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 3 selftests: vDSO: vdso_test_abi
# timeout set to 45
# selftests: vDSO: 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
ok 4 selftests: vDSO: vdso_test_clock_getres
# timeout set to 45
# selftests: vDSO: vdso_standalone_test_x86
# The time is           1718178343.398375
ok 5 selftests: vDSO: vdso_standalone_test_x86
# timeout set to 45
# selftests: vDSO: vdso_test_correctness
# TAP version 13
# 1..47
# # Warning: failed to find vsyscall getcpu
# # Testing clock_gettime for clock CLOCK_REALTIME (0)...
# # 	1718178343.442886351 1718178343.442900598 1718178343.442902484
# ok 1 Test Passed.
# # Testing clock_gettime for clock CLOCK_MONOTONIC (1)...
# # 	399550.364607720 399550.364610094 399550.364612050
# ok 2 Test Passed.
# # Testing clock_gettime for clock CLOCK_PROCESS_CPUTIME_ID (2)...
# # 	0.003587313 0.003590008 0.003591481
# ok 3 Test Passed.
# # Testing clock_gettime for clock CLOCK_THREAD_CPUTIME_ID (3)...
# # 	0.003600978 0.003602651 0.003603964
# ok 4 Test Passed.
# # Testing clock_gettime for clock CLOCK_MONOTONIC_RAW (4)...
# # 	399550.322107274 399550.322109649 399550.322111534
# ok 5 Test Passed.
# # Testing clock_gettime for clock CLOCK_REALTIME_COARSE (5)...
# # 	1718178343.439308591 1718178343.439308591 1718178343.439308591
# ok 6 Test Passed.
# # Testing clock_gettime for clock CLOCK_MONOTONIC_COARSE (6)...
# # 	399550.361001884 399550.361001884 399550.361001884
# ok 7 Test Passed.
# # Testing clock_gettime for clock CLOCK_BOOTTIME (7)...
# # 	1373293.007423993 1373293.007426368 1373293.007428254
# ok 8 Test Passed.
# # Testing clock_gettime for clock CLOCK_REALTIME_ALARM (8)...
# # 	1718178343.442996909 1718178343.442999214 1718178343.443000681
# ok 9 Test Passed.
# # Testing clock_gettime for clock CLOCK_BOOTTIME_ALARM (9)...
# # 	1373293.007449904 1373293.007450882 1373293.007453257
# ok 10 Test Passed.
# # Testing clock_gettime for clock CLOCK_SGI_CYCLE (10)...
# ok 11 # SKIP No such clock.
# # Testing clock_gettime for clock CLOCK_TAI (11)...
# # 	1718178343.443026801 1718178343.443028198 1718178343.443030014
# ok 12 Test Passed.
# # Testing clock_gettime for clock invalid (-1)...
# ok 13 # SKIP No such clock.
# # Testing clock_gettime for clock invalid (-2147483648)...
# ok 14 # SKIP No such clock.
# # Testing clock_gettime for clock invalid (2147483647)...
# ok 15 # SKIP No such clock.
# ok 16 # SKIP No vDSO, so skipping CLOCK_REALTIME
# ok 17 # SKIP No vDSO, so skipping CLOCK_MONOTONIC
# ok 18 # SKIP No vDSO, so skipping CLOCK_PROCESS_CPUTIME_ID
# ok 19 # SKIP No vDSO, so skipping CLOCK_THREAD_CPUTIME_ID
# ok 20 # SKIP No vDSO, so skipping CLOCK_MONOTONIC_RAW
# ok 21 # SKIP No vDSO, so skipping CLOCK_REALTIME_COARSE
# ok 22 # SKIP No vDSO, so skipping CLOCK_MONOTONIC_COARSE
# ok 23 # SKIP No vDSO, so skipping CLOCK_BOOTTIME
# ok 24 # SKIP No vDSO, so skipping CLOCK_REALTIME_ALARM
# ok 25 # SKIP No vDSO, so skipping CLOCK_BOOTTIME_ALARM
# ok 26 # SKIP No vDSO, so skipping CLOCK_SGI_CYCLE
# ok 27 # SKIP No vDSO, so skipping CLOCK_TAI
# ok 28 # SKIP No vDSO, so skipping clock_gettime() tests -1
# ok 29 # SKIP No vDSO, so skipping clock_gettime() tests min
# ok 30 # SKIP No vDSO, so skipping clock_gettime() tests max
# # Testing gettimeofday...
# # 	1718178343.443086 1718178343.443089 1718178343.443091
# ok 31 timezones match: minuteswest=0, dsttime=0
# # Testing getcpu...
# # CPU 3616523017:#  syscall: cpu 0, node 0#  vdso: cpu 0, node 0# 
# ok 32 Succeeded
# # CPU 3616523017:#  syscall: cpu 1, node 0#  vdso: cpu 1, node 0# 
# ok 33 Succeeded
# # CPU 3616523017:#  syscall: cpu 2, node 0#  vdso: cpu 2, node 0# 
# ok 34 Succeeded
# # CPU 3616523017:#  syscall: cpu 3, node 0#  vdso: cpu 3, node 0# 
# ok 35 Succeeded
# # CPU 3616523017:#  syscall: cpu 4, node 0#  vdso: cpu 4, node 0# 
# ok 36 Succeeded
# # CPU 3616523017:#  syscall: cpu 5, node 0#  vdso: cpu 5, node 0# 
# ok 37 Succeeded
# # CPU 3616523017:#  syscall: cpu 6, node 0#  vdso: cpu 6, node 0# 
# ok 38 Succeeded
# # CPU 3616523017:#  syscall: cpu 7, node 0#  vdso: cpu 7, node 0# 
# ok 39 Succeeded
# # CPU 3616523017:#  syscall: cpu 8, node 0#  vdso: cpu 8, node 0# 
# ok 40 Succeeded
# # CPU 3616523017:#  syscall: cpu 9, node 0#  vdso: cpu 9, node 0# 
# ok 41 Succeeded
# # CPU 3616523017:#  syscall: cpu 10, node 0#  vdso: cpu 10, node 0# 
# ok 42 Succeeded
# # CPU 3616523017:#  syscall: cpu 11, node 0#  vdso: cpu 11, node 0# 
# ok 43 Succeeded
# # CPU 3616523017:#  syscall: cpu 12, node 0#  vdso: cpu 12, node 0# 
# ok 44 Succeeded
# # CPU 3616523017:#  syscall: cpu 13, node 0#  vdso: cpu 13, node 0# 
# ok 45 Succeeded
# # CPU 3616523017:#  syscall: cpu 14, node 0#  vdso: cpu 14, node 0# 
# ok 46 Succeeded
# # CPU 3616523017:#  syscall: cpu 15, node 0#  vdso: cpu 15, node 0# 
# ok 47 Succeeded
# # Totals: pass:28 fail:0 xfail:0 xpass:0 skip:19 error:0
ok 6 selftests: vDSO: vdso_test_correctness
make[1]: Leaving directory '/home/usama/repos/kernel/linux_mainline/tools/testing/selftests/vDSO'

[-- Attachment #3: before --]
[-- Type: text/plain, Size: 6020 bytes --]

TAP version 13
1..6
# timeout set to 45
# selftests: vDSO: vdso_test_gettimeofday
# The time is 1718178357.227235
ok 1 selftests: vDSO: vdso_test_gettimeofday
# timeout set to 45
# selftests: vDSO: vdso_test_getcpu
# Running on CPU 10 node 0
ok 2 selftests: vDSO: vdso_test_getcpu
# timeout set to 45
# selftests: vDSO: vdso_test_abi
# TAP version 13
# 1..16
# # [vDSO kselftest] VDSO_VERSION: LINUX_2.6
# # The time is 1718178357.315664
# ok 1 __vdso_gettimeofday
# # clock_id: CLOCK_REALTIME
# # The time is 1718178357.315676746
# ok 2 __vdso_clock_gettime CLOCK_REALTIME
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 3 __vdso_clock_getres CLOCK_REALTIME
# # clock_id: CLOCK_BOOTTIME
# # The time is 1373306.880135677
# ok 4 __vdso_clock_gettime CLOCK_BOOTTIME
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 5 __vdso_clock_getres CLOCK_BOOTTIME
# # clock_id: CLOCK_TAI
# # The time is 1718178357.315711806
# ok 6 __vdso_clock_gettime CLOCK_TAI
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 7 __vdso_clock_getres CLOCK_TAI
# # clock_id: CLOCK_REALTIME_COARSE
# # The time is 1718178357.311376408
# ok 8 __vdso_clock_gettime CLOCK_REALTIME_COARSE
# # The vdso resolution is 0 4000000
# # The syscall resolution is 0 4000000
# ok 9 __vdso_clock_getres CLOCK_REALTIME_COARSE
# # clock_id: CLOCK_MONOTONIC
# # The time is 399564.237434223
# ok 10 __vdso_clock_gettime CLOCK_MONOTONIC
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 11 __vdso_clock_getres CLOCK_MONOTONIC
# # clock_id: CLOCK_MONOTONIC_RAW
# # The time is 399564.194910799
# ok 12 __vdso_clock_gettime CLOCK_MONOTONIC_RAW
# # The vdso resolution is 0 1
# # The syscall resolution is 0 1
# ok 13 __vdso_clock_getres CLOCK_MONOTONIC_RAW
# # clock_id: CLOCK_MONOTONIC_COARSE
# # The time is 399564.233069701
# ok 14 __vdso_clock_gettime CLOCK_MONOTONIC_COARSE
# # The vdso resolution is 0 4000000
# # The syscall resolution is 0 4000000
# ok 15 __vdso_clock_getres CLOCK_MONOTONIC_COARSE
# # The time in hours since January 1, 1970 is 477271
# ok 16 __vdso_time
# # Totals: pass:16 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 3 selftests: vDSO: vdso_test_abi
# timeout set to 45
# selftests: vDSO: 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]
ok 4 selftests: vDSO: vdso_test_clock_getres
# timeout set to 45
# selftests: vDSO: vdso_standalone_test_x86
# The time is           1718178357.409346
ok 5 selftests: vDSO: vdso_standalone_test_x86
# timeout set to 45
# selftests: vDSO: vdso_test_correctness
# Warning: failed to find vsyscall getcpu
# [RUN]	Testing clock_gettime for clock CLOCK_REALTIME (0)...
# 	1718178357.456989189 1718178357.457004554 1718178357.457006929
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_MONOTONIC (1)...
# 	399564.378711117 399564.378713491 399564.378715866
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_PROCESS_CPUTIME_ID (2)...
# 	0.003684381 0.003687106 0.003688639
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_THREAD_CPUTIME_ID (3)...
# 	0.003697455 0.003699018 0.003700321
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_MONOTONIC_RAW (4)...
# 	399564.336209554 399564.336211928 399564.336213814
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_REALTIME_COARSE (5)...
# 	1718178357.451377093 1718178357.451377093 1718178357.451377093
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_MONOTONIC_COARSE (6)...
# 	399564.373070386 399564.373070386 399564.373070386
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_BOOTTIME (7)...
# 	1373307.021523898 1373307.021526273 1373307.021528159
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_REALTIME_ALARM (8)...
# 	1718178357.457097024 1718178357.457098002 1718178357.457099957
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_BOOTTIME_ALARM (9)...
# 	1373307.021548692 1373307.021551067 1373307.021552952
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock CLOCK_SGI_CYCLE (10)...
# [OK]	No such clock.
# [RUN]	Testing clock_gettime for clock CLOCK_TAI (11)...
# 	1718178357.457127056 1718178357.457129500 1718178357.457130478
# [OK]	Test Passed.
# [RUN]	Testing clock_gettime for clock invalid (-1)...
# [OK]	No such clock.
# [RUN]	Testing clock_gettime for clock invalid (-2147483648)...
# [OK]	No such clock.
# [RUN]	Testing clock_gettime for clock invalid (2147483647)...
# [OK]	No such clock.
# [SKIP]	No vDSO, so skipping clock_gettime64() tests
# [RUN]	Testing gettimeofday...
# 	1718178357.457155 1718178357.457156 1718178357.457158
# [OK]	timezones match: minuteswest=0, dsttime=0
# [RUN]	Testing getcpu...
# [OK]	CPU 0: syscall: cpu 0, node 0 vdso: cpu 0, node 0
# [OK]	CPU 1: syscall: cpu 1, node 0 vdso: cpu 1, node 0
# [OK]	CPU 2: syscall: cpu 2, node 0 vdso: cpu 2, node 0
# [OK]	CPU 3: syscall: cpu 3, node 0 vdso: cpu 3, node 0
# [OK]	CPU 4: syscall: cpu 4, node 0 vdso: cpu 4, node 0
# [OK]	CPU 5: syscall: cpu 5, node 0 vdso: cpu 5, node 0
# [OK]	CPU 6: syscall: cpu 6, node 0 vdso: cpu 6, node 0
# [OK]	CPU 7: syscall: cpu 7, node 0 vdso: cpu 7, node 0
# [OK]	CPU 8: syscall: cpu 8, node 0 vdso: cpu 8, node 0
# [OK]	CPU 9: syscall: cpu 9, node 0 vdso: cpu 9, node 0
# [OK]	CPU 10: syscall: cpu 10, node 0 vdso: cpu 10, node 0
# [OK]	CPU 11: syscall: cpu 11, node 0 vdso: cpu 11, node 0
# [OK]	CPU 12: syscall: cpu 12, node 0 vdso: cpu 12, node 0
# [OK]	CPU 13: syscall: cpu 13, node 0 vdso: cpu 13, node 0
# [OK]	CPU 14: syscall: cpu 14, node 0 vdso: cpu 14, node 0
# [OK]	CPU 15: syscall: cpu 15, node 0 vdso: cpu 15, node 0
ok 6 selftests: vDSO: vdso_test_correctness
make[1]: Leaving directory '/home/usama/repos/kernel/linux_mainline/tools/testing/selftests/vDSO'

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test to TAP output
  2024-06-12  8:17     ` Muhammad Usama Anjum
@ 2024-07-02 10:16       ` Muhammad Usama Anjum
  2024-07-09 23:15       ` Shuah Khan
  1 sibling, 0 replies; 11+ messages in thread
From: Muhammad Usama Anjum @ 2024-07-02 10:16 UTC (permalink / raw)
  To: Shuah Khan, Shuah Khan, Vincenzo Frascino, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: Muhammad Usama Anjum, kernel

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
>>
>>
> 

-- 
BR,
Muhammad Usama Anjum

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test to TAP output
  2024-06-12  8:17     ` Muhammad Usama Anjum
  2024-07-02 10:16       ` Muhammad Usama Anjum
@ 2024-07-09 23:15       ` Shuah Khan
  1 sibling, 0 replies; 11+ messages in thread
From: Shuah Khan @ 2024-07-09 23:15 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Vincenzo Frascino, Tiezhu Yang,
	linux-kselftest, linux-kernel
  Cc: kernel, Shuah Khan

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-07-09 23:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-10  5:41 [PATCH v2 0/4] kselftests: vdso: conform tests to TAP output Muhammad Usama Anjum
2024-06-10  5:41 ` [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform test " Muhammad Usama Anjum
2024-06-11 20:32   ` Shuah Khan
2024-06-12  8:17     ` Muhammad Usama Anjum
2024-07-02 10:16       ` Muhammad Usama Anjum
2024-07-09 23:15       ` Shuah Khan
2024-06-10  5:41 ` [PATCH v2 2/4] kselftests: vdso: vdso_test_correctness: " Muhammad Usama Anjum
2024-06-10  5:41 ` [PATCH v2 3/4] kselftests: vdso: vdso_test_getcpu: " Muhammad Usama Anjum
2024-06-10  5:41 ` [PATCH v2 4/4] kselftests: vdso: vdso_test_gettimeofday: " Muhammad Usama Anjum
2024-06-11 20:37 ` [PATCH v2 0/4] kselftests: vdso: conform tests " Shuah Khan
2024-06-12  8:22   ` Muhammad Usama Anjum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox