* [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed
@ 2021-01-04 12:54 Xiao Yang
2021-01-04 12:54 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded Xiao Yang
2021-01-05 13:15 ` [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed Cyril Hrubis
0 siblings, 2 replies; 13+ messages in thread
From: Xiao Yang @ 2021-01-04 12:54 UTC (permalink / raw)
To: ltp
Avoid calling fstat() with invalid fd:
-------------------------------------------
./open01
...
open01.c:53: TBROK: fstat(-1,0x7fff731410a0) failed: EBADF (9)
-------------------------------------------
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
testcases/kernel/syscalls/open/open01.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/testcases/kernel/syscalls/open/open01.c b/testcases/kernel/syscalls/open/open01.c
index 1172f832b..2f0ad550a 100644
--- a/testcases/kernel/syscalls/open/open01.c
+++ b/testcases/kernel/syscalls/open/open01.c
@@ -47,6 +47,9 @@ static void verify_open(unsigned int n)
TST_EXP_FD(open(tc->filename, tc->flag, tc->mode),
"open() with %s", tc->desc);
+ if (!TST_PASS)
+ return;
+
fd = TST_RET;
SAFE_FSTAT(fd, &buf);
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded
2021-01-04 12:54 [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed Xiao Yang
@ 2021-01-04 12:54 ` Xiao Yang
2021-01-04 13:07 ` yangx.jy
2021-01-05 13:36 ` Cyril Hrubis
2021-01-05 13:15 ` [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed Cyril Hrubis
1 sibling, 2 replies; 13+ messages in thread
From: Xiao Yang @ 2021-01-04 12:54 UTC (permalink / raw)
To: ltp
In Summary output, avoid counting the double passed for one test:
-------------------------------------
./open01
tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
open01.c:48: TPASS: open() with sticky bit returned fd 3
open01.c:59: TPASS: sticky bit is set as expected
open01.c:48: TPASS: open() with sirectory bit returned fd 3
open01.c:59: TPASS: sirectory bit is set as expected
Summary:
passed 4
failed 0
broken 0
skipped 0
warnings 0
-------------------------------------
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
include/tst_test_macros.h | 2 +-
lib/newlib_tests/test_macros01.c | 3 ++-
testcases/kernel/syscalls/open/open11.c | 2 ++
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index 3016d95c2..d1fc3cf70 100644
--- a/include/tst_test_macros.h
+++ b/include/tst_test_macros.h
@@ -64,7 +64,7 @@ extern void *TST_RET_PTR;
break; \
} \
\
- TST_MSGP_(TPASS, " returned fd %ld", TST_RET, \
+ TST_MSGP_(TINFO, " returned fd %ld", TST_RET, \
#SCALL, ##__VA_ARGS__); \
\
TST_PASS = 1; \
diff --git a/lib/newlib_tests/test_macros01.c b/lib/newlib_tests/test_macros01.c
index 9a920f8e4..9aa3885c7 100644
--- a/lib/newlib_tests/test_macros01.c
+++ b/lib/newlib_tests/test_macros01.c
@@ -30,7 +30,8 @@ static void do_test(void)
TST_EXP_FD(fail_fd(), "TEST DESCRIPTION");
tst_res(TINFO, "TST_PASS = %i", TST_PASS);
TST_EXP_FD(pass_fd(), "%s", "TEST DESCRIPTION PARAM");
- tst_res(TINFO, "TST_PASS = %i", TST_PASS);
+ if (TST_PASS)
+ tst_res(TPASS, "TST_PASS = %i", TST_PASS);
TST_EXP_FD(inval_val());
tst_res(TINFO, "TST_PASS = %i", TST_PASS);
}
diff --git a/testcases/kernel/syscalls/open/open11.c b/testcases/kernel/syscalls/open/open11.c
index ded384fa8..f7ac96d90 100644
--- a/testcases/kernel/syscalls/open/open11.c
+++ b/testcases/kernel/syscalls/open/open11.c
@@ -283,6 +283,8 @@ static void verify_open(unsigned int n)
} else if (tc[n].err == 0) {
TST_EXP_FD(open(tc[n].path, tc[n].flags, tc[n].mode),
"%s", tc[n].desc);
+ if (TST_PASS)
+ tst_res(TPASS, "%s", tc[n].desc);
} else {
TEST(open(tc[n].path, tc[n].flags, tc[n].mode));
tst_res(TPASS, "%s", tc[n].desc);
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded
2021-01-04 12:54 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded Xiao Yang
@ 2021-01-04 13:07 ` yangx.jy
2021-01-05 13:38 ` Cyril Hrubis
2021-01-05 13:36 ` Cyril Hrubis
1 sibling, 1 reply; 13+ messages in thread
From: yangx.jy @ 2021-01-04 13:07 UTC (permalink / raw)
To: ltp
Hi Cyril,
I will change the TST_EXP_PASS as well if you approve the patch.
Best Regards,
Xiao Yang
On 2021/1/4 20:54, Xiao Yang wrote:
> In Summary output, avoid counting the double passed for one test:
> -------------------------------------
> ./open01
> tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
> open01.c:48: TPASS: open() with sticky bit returned fd 3
> open01.c:59: TPASS: sticky bit is set as expected
> open01.c:48: TPASS: open() with sirectory bit returned fd 3
> open01.c:59: TPASS: sirectory bit is set as expected
>
> Summary:
> passed 4
> failed 0
> broken 0
> skipped 0
> warnings 0
> -------------------------------------
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> include/tst_test_macros.h | 2 +-
> lib/newlib_tests/test_macros01.c | 3 ++-
> testcases/kernel/syscalls/open/open11.c | 2 ++
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
> index 3016d95c2..d1fc3cf70 100644
> --- a/include/tst_test_macros.h
> +++ b/include/tst_test_macros.h
> @@ -64,7 +64,7 @@ extern void *TST_RET_PTR;
> break; \
> } \
> \
> - TST_MSGP_(TPASS, " returned fd %ld", TST_RET, \
> + TST_MSGP_(TINFO, " returned fd %ld", TST_RET, \
> #SCALL, ##__VA_ARGS__); \
> \
> TST_PASS = 1; \
> diff --git a/lib/newlib_tests/test_macros01.c b/lib/newlib_tests/test_macros01.c
> index 9a920f8e4..9aa3885c7 100644
> --- a/lib/newlib_tests/test_macros01.c
> +++ b/lib/newlib_tests/test_macros01.c
> @@ -30,7 +30,8 @@ static void do_test(void)
> TST_EXP_FD(fail_fd(), "TEST DESCRIPTION");
> tst_res(TINFO, "TST_PASS = %i", TST_PASS);
> TST_EXP_FD(pass_fd(), "%s", "TEST DESCRIPTION PARAM");
> - tst_res(TINFO, "TST_PASS = %i", TST_PASS);
> + if (TST_PASS)
> + tst_res(TPASS, "TST_PASS = %i", TST_PASS);
> TST_EXP_FD(inval_val());
> tst_res(TINFO, "TST_PASS = %i", TST_PASS);
> }
> diff --git a/testcases/kernel/syscalls/open/open11.c b/testcases/kernel/syscalls/open/open11.c
> index ded384fa8..f7ac96d90 100644
> --- a/testcases/kernel/syscalls/open/open11.c
> +++ b/testcases/kernel/syscalls/open/open11.c
> @@ -283,6 +283,8 @@ static void verify_open(unsigned int n)
> } else if (tc[n].err == 0) {
> TST_EXP_FD(open(tc[n].path, tc[n].flags, tc[n].mode),
> "%s", tc[n].desc);
> + if (TST_PASS)
> + tst_res(TPASS, "%s", tc[n].desc);
> } else {
> TEST(open(tc[n].path, tc[n].flags, tc[n].mode));
> tst_res(TPASS, "%s", tc[n].desc);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed
2021-01-04 12:54 [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed Xiao Yang
2021-01-04 12:54 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded Xiao Yang
@ 2021-01-05 13:15 ` Cyril Hrubis
2021-01-05 14:20 ` yangx.jy
1 sibling, 1 reply; 13+ messages in thread
From: Cyril Hrubis @ 2021-01-05 13:15 UTC (permalink / raw)
To: ltp
Hi!
> Avoid calling fstat() with invalid fd:
> -------------------------------------------
> ./open01
> ...
> open01.c:53: TBROK: fstat(-1,0x7fff731410a0) failed: EBADF (9)
> -------------------------------------------
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> testcases/kernel/syscalls/open/open01.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/open/open01.c b/testcases/kernel/syscalls/open/open01.c
> index 1172f832b..2f0ad550a 100644
> --- a/testcases/kernel/syscalls/open/open01.c
> +++ b/testcases/kernel/syscalls/open/open01.c
> @@ -47,6 +47,9 @@ static void verify_open(unsigned int n)
>
> TST_EXP_FD(open(tc->filename, tc->flag, tc->mode),
> "open() with %s", tc->desc);
> + if (!TST_PASS)
> + return;
> +
Doesn't this only happen if the open() that is supposed to return a
valid file descriptor fails unexpectedly? In that the test fails, right?
But I guess that it does not harm, so:
Acked-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded
2021-01-04 12:54 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded Xiao Yang
2021-01-04 13:07 ` yangx.jy
@ 2021-01-05 13:36 ` Cyril Hrubis
2021-01-05 14:39 ` yangx.jy
1 sibling, 1 reply; 13+ messages in thread
From: Cyril Hrubis @ 2021-01-05 13:36 UTC (permalink / raw)
To: ltp
Hi!
> In Summary output, avoid counting the double passed for one test:
> -------------------------------------
> ./open01
> tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
> open01.c:48: TPASS: open() with sticky bit returned fd 3
> open01.c:59: TPASS: sticky bit is set as expected
> open01.c:48: TPASS: open() with sirectory bit returned fd 3
> open01.c:59: TPASS: sirectory bit is set as expected
>
> Summary:
> passed 4
> failed 0
> broken 0
> skipped 0
> warnings 0
> -------------------------------------
What exactly is the problem of having TPASS generated for each open()
that produces a valid file descriptor in the open testcases?
These macros are especially tailored to generate TPASS/TFAIL messages so
that the caller does not have to. So even if having two TPASS per run()
function in open01 was a problem, which I do not think is the case, we
should just use SAFE_OPEN() there instead.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded
2021-01-04 13:07 ` yangx.jy
@ 2021-01-05 13:38 ` Cyril Hrubis
0 siblings, 0 replies; 13+ messages in thread
From: Cyril Hrubis @ 2021-01-05 13:38 UTC (permalink / raw)
To: ltp
Hi!
> I will change the TST_EXP_PASS as well if you approve the patch.
Please do not. The very purpose of TST_EXP_PASS() is to produce
TFAIL or TPASS.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed
2021-01-05 13:15 ` [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed Cyril Hrubis
@ 2021-01-05 14:20 ` yangx.jy
0 siblings, 0 replies; 13+ messages in thread
From: yangx.jy @ 2021-01-05 14:20 UTC (permalink / raw)
To: ltp
On 2021/1/5 21:15, Cyril Hrubis wrote:
> Hi!
>> Avoid calling fstat() with invalid fd:
>> -------------------------------------------
>> ./open01
>> ...
>> open01.c:53: TBROK: fstat(-1,0x7fff731410a0) failed: EBADF (9)
>> -------------------------------------------
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>> testcases/kernel/syscalls/open/open01.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/testcases/kernel/syscalls/open/open01.c b/testcases/kernel/syscalls/open/open01.c
>> index 1172f832b..2f0ad550a 100644
>> --- a/testcases/kernel/syscalls/open/open01.c
>> +++ b/testcases/kernel/syscalls/open/open01.c
>> @@ -47,6 +47,9 @@ static void verify_open(unsigned int n)
>>
>> TST_EXP_FD(open(tc->filename, tc->flag, tc->mode),
>> "open() with %s", tc->desc);
>> + if (!TST_PASS)
>> + return;
>> +
> Doesn't this only happen if the open() that is supposed to return a
> valid file descriptor fails unexpectedly? In that the test fails, right?
Hi Cyril,
Right.
> But I guess that it does not harm, so:
>
> Acked-by: Cyril Hrubis<chrubis@suse.cz>
I will push it soon.
Best Regards,
Xiao Yang
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded
2021-01-05 13:36 ` Cyril Hrubis
@ 2021-01-05 14:39 ` yangx.jy
2021-01-05 15:03 ` Cyril Hrubis
0 siblings, 1 reply; 13+ messages in thread
From: yangx.jy @ 2021-01-05 14:39 UTC (permalink / raw)
To: ltp
On 2021/1/5 21:36, Cyril Hrubis wrote:
> Hi!
>> In Summary output, avoid counting the double passed for one test:
>> -------------------------------------
>> ./open01
>> tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
>> open01.c:48: TPASS: open() with sticky bit returned fd 3
>> open01.c:59: TPASS: sticky bit is set as expected
>> open01.c:48: TPASS: open() with sirectory bit returned fd 3
>> open01.c:59: TPASS: sirectory bit is set as expected
>>
>> Summary:
>> passed 4
>> failed 0
>> broken 0
>> skipped 0
>> warnings 0
>> -------------------------------------
> What exactly is the problem of having TPASS generated for each open()
> that produces a valid file descriptor in the open testcases?
>
> These macros are especially tailored to generate TPASS/TFAIL messages so
> that the caller does not have to. So even if having two TPASS per run()
> function in open01 was a problem, which I do not think is the case, we
> should just use SAFE_OPEN() there instead.
Hi Cyril,
I perfer to report one TPASS message when finishing one subtest instead
of one step.
because of two reasons:
a) It seems clearer for user to know how many subtests were run
sucessfully.
b) There are too many TPASS/TFAIL messages when a testcase(e.g.
open11) contains many subtests or multiple TST_EXP_* macros.
Could we make TST_EXP_* macros do common check and generate only TFAIL
messages? (Make testcases report TPASS message by themself)
Best Regards,
Xiao Yang
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded
2021-01-05 14:39 ` yangx.jy
@ 2021-01-05 15:03 ` Cyril Hrubis
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Xiao Yang
2021-01-10 13:48 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded yangx.jy
0 siblings, 2 replies; 13+ messages in thread
From: Cyril Hrubis @ 2021-01-05 15:03 UTC (permalink / raw)
To: ltp
Hi!
> > What exactly is the problem of having TPASS generated for each open()
> > that produces a valid file descriptor in the open testcases?
> >
> > These macros are especially tailored to generate TPASS/TFAIL messages so
> > that the caller does not have to. So even if having two TPASS per run()
> > function in open01 was a problem, which I do not think is the case, we
> > should just use SAFE_OPEN() there instead.
> Hi Cyril,
>
> I perfer to report one TPASS message when finishing one subtest instead
> of one step.
> because of two reasons:
> a) It seems clearer for user to know how many subtests were run
> sucessfully.
This depends on a definition of subtest which, as far as I can tell, is
not that well defined.
If you want to make the output easier to read in the case of a test
function that is called more than once we should change the test library
to insert empty lines between the iterations or something along these
lines, so that the output would look like:
./open01
tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
open01.c:48: TPASS: open() with sticky bit returned fd 3
open01.c:56: TPASS: sticky bit is set as expected
open01.c:48: TPASS: open() with sirectory bit returned fd 3
open01.c:56: TPASS: sirectory bit is set as expected
Summary:
passed 4
failed 0
broken 0
skipped 0
warnings 0
Which is way better than imposing any rules on how many TPASS/TFAIL
messages should be printed per iteration.
> b) There are too many TPASS/TFAIL messages when a testcase(e.g.
> open11) contains many subtests or multiple TST_EXP_* macros.
There are 28 testcases in open11 and we print 28 messages TPASS/TFAIL
messages, we just use different EXP macro for different testcases. I do
not think that silencing the output would make things any better.
> Could we make TST_EXP_* macros do common check and generate only TFAIL
> messages? (Make testcases report TPASS message by themself)
The whole idea of having the TST_EXP_* macros is that they replace code
that looks like:
if (condition)
tst_res(TPASS, "...");
else
tst_res(TFAIL, "...");
And that they actually unify how these messages are formatted.
If you think that there are places where we use these macros where
SAFE_MACRO() would be better we can talk about changing these.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros
2021-01-05 15:03 ` Cyril Hrubis
@ 2021-01-10 13:04 ` Xiao Yang
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 2/2] syscalls/access02.c: Take use of TST_EXP_SILENT_PASS Xiao Yang
2021-01-11 14:07 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Cyril Hrubis
2021-01-10 13:48 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded yangx.jy
1 sibling, 2 replies; 13+ messages in thread
From: Xiao Yang @ 2021-01-10 13:04 UTC (permalink / raw)
To: ltp
1) TST_EXP_SILENT_{PASS,FD} don't report TPASS when SCALL succeeds.
2) TST_EXP_{PASS,FD} calls TST_EXP_SILENT_{PASS,FD} and report
TPASS when SCALL succeeds.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
include/tst_test_macros.h | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index 3016d95c2..8b71da00b 100644
--- a/include/tst_test_macros.h
+++ b/include/tst_test_macros.h
@@ -46,7 +46,7 @@ extern void *TST_RET_PTR;
tst_res_(__FILE__, __LINE__, RES, \
TST_FMT_(TST_2_(dummy, ##__VA_ARGS__, SCALL) FMT, __VA_ARGS__), PAR)
-#define TST_EXP_FD(SCALL, ...) \
+#define TST_EXP_SILENT_FD(SCALL, ...) \
do { \
TEST(SCALL); \
\
@@ -64,14 +64,20 @@ extern void *TST_RET_PTR;
break; \
} \
\
- TST_MSGP_(TPASS, " returned fd %ld", TST_RET, \
- #SCALL, ##__VA_ARGS__); \
- \
TST_PASS = 1; \
\
} while (0)
-#define TST_EXP_PASS(SCALL, ...) \
+#define TST_EXP_FD(SCALL, ...) \
+ do { \
+ TST_EXP_SILENT_FD(SCALL, __VA_ARGS__); \
+ \
+ if (TST_PASS) \
+ TST_MSGP_(TPASS, " returned fd %ld", TST_RET, \
+ #SCALL, ##__VA_ARGS__); \
+ } while (0)
+
+#define TST_EXP_SILENT_PASS(SCALL, ...) \
do { \
TEST(SCALL); \
\
@@ -89,12 +95,17 @@ extern void *TST_RET_PTR;
break; \
} \
\
- TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__); \
- \
TST_PASS = 1; \
\
} while (0)
+#define TST_EXP_PASS(SCALL, ...) \
+ do { \
+ TST_EXP_SILENT_PASS(SCALL, __VA_ARGS__); \
+ \
+ if (TST_PASS) \
+ TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__); \
+ } while (0) \
#define TST_EXP_FAIL(SCALL, ERRNO, ...) \
do { \
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH v2 2/2] syscalls/access02.c: Take use of TST_EXP_SILENT_PASS
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Xiao Yang
@ 2021-01-10 13:04 ` Xiao Yang
2021-01-11 14:07 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Cyril Hrubis
1 sibling, 0 replies; 13+ messages in thread
From: Xiao Yang @ 2021-01-10 13:04 UTC (permalink / raw)
To: ltp
Current TST_EXP_PASS macro reports the double passed for one subtest:
--------------------------------------------
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.
--------------------------------------------
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 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/testcases/kernel/syscalls/access/access02.c b/testcases/kernel/syscalls/access/access02.c
index ff3e7b6f4..19bf87c54 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_SILENT_PASS(access(tc->pathname, tc->mode),
"access(%s, %s) as %s", tc->pathname, tc->name, user);
if (!TST_PASS)
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded
2021-01-05 15:03 ` Cyril Hrubis
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Xiao Yang
@ 2021-01-10 13:48 ` yangx.jy
1 sibling, 0 replies; 13+ messages in thread
From: yangx.jy @ 2021-01-10 13:48 UTC (permalink / raw)
To: ltp
Hi Cyril,
Sorry for the late reply because I am busy with other things recently.
On 2021/1/5 23:03, Cyril Hrubis wrote:
> Hi!
>>> What exactly is the problem of having TPASS generated for each open()
>>> that produces a valid file descriptor in the open testcases?
>>>
>>> These macros are especially tailored to generate TPASS/TFAIL messages so
>>> that the caller does not have to. So even if having two TPASS per run()
>>> function in open01 was a problem, which I do not think is the case, we
>>> should just use SAFE_OPEN() there instead.
>> Hi Cyril,
>>
>> I perfer to report one TPASS message when finishing one subtest instead
>> of one step.
>> because of two reasons:
>> a) It seems clearer for user to know how many subtests were run
>> sucessfully.
> This depends on a definition of subtest which, as far as I can tell, is
> not that well defined.
>
> If you want to make the output easier to read in the case of a test
> function that is called more than once we should change the test library
> to insert empty lines between the iterations or something along these
> lines, so that the output would look like:
>
> ./open01
> tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
>
> open01.c:48: TPASS: open() with sticky bit returned fd 3
> open01.c:56: TPASS: sticky bit is set as expected
>
> open01.c:48: TPASS: open() with sirectory bit returned fd 3
> open01.c:56: TPASS: sirectory bit is set as expected
>
> Summary:
> passed 4
> failed 0
> broken 0
> skipped 0
> warnings 0
>
> Which is way better than imposing any rules on how many TPASS/TFAIL
> messages should be printed per iteration.
>
>> b) There are too many TPASS/TFAIL messages when a testcase(e.g.
>> open11) contains many subtests or multiple TST_EXP_* macros.
> There are 28 testcases in open11 and we print 28 messages TPASS/TFAIL
> messages, we just use different EXP macro for different testcases. I do
> not think that silencing the output would make things any better.
I gave a wrong example. access02 may be the correct one:
-----------------------------------------------------------------
# ./access02
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
access02.c:141: TPASS: access(file_r, R_OK) as root behaviour is correct.
access02.c:62: TPASS: access(file_r, R_OK) as nobody passed
access02.c:141: TPASS: access(file_r, R_OK) as nobody behaviour is correct.
...
Summary:
passed 32
failed 0
broken 0
skipped 0
warnings 0
-----------------------------------------------------------------
>> Could we make TST_EXP_* macros do common check and generate only TFAIL
>> messages? (Make testcases report TPASS message by themself)
> The whole idea of having the TST_EXP_* macros is that they replace code
> that looks like:
>
> if (condition)
> tst_res(TPASS, "...");
> else
> tst_res(TFAIL, "...");
>
> And that they actually unify how these messages are formatted.
>
> If you think that there are places where we use these macros where
> SAFE_MACRO() would be better we can talk about changing these.
I thought about it again yesterday and wanted to introduce new TST_EXP_
SILENT_{FD,PASS} macros.
1) TST_EXP_SILENT_{PASS,FD} do common checks and don't report TPASS when
SCALL succeeds.
2) TST_EXP_{PASS,FD} take use of TST_EXP_SILENT_{PASS,FD} and report
TPASS when SCALL succeeds.
BTW: Make testcase use which macros by themselves.
Please see my v2 patch set for detailed implement:
http://lists.linux.it/pipermail/ltp/2021-January/020472.html
http://lists.linux.it/pipermail/ltp/2021-January/020473.html
Best Regards,
Xiao Yang
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Xiao Yang
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 2/2] syscalls/access02.c: Take use of TST_EXP_SILENT_PASS Xiao Yang
@ 2021-01-11 14:07 ` Cyril Hrubis
1 sibling, 0 replies; 13+ messages in thread
From: Cyril Hrubis @ 2021-01-11 14:07 UTC (permalink / raw)
To: ltp
Hi!
> 1) TST_EXP_SILENT_{PASS,FD} don't report TPASS when SCALL succeeds.
> 2) TST_EXP_{PASS,FD} calls TST_EXP_SILENT_{PASS,FD} and report
> TPASS when SCALL succeeds.
I think that the macros should have the silent after the {PASS,FD} since
that would be less confusing. I other words what is silent fd?
So I would name these as TST_EXP_FD_SILENT() or maybe
TST_EXP_FD_QUIET().
Other than that the change looks good.
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> include/tst_test_macros.h | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
> index 3016d95c2..8b71da00b 100644
> --- a/include/tst_test_macros.h
> +++ b/include/tst_test_macros.h
> @@ -46,7 +46,7 @@ extern void *TST_RET_PTR;
> tst_res_(__FILE__, __LINE__, RES, \
> TST_FMT_(TST_2_(dummy, ##__VA_ARGS__, SCALL) FMT, __VA_ARGS__), PAR)
>
> -#define TST_EXP_FD(SCALL, ...) \
> +#define TST_EXP_SILENT_FD(SCALL, ...) \
> do { \
> TEST(SCALL); \
> \
> @@ -64,14 +64,20 @@ extern void *TST_RET_PTR;
> break; \
> } \
> \
> - TST_MSGP_(TPASS, " returned fd %ld", TST_RET, \
> - #SCALL, ##__VA_ARGS__); \
> - \
> TST_PASS = 1; \
> \
> } while (0)
>
> -#define TST_EXP_PASS(SCALL, ...) \
> +#define TST_EXP_FD(SCALL, ...) \
> + do { \
> + TST_EXP_SILENT_FD(SCALL, __VA_ARGS__); \
> + \
> + if (TST_PASS) \
> + TST_MSGP_(TPASS, " returned fd %ld", TST_RET, \
> + #SCALL, ##__VA_ARGS__); \
> + } while (0)
> +
> +#define TST_EXP_SILENT_PASS(SCALL, ...) \
> do { \
> TEST(SCALL); \
> \
> @@ -89,12 +95,17 @@ extern void *TST_RET_PTR;
> break; \
> } \
> \
> - TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__); \
> - \
> TST_PASS = 1; \
> \
> } while (0)
>
> +#define TST_EXP_PASS(SCALL, ...) \
> + do { \
> + TST_EXP_SILENT_PASS(SCALL, __VA_ARGS__); \
> + \
> + if (TST_PASS) \
> + TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__); \
> + } while (0) \
>
> #define TST_EXP_FAIL(SCALL, ERRNO, ...) \
> do { \
> --
> 2.21.0
>
>
>
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-01-11 14:07 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 12:54 [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed Xiao Yang
2021-01-04 12:54 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded Xiao Yang
2021-01-04 13:07 ` yangx.jy
2021-01-05 13:38 ` Cyril Hrubis
2021-01-05 13:36 ` Cyril Hrubis
2021-01-05 14:39 ` yangx.jy
2021-01-05 15:03 ` Cyril Hrubis
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Xiao Yang
2021-01-10 13:04 ` [LTP] [RFC PATCH v2 2/2] syscalls/access02.c: Take use of TST_EXP_SILENT_PASS Xiao Yang
2021-01-11 14:07 ` [LTP] [RFC PATCH v2 1/2] include/tst_test_macros.h: Add TST_EXP_SILENT_{PASS, FD} macros Cyril Hrubis
2021-01-10 13:48 ` [LTP] [RFC PATCH 2/2] include/tst_test_macros.h: Report TINFO when TST_EXP_FD() succeeded yangx.jy
2021-01-05 13:15 ` [LTP] [PATCH 1/2] syscalls/open01.c: Don't continue when open() failed Cyril Hrubis
2021-01-05 14:20 ` yangx.jy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.