diff mbox series

[v3,3/3] syscalls: Take use of TST_EXP_PASS_SILENT

Message ID 20210112020227.11775-3-yangx.jy@cn.fujitsu.com
State Accepted
Headers show
Series [v3,1/3] include/tst_test_macros.h: Add TST_EXP_{PASS, FD}_SILENT macros | expand

Commit Message

Xiao Yang Jan. 12, 2021, 2:02 a.m. UTC 
Current TST_EXP_PASS macro reports the double passed for
one subtest, for example:
--------------------------------------------
tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
access02.c:62: TPASS: access(file_f, F_OK) as root passed
access02.c:141: TPASS: access(file_f, F_OK) as root behaviour is correct.
access02.c:62: TPASS: access(file_f, F_OK) as nobody passed
access02.c:141: TPASS: access(file_f, F_OK) as nobody behaviour is correct.
access02.c:62: TPASS: access(file_r, R_OK) as root passed
...

Summary:
passed   32
failed   0
broken   0
skipped  0
warnings 0
--------------------------------------------

It is just a minor cleanup rather than a fix.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/access/access02.c | 2 +-
 testcases/kernel/syscalls/bind/bind04.c     | 2 +-
 testcases/kernel/syscalls/bind/bind05.c     | 2 +-
 testcases/kernel/syscalls/brk/brk01.c       | 6 +++++-
 testcases/kernel/syscalls/capget/capget01.c | 4 +++-
 5 files changed, 11 insertions(+), 5 deletions(-)

Comments

Cyril Hrubis Jan. 20, 2021, 10:34 a.m. UTC | #1 
Hi!
> diff --git a/testcases/kernel/syscalls/capget/capget01.c b/testcases/kernel/syscalls/capget/capget01.c
> index 6c17a7c7c..af088e2fc 100644
> --- a/testcases/kernel/syscalls/capget/capget01.c
> +++ b/testcases/kernel/syscalls/capget/capget01.c
> @@ -32,8 +32,10 @@ static void verify_capget(unsigned int n)
>  	hdr->version = tc->version;
>  	hdr->pid = pid;
>  
> -	TST_EXP_PASS(tst_syscall(__NR_capget, hdr, data),
> +	TST_EXP_PASS_SILENT(tst_syscall(__NR_capget, hdr, data),
>  	             "capget() with %s", tc->message);
> +	if (!TST_PASS)
> +		return;
>  
>  	if (data[0].effective & 1 << CAP_NET_RAW)
>  		tst_res(TFAIL, "capget() gets CAP_NET_RAW unexpectedly in pE");

I do not agree with the change in the capget01 here since there are
really two testcases there and the test was producing the same amount of
TPASS messages before the change to the TST_EXP_PASS() as well.

Other than that the patchset is fine.

So with the capget change removed you can add my:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Xiao Yang Jan. 21, 2021, 2:21 a.m. UTC | #2 
On 2021/1/20 18:34, Cyril Hrubis wrote:
> Hi!
>> diff --git a/testcases/kernel/syscalls/capget/capget01.c b/testcases/kernel/syscalls/capget/capget01.c
>> index 6c17a7c7c..af088e2fc 100644
>> --- a/testcases/kernel/syscalls/capget/capget01.c
>> +++ b/testcases/kernel/syscalls/capget/capget01.c
>> @@ -32,8 +32,10 @@ static void verify_capget(unsigned int n)
>>   	hdr->version = tc->version;
>>   	hdr->pid = pid;
>>
>> -	TST_EXP_PASS(tst_syscall(__NR_capget, hdr, data),
>> +	TST_EXP_PASS_SILENT(tst_syscall(__NR_capget, hdr, data),
>>   	             "capget() with %s", tc->message);
>> +	if (!TST_PASS)
>> +		return;
>>
>>   	if (data[0].effective&  1<<  CAP_NET_RAW)
>>   		tst_res(TFAIL, "capget() gets CAP_NET_RAW unexpectedly in pE");
> I do not agree with the change in the capget01 here since there are
> really two testcases there and the test was producing the same amount of
> TPASS messages before the change to the TST_EXP_PASS() as well.
Hi Cyril,

Running  capget01 with TST_EXP_PASS() shows:
--------------------------------------------------------
capget01.c:35: TPASS: capget() with LINUX_CAPABILITY_VERSION_1 passed
capget01.c:41: TPASS: capget() doesn't get CAP_NET_RAW as expected in PE
capget01.c:35: TPASS: capget() with LINUX_CAPABILITY_VERSION_2 passed
capget01.c:41: TPASS: capget() doesn't get CAP_NET_RAW as expected in PE
capget01.c:35: TPASS: capget() with LINUX_CAPABILITY_VERSION_3 passed
capget01.c:41: TPASS: capget() doesn't get CAP_NET_RAW as expected in PE

Summary:
passed   6
--------------------------------------------------------

Running  capget01 with TST_EXP_PASS_SILENT() shows:
--------------------------------------------------------
capget01.c:43: TPASS: capget() doesn't get CAP_NET_RAW as expected in PE
capget01.c:43: TPASS: capget() doesn't get CAP_NET_RAW as expected in PE
capget01.c:43: TPASS: capget() doesn't get CAP_NET_RAW as expected in PE

Summary:
passed   3
--------------------------------------------------------

If you want to keep two TPASS for a subtest, how about merging two TPASS 
into one? like this:
------------------------------------------------------------------------------------------
capget01.c:43: TPASS: capget() with LINUX_CAPABILITY_VERSION_1 doesn't 
get CAP_NET_RAW as expected in PE
capget01.c:43: TPASS: capget() with LINUX_CAPABILITY_VERSION_2 doesn't 
get CAP_NET_RAW as expected in PE
capget01.c:43: TPASS: capget() with LINUX_CAPABILITY_VERSION_3 doesn't 
get CAP_NET_RAW as expected in PE

Summary:
passed   3
------------------------------------------------------------------------------------------ 

BTW, I just want to avoid many TPASS for a subtest.

Best Regards,
Xiao Yang
> Other than that the patchset is fine.
>
> So with the capget change removed you can add my:
>
> Reviewed-by: Cyril Hrubis<chrubis@suse.cz>
>
Cyril Hrubis Jan. 22, 2021, 1:47 p.m. UTC | #3 
Hi!
> BTW, I just want to avoid many TPASS for a subtest.

Why? The capget01 clearly does two different tests per iteration.

There is absolutely no correlation between the number of TPASS test
produces and the number of iteration test does.
Xiao Yang Jan. 25, 2021, 3:44 a.m. UTC | #4 
On 2021/1/22 21:47, Cyril Hrubis wrote:
> Hi!
>> BTW, I just want to avoid many TPASS for a subtest.
> Why? The capget01 clearly does two different tests per iteration.
>
> There is absolutely no correlation between the number of TPASS test
> produces and the number of iteration test does.
Hi Cyril,

I pushed the patchset without capget01 for now.  But I think
there is a whole test per iteration, like three steps:
1) Drop CAP_NET_RAW.
2) Get effective by specifying kernel revision.
3) Check that the effective doesn't include CAP_NET_RAW.
Is it necessary for per steps to report TPASS? :-)

