* [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] [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] [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-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 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
* [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] [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] [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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox