| Message ID | 20240426101824.2894574-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] selftests: x86: conform test to TAP format output | expand |
Kind reminder On 4/26/24 3:18 PM, 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. > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Changes since v1: > - No changes, sending it again as got no response on v1 even after weeks > --- > tools/testing/selftests/x86/vdso_restorer.c | 29 +++++++++------------ > 1 file changed, 12 insertions(+), 17 deletions(-) > > diff --git a/tools/testing/selftests/x86/vdso_restorer.c b/tools/testing/selftests/x86/vdso_restorer.c > index fe99f24341554..f621167424a9c 100644 > --- a/tools/testing/selftests/x86/vdso_restorer.c > +++ b/tools/testing/selftests/x86/vdso_restorer.c > @@ -21,6 +21,7 @@ > #include <unistd.h> > #include <syscall.h> > #include <sys/syscall.h> > +#include "../kselftest.h" > > /* Open-code this -- the headers are too messy to easily use them. */ > struct real_sigaction { > @@ -44,17 +45,19 @@ static void handler_without_siginfo(int sig) > > int main() > { > - int nerrs = 0; > struct real_sigaction sa; > > + ksft_print_header(); > + ksft_set_plan(2); > + > void *vdso = dlopen("linux-vdso.so.1", > RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); > if (!vdso) > vdso = dlopen("linux-gate.so.1", > RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); > if (!vdso) { > - printf("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); > - return 0; > + ksft_print_msg("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); > + return KSFT_SKIP; > } > > memset(&sa, 0, sizeof(sa)); > @@ -62,21 +65,16 @@ int main() > sa.flags = SA_SIGINFO; > sa.restorer = NULL; /* request kernel-provided restorer */ > > - printf("[RUN]\tRaise a signal, SA_SIGINFO, sa.restorer == NULL\n"); > + ksft_print_msg("Raise a signal, SA_SIGINFO, sa.restorer == NULL\n"); > > if (syscall(SYS_rt_sigaction, SIGUSR1, &sa, NULL, 8) != 0) > err(1, "raw rt_sigaction syscall"); > > raise(SIGUSR1); > > - if (handler_called) { > - printf("[OK]\tSA_SIGINFO handler returned successfully\n"); > - } else { > - printf("[FAIL]\tSA_SIGINFO handler was not called\n"); > - nerrs++; > - } > + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); > > - printf("[RUN]\tRaise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); > + ksft_print_msg("Raise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); > > sa.flags = 0; > sa.handler = handler_without_siginfo; > @@ -86,10 +84,7 @@ int main() > > raise(SIGUSR1); > > - if (handler_called) { > - printf("[OK]\t!SA_SIGINFO handler returned successfully\n"); > - } else { > - printf("[FAIL]\t!SA_SIGINFO handler was not called\n"); > - nerrs++; > - } > + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); > + > + ksft_finished(); > }
Adding more reviewers. Please review. On 5/28/24 10:05 AM, Muhammad Usama Anjum wrote: > Kind reminder > > On 4/26/24 3:18 PM, 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. >> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >> --- >> Changes since v1: >> - No changes, sending it again as got no response on v1 even after weeks >> --- >> tools/testing/selftests/x86/vdso_restorer.c | 29 +++++++++------------ >> 1 file changed, 12 insertions(+), 17 deletions(-) >> >> diff --git a/tools/testing/selftests/x86/vdso_restorer.c b/tools/testing/selftests/x86/vdso_restorer.c >> index fe99f24341554..f621167424a9c 100644 >> --- a/tools/testing/selftests/x86/vdso_restorer.c >> +++ b/tools/testing/selftests/x86/vdso_restorer.c >> @@ -21,6 +21,7 @@ >> #include <unistd.h> >> #include <syscall.h> >> #include <sys/syscall.h> >> +#include "../kselftest.h" >> >> /* Open-code this -- the headers are too messy to easily use them. */ >> struct real_sigaction { >> @@ -44,17 +45,19 @@ static void handler_without_siginfo(int sig) >> >> int main() >> { >> - int nerrs = 0; >> struct real_sigaction sa; >> >> + ksft_print_header(); >> + ksft_set_plan(2); >> + >> void *vdso = dlopen("linux-vdso.so.1", >> RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); >> if (!vdso) >> vdso = dlopen("linux-gate.so.1", >> RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); >> if (!vdso) { >> - printf("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); >> - return 0; >> + ksft_print_msg("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); >> + return KSFT_SKIP; >> } >> >> memset(&sa, 0, sizeof(sa)); >> @@ -62,21 +65,16 @@ int main() >> sa.flags = SA_SIGINFO; >> sa.restorer = NULL; /* request kernel-provided restorer */ >> >> - printf("[RUN]\tRaise a signal, SA_SIGINFO, sa.restorer == NULL\n"); >> + ksft_print_msg("Raise a signal, SA_SIGINFO, sa.restorer == NULL\n"); >> >> if (syscall(SYS_rt_sigaction, SIGUSR1, &sa, NULL, 8) != 0) >> err(1, "raw rt_sigaction syscall"); >> >> raise(SIGUSR1); >> >> - if (handler_called) { >> - printf("[OK]\tSA_SIGINFO handler returned successfully\n"); >> - } else { >> - printf("[FAIL]\tSA_SIGINFO handler was not called\n"); >> - nerrs++; >> - } >> + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); >> >> - printf("[RUN]\tRaise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); >> + ksft_print_msg("Raise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); >> >> sa.flags = 0; >> sa.handler = handler_without_siginfo; >> @@ -86,10 +84,7 @@ int main() >> >> raise(SIGUSR1); >> >> - if (handler_called) { >> - printf("[OK]\t!SA_SIGINFO handler returned successfully\n"); >> - } else { >> - printf("[FAIL]\t!SA_SIGINFO handler was not called\n"); >> - nerrs++; >> - } >> + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); >> + >> + ksft_finished(); >> } >
This patch brings just readability implements by using kselftests wrappers instead of manual pass/fail test cases counting. It has been on mailing list from several months now. Please can someone ack or nack? On 7/1/24 1:38 PM, Muhammad Usama Anjum wrote: > Adding more reviewers. Please review. > > On 5/28/24 10:05 AM, Muhammad Usama Anjum wrote: >> Kind reminder >> >> On 4/26/24 3:18 PM, 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. >>> >>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >>> --- >>> Changes since v1: >>> - No changes, sending it again as got no response on v1 even after weeks >>> --- >>> tools/testing/selftests/x86/vdso_restorer.c | 29 +++++++++------------ >>> 1 file changed, 12 insertions(+), 17 deletions(-) >>> >>> diff --git a/tools/testing/selftests/x86/vdso_restorer.c b/tools/testing/selftests/x86/vdso_restorer.c >>> index fe99f24341554..f621167424a9c 100644 >>> --- a/tools/testing/selftests/x86/vdso_restorer.c >>> +++ b/tools/testing/selftests/x86/vdso_restorer.c >>> @@ -21,6 +21,7 @@ >>> #include <unistd.h> >>> #include <syscall.h> >>> #include <sys/syscall.h> >>> +#include "../kselftest.h" >>> >>> /* Open-code this -- the headers are too messy to easily use them. */ >>> struct real_sigaction { >>> @@ -44,17 +45,19 @@ static void handler_without_siginfo(int sig) >>> >>> int main() >>> { >>> - int nerrs = 0; >>> struct real_sigaction sa; >>> >>> + ksft_print_header(); >>> + ksft_set_plan(2); >>> + >>> void *vdso = dlopen("linux-vdso.so.1", >>> RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); >>> if (!vdso) >>> vdso = dlopen("linux-gate.so.1", >>> RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); >>> if (!vdso) { >>> - printf("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); >>> - return 0; >>> + ksft_print_msg("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); >>> + return KSFT_SKIP; >>> } >>> >>> memset(&sa, 0, sizeof(sa)); >>> @@ -62,21 +65,16 @@ int main() >>> sa.flags = SA_SIGINFO; >>> sa.restorer = NULL; /* request kernel-provided restorer */ >>> >>> - printf("[RUN]\tRaise a signal, SA_SIGINFO, sa.restorer == NULL\n"); >>> + ksft_print_msg("Raise a signal, SA_SIGINFO, sa.restorer == NULL\n"); >>> >>> if (syscall(SYS_rt_sigaction, SIGUSR1, &sa, NULL, 8) != 0) >>> err(1, "raw rt_sigaction syscall"); >>> >>> raise(SIGUSR1); >>> >>> - if (handler_called) { >>> - printf("[OK]\tSA_SIGINFO handler returned successfully\n"); >>> - } else { >>> - printf("[FAIL]\tSA_SIGINFO handler was not called\n"); >>> - nerrs++; >>> - } >>> + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); >>> >>> - printf("[RUN]\tRaise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); >>> + ksft_print_msg("Raise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); >>> >>> sa.flags = 0; >>> sa.handler = handler_without_siginfo; >>> @@ -86,10 +84,7 @@ int main() >>> >>> raise(SIGUSR1); >>> >>> - if (handler_called) { >>> - printf("[OK]\t!SA_SIGINFO handler returned successfully\n"); >>> - } else { >>> - printf("[FAIL]\t!SA_SIGINFO handler was not called\n"); >>> - nerrs++; >>> - } >>> + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); >>> + >>> + ksft_finished(); >>> } >> >
On 7/10/24 03:37, Muhammad Usama Anjum wrote: > This patch brings just readability implements by using kselftests wrappers > instead of manual pass/fail test cases counting. It has been on mailing > list from several months now. Please can someone ack or nack? > Okay. I think I responded to your other patches that are adding TAP to individual tests when kselftest wrapped does it for you based on return values. 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
On 7/10/24 9:16 PM, Shuah Khan wrote: > On 7/10/24 03:37, Muhammad Usama Anjum wrote: >> This patch brings just readability implements by using kselftests wrappers >> instead of manual pass/fail test cases counting. It has been on mailing >> list from several months now. Please can someone ack or nack? >> > > Okay. I think I responded to your other patches that are adding TAP > to individual tests when kselftest wrapped does it for you based on > return values. The current test doesn't return any exit value (hence implicitly always 0 is returned). The return value in addition to some other changes is getting fixed in this patch. > > 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. This patch is required to correctly return the value so that runtests fraework can correctly mark the result of the test. > > 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 >
On 7/11/24 00:52, Muhammad Usama Anjum wrote: > On 7/10/24 9:16 PM, Shuah Khan wrote: >> On 7/10/24 03:37, Muhammad Usama Anjum wrote: >>> This patch brings just readability implements by using kselftests wrappers >>> instead of manual pass/fail test cases counting. It has been on mailing >>> list from several months now. Please can someone ack or nack? >>> >> >> Okay. I think I responded to your other patches that are adding TAP >> to individual tests when kselftest wrapped does it for you based on >> return values. > The current test doesn't return any exit value (hence implicitly always 0 > is returned). The return value in addition to some other changes is getting > fixed in this patch. Yes. Fixing the return the problems. Please send patches to do that and I will take them. thanks, -- Shuah
On 7/11/24 9:39 PM, Shuah Khan wrote: > On 7/11/24 00:52, Muhammad Usama Anjum wrote: >> On 7/10/24 9:16 PM, Shuah Khan wrote: >>> On 7/10/24 03:37, Muhammad Usama Anjum wrote: >>>> This patch brings just readability implements by using kselftests wrappers >>>> instead of manual pass/fail test cases counting. It has been on mailing >>>> list from several months now. Please can someone ack or nack? >>>> >>> >>> Okay. I think I responded to your other patches that are adding TAP >>> to individual tests when kselftest wrapped does it for you based on >>> return values. >> The current test doesn't return any exit value (hence implicitly always 0 >> is returned). The return value in addition to some other changes is getting >> fixed in this patch. > > Yes. Fixing the return the problems. Please send patches to do that > and I will take them. I'll send today. > > thanks, > -- Shuah > >
diff --git a/tools/testing/selftests/x86/vdso_restorer.c b/tools/testing/selftests/x86/vdso_restorer.c index fe99f24341554..f621167424a9c 100644 --- a/tools/testing/selftests/x86/vdso_restorer.c +++ b/tools/testing/selftests/x86/vdso_restorer.c @@ -21,6 +21,7 @@ #include <unistd.h> #include <syscall.h> #include <sys/syscall.h> +#include "../kselftest.h" /* Open-code this -- the headers are too messy to easily use them. */ struct real_sigaction { @@ -44,17 +45,19 @@ static void handler_without_siginfo(int sig) int main() { - int nerrs = 0; struct real_sigaction sa; + ksft_print_header(); + ksft_set_plan(2); + void *vdso = dlopen("linux-vdso.so.1", RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); if (!vdso) vdso = dlopen("linux-gate.so.1", RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD); if (!vdso) { - printf("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); - return 0; + ksft_print_msg("[SKIP]\tFailed to find vDSO. Tests are not expected to work.\n"); + return KSFT_SKIP; } memset(&sa, 0, sizeof(sa)); @@ -62,21 +65,16 @@ int main() sa.flags = SA_SIGINFO; sa.restorer = NULL; /* request kernel-provided restorer */ - printf("[RUN]\tRaise a signal, SA_SIGINFO, sa.restorer == NULL\n"); + ksft_print_msg("Raise a signal, SA_SIGINFO, sa.restorer == NULL\n"); if (syscall(SYS_rt_sigaction, SIGUSR1, &sa, NULL, 8) != 0) err(1, "raw rt_sigaction syscall"); raise(SIGUSR1); - if (handler_called) { - printf("[OK]\tSA_SIGINFO handler returned successfully\n"); - } else { - printf("[FAIL]\tSA_SIGINFO handler was not called\n"); - nerrs++; - } + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); - printf("[RUN]\tRaise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); + ksft_print_msg("Raise a signal, !SA_SIGINFO, sa.restorer == NULL\n"); sa.flags = 0; sa.handler = handler_without_siginfo; @@ -86,10 +84,7 @@ int main() raise(SIGUSR1); - if (handler_called) { - printf("[OK]\t!SA_SIGINFO handler returned successfully\n"); - } else { - printf("[FAIL]\t!SA_SIGINFO handler was not called\n"); - nerrs++; - } + ksft_test_result(handler_called, "SA_SIGINFO handler returned\n"); + + 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> --- Changes since v1: - No changes, sending it again as got no response on v1 even after weeks --- tools/testing/selftests/x86/vdso_restorer.c | 29 +++++++++------------ 1 file changed, 12 insertions(+), 17 deletions(-)