Best Regards,
Xiao Yang
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/access/access02.c b/testcases/kernel/syscalls/access/access02.c
index ff3e7b6f4..bf636f9f4 100644
--- a/testcases/kernel/syscalls/access/access02.c
+++ b/testcases/kernel/syscalls/access/access02.c
@@ -59,7 +59,7 @@  static void access_test(struct tcase *tc, const char *user)
 	struct stat stat_buf;
 	char command[64];
 
-	TST_EXP_PASS(access(tc->pathname, tc->mode),
+	TST_EXP_PASS_SILENT(access(tc->pathname, tc->mode),
 	             "access(%s, %s) as %s", tc->pathname, tc->name, user);
 
 	if (!TST_PASS)
diff --git a/testcases/kernel/syscalls/bind/bind04.c b/testcases/kernel/syscalls/bind/bind04.c
index 49e784ccd..de43b6c13 100644
--- a/testcases/kernel/syscalls/bind/bind04.c
+++ b/testcases/kernel/syscalls/bind/bind04.c
@@ -118,7 +118,7 @@  static void test_bind(unsigned int n)
 	listen_sock = SAFE_SOCKET(tc->address->sa_family, tc->type,
 		tc->protocol);
 
-	TST_EXP_PASS(bind(listen_sock, tc->address, tc->addrlen), "bind()");
+	TST_EXP_PASS_SILENT(bind(listen_sock, tc->address, tc->addrlen), "bind()");
 
 	if (!TST_PASS) {
 		SAFE_CLOSE(listen_sock);
diff --git a/testcases/kernel/syscalls/bind/bind05.c b/testcases/kernel/syscalls/bind/bind05.c
index 3b384cf1b..c43593fe1 100644
--- a/testcases/kernel/syscalls/bind/bind05.c
+++ b/testcases/kernel/syscalls/bind/bind05.c
@@ -131,7 +131,7 @@  static void test_bind(unsigned int n)
 	tst_res(TINFO, "Testing %s", tc->description);
 	sock = SAFE_SOCKET(tc->address->sa_family, tc->type, tc->protocol);
 
-	TST_EXP_PASS(bind(sock, tc->address, tc->addrlen), "bind()");
+	TST_EXP_PASS_SILENT(bind(sock, tc->address, tc->addrlen), "bind()");
 
 	if (!TST_PASS) {
 		SAFE_CLOSE(sock);
diff --git a/testcases/kernel/syscalls/brk/brk01.c b/testcases/kernel/syscalls/brk/brk01.c
index 3f8606375..a9b89bce5 100644
--- a/testcases/kernel/syscalls/brk/brk01.c
+++ b/testcases/kernel/syscalls/brk/brk01.c
@@ -31,7 +31,9 @@  void verify_brk(void)
 		break;
 		}
 
-		TST_EXP_PASS(brk((void *)new_brk), "brk()");
+		TST_EXP_PASS_SILENT(brk((void *)new_brk), "brk()");
+		if (!TST_PASS)
+			return;
 
 		cur_brk = (uintptr_t)sbrk(0);
 
@@ -46,6 +48,8 @@  void verify_brk(void)
 		if (i % 3 == 0)
 			*((char *)cur_brk) = 0;
 	}
+
+	tst_res(TPASS, "brk() works fine");
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/capget/capget01.c b/testcases/kernel/syscalls/capget/capget01.c
index 6c17a7c7c..af088e2fc 100644
--- a/testcases/kernel/syscalls/capget/capget01.c
+++ b/testcases/kernel/syscalls/capget/capget01.c
@@ -32,8 +32,10 @@  static void verify_capget(unsigned int n)
 	hdr->version = tc->version;
 	hdr->pid = pid;
 
-	TST_EXP_PASS(tst_syscall(__NR_capget, hdr, data),
+	TST_EXP_PASS_SILENT(tst_syscall(__NR_capget, hdr, data),
 	             "capget() with %s", tc->message);
+	if (!TST_PASS)
+		return;
 
 	if (data[0].effective & 1 << CAP_NET_RAW)
 		tst_res(TFAIL, "capget() gets CAP_NET_RAW unexpectedly in pE");