Message ID | 20240626105431.746411-1-liwang@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | [RFC,1/2] lib: add TST_RES_ serious macros | expand |
Hi Li, Sorry but I don't see the point on it. What we can do with tst_res() is enough and this patch adds one more layer of complexity to the main library. Both approaches are equivalent, also from the point of view of the ending result. I will let other maintainers discussing this feature, but from my point of view (as reviewer), I would reject it. Best regards, Andrea On 6/26/24 12:54, Li Wang wrote: > This patch introduces new macros to encapsulate existing tst_res and > tst_brk functions in the LTP library. These macros simplify the usage > of tst_ functions by providing a more user-friendly interface for > generating standardized test output. > > TST_RES_TINFO() -> tst_res(TINFO, ...) > TST_RES_TWARN() -> tst_res(TINFO, "WARNING: ", ...) > TST_RES_TPASS() -> tst_res(TPASS, ...) > TST_RES_TDEBUG() -> tst_res(TDEBUG, ...) > TST_RES_TFAIL() -> tst_res(TFAIL, ...) > > TST_BRK_TCONF() -> tst_brk(TCONF, ...) > TST_BRK_TBROK() -> tst_brk(TBROK, ...) > > TST_RES_TPASS_ER() -> tst_res(TPASS | errno, ...) > TST_RES_TFAIL_ER() -> tst_res(TFAIL | errno, ...) > TST_BRK_TBROK_ER() -> tst_brk(TBROK | errno, ... ) > > The macros handle various scenarios including simple messages and messages > with error flags, ensuring consistent logging of file and line information. > > Additionally, a new test case in tst_res_macros.c demonstrates the usage > of these macros. > > Signed-off-by: Li Wang <liwang@redhat.com> > --- > > Notes: > At the begining I wanted to implement one macro to implement functionality > like handling both messages and printing errno with the message. Still, > after trying some complex macro definition usages I found that this made > things too complex and confusing, so why not define more macros for special > use, and then this patch was signed. > > Maybe something like TST_RES_TFAIL_ER is not a good name, but please help to > give another, or any advise will be appreciated. > > include/tst_test_macros.h | 34 +++++++++++++++++++++ > lib/newlib_tests/tst_res_macros.c | 50 +++++++++++++++++++++++++++++++ > 2 files changed, 84 insertions(+) > create mode 100644 lib/newlib_tests/tst_res_macros.c > > diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h > index 22b39fb14..e36df9c0d 100644 > --- a/include/tst_test_macros.h > +++ b/include/tst_test_macros.h > @@ -368,4 +368,38 @@ const char *tst_errno_names(char *buf, const int *exp_errs, int exp_errs_cnt); > #define TST_EXP_EQ_SSZ(VAL_A, VAL_B) \ > TST_EXP_EQ_(VAL_A, #VAL_A, VAL_B, #VAL_B, ssize_t, "%zi") > > +/* Test result print macros */ > +#define TST_RES_TINFO(MESSAGE, ...) \ > + tst_res(TINFO, MESSAGE, ##__VA_ARGS__) > + > +#define TST_RES_TCONF(MESSAGE, ...) \ > + tst_res(TCONF, MESSAGE, ##__VA_ARGS__) > + > +#define TST_BRK_TCONF(MESSAGE, ...) \ > + tst_brk(TCONF, MESSAGE, ##__VA_ARGS__) > + > +#define TST_RES_TWARN(MESSAGE, ...) \ > + tst_res(TINFO, "WARNING: "MESSAGE, ##__VA_ARGS__) > + > +#define TST_RES_TPASS(MESSAGE, ...) \ > + tst_res(TPASS, MESSAGE, ##__VA_ARGS__) > + > +#define TST_RES_TPASS_ER(flag, MESSAGE, ...) \ > + tst_res(TPASS | flag, MESSAGE, ##__VA_ARGS__) > + > +#define TST_RES_TDEBUG(MESSAGE, ...) \ > + tst_res(TDEBUG, MESSAGE, ##__VA_ARGS__) > + > +#define TST_RES_TFAIL(MESSAGE, ...) \ > + tst_res(TFAIL, MESSAGE, ##__VA_ARGS__) > + > +#define TST_RES_TFAIL_ER(flag, MESSAGE, ...) \ > + tst_res(TFAIL | flag, MESSAGE, ##__VA_ARGS__) > + > +#define TST_BRK_TBROK(MESSAGE, ...) \ > + tst_brk(TBROK, MESSAGE, ##__VA_ARGS__) > + > +#define TST_BRK_TBROK_ER(flag, MESSAGE, ...) \ > + tst_brk(TBROK | flag, MESSAGE, ##__VA_ARGS__) > + > #endif /* TST_TEST_MACROS_H__ */ > diff --git a/lib/newlib_tests/tst_res_macros.c b/lib/newlib_tests/tst_res_macros.c > new file mode 100644 > index 000000000..74b656da0 > --- /dev/null > +++ b/lib/newlib_tests/tst_res_macros.c > @@ -0,0 +1,50 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2024 Li Wang <liwang@redhat.com> > + */ > + > +#include "tst_test.h" > + > +static void do_test(void) > +{ > + int i = 10, j = 20; > + char *str = "test"; > + > + TST_RES_TINFO("message"); > + TST_RES_TINFO("message, i = %d", i); > + TST_RES_TDEBUG("message, i = %d", i); > + > + TST_RES_TPASS("message"); > + TST_RES_TPASS("message, i = %d, j = %d, str = %s", i, j, str); > + TST_RES_TPASS_ER(TTERRNO, "message, i = %d, j = %d", i, j); > + > + TST_RES_TWARN("message"); > + TST_RES_TWARN("message, i = %d", i); > + > + TST_RES_TFAIL("message"); > + TST_RES_TFAIL("message, i = %d, j = %d", i, j); > + > + TST_RES_TFAIL_ER(TERRNO, "message"); > + TST_RES_TFAIL_ER(TERRNO, "message, i = %d, j = %d", i, j); > + TST_RES_TFAIL_ER(TERRNO, "message, i = %d, str = %s", i, str); > + > + TST_RES_TFAIL_ER(TTERRNO, "message"); > + TST_RES_TFAIL_ER(TTERRNO, "message, i = %d", i); > + TST_RES_TFAIL_ER(TTERRNO, "message, i = %d, str = %s", i, str); > + > + TST_BRK_TBROK("message"); > + TST_BRK_TBROK("message, i = %d, j = %d", i, j); > + > + TST_BRK_TBROK_ER(TERRNO, "message"); > + TST_BRK_TBROK_ER(TERRNO, "message, i = %d, str = %s", i, str); > + > + TST_BRK_TBROK_ER(TTERRNO, "message"); > + TST_BRK_TBROK_ER(TTERRNO, "message, i = %d", i); > + TST_BRK_TBROK_ER(TTERRNO, "message, i = %d, str = %s", i, str); > + > + TST_BRK_TCONF("message, i = %d, j = %d, str = %s", i, j, str); > +} > + > +static struct tst_test test = { > + .test_all = do_test, > +};
Hi! > This patch introduces new macros to encapsulate existing tst_res and > tst_brk functions in the LTP library. These macros simplify the usage > of tst_ functions by providing a more user-friendly interface for > generating standardized test output. > > TST_RES_TINFO() -> tst_res(TINFO, ...) > TST_RES_TWARN() -> tst_res(TINFO, "WARNING: ", ...) > TST_RES_TPASS() -> tst_res(TPASS, ...) > TST_RES_TDEBUG() -> tst_res(TDEBUG, ...) > TST_RES_TFAIL() -> tst_res(TFAIL, ...) > > TST_BRK_TCONF() -> tst_brk(TCONF, ...) > TST_BRK_TBROK() -> tst_brk(TBROK, ...) > > TST_RES_TPASS_ER() -> tst_res(TPASS | errno, ...) > TST_RES_TFAIL_ER() -> tst_res(TFAIL | errno, ...) > TST_BRK_TBROK_ER() -> tst_brk(TBROK | errno, ... ) > > The macros handle various scenarios including simple messages and messages > with error flags, ensuring consistent logging of file and line information. > > Additionally, a new test case in tst_res_macros.c demonstrates the usage > of these macros. I actually like the function better. It makes sense to add macros when they do something more complicated e.g. the TST_EXP_ macros save a lot of code, but in this case I do not think that they add any value, the change is only cosmetic.
Hi Cyril and Andrea, Cyril Hrubis <chrubis@suse.cz> wrote: > > > The macros handle various scenarios including simple messages and > messages > > with error flags, ensuring consistent logging of file and line > information. > > > > Additionally, a new test case in tst_res_macros.c demonstrates the usage > > of these macros. > > I actually like the function better. It makes sense to add macros when > they do something more complicated e.g. the TST_EXP_ macros save a lot > of code, but in this case I do not think that they add any value, the > change is only cosmetic. > Thanks for the feedback, I admit that those macros are a bit redundant. But back to the starting point [1], my original intention was just to mask TWARN as a failure in the test log. It keeps boring our engineers when reviewing the warnings in the automation job. What about just adding one macro to save energy on TWARN review? #define TST_RES_TWARN(MESSAGE, ...) \ tst_res(TINFO, "WARNING: "MESSAGE, ##__VA_ARGS__) [1] https://lists.linux.it/pipermail/ltp/2024-May/038561.html
diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h index 22b39fb14..e36df9c0d 100644 --- a/include/tst_test_macros.h +++ b/include/tst_test_macros.h @@ -368,4 +368,38 @@ const char *tst_errno_names(char *buf, const int *exp_errs, int exp_errs_cnt); #define TST_EXP_EQ_SSZ(VAL_A, VAL_B) \ TST_EXP_EQ_(VAL_A, #VAL_A, VAL_B, #VAL_B, ssize_t, "%zi") +/* Test result print macros */ +#define TST_RES_TINFO(MESSAGE, ...) \ + tst_res(TINFO, MESSAGE, ##__VA_ARGS__) + +#define TST_RES_TCONF(MESSAGE, ...) \ + tst_res(TCONF, MESSAGE, ##__VA_ARGS__) + +#define TST_BRK_TCONF(MESSAGE, ...) \ + tst_brk(TCONF, MESSAGE, ##__VA_ARGS__) + +#define TST_RES_TWARN(MESSAGE, ...) \ + tst_res(TINFO, "WARNING: "MESSAGE, ##__VA_ARGS__) + +#define TST_RES_TPASS(MESSAGE, ...) \ + tst_res(TPASS, MESSAGE, ##__VA_ARGS__) + +#define TST_RES_TPASS_ER(flag, MESSAGE, ...) \ + tst_res(TPASS | flag, MESSAGE, ##__VA_ARGS__) + +#define TST_RES_TDEBUG(MESSAGE, ...) \ + tst_res(TDEBUG, MESSAGE, ##__VA_ARGS__) + +#define TST_RES_TFAIL(MESSAGE, ...) \ + tst_res(TFAIL, MESSAGE, ##__VA_ARGS__) + +#define TST_RES_TFAIL_ER(flag, MESSAGE, ...) \ + tst_res(TFAIL | flag, MESSAGE, ##__VA_ARGS__) + +#define TST_BRK_TBROK(MESSAGE, ...) \ + tst_brk(TBROK, MESSAGE, ##__VA_ARGS__) + +#define TST_BRK_TBROK_ER(flag, MESSAGE, ...) \ + tst_brk(TBROK | flag, MESSAGE, ##__VA_ARGS__) + #endif /* TST_TEST_MACROS_H__ */ diff --git a/lib/newlib_tests/tst_res_macros.c b/lib/newlib_tests/tst_res_macros.c new file mode 100644 index 000000000..74b656da0 --- /dev/null +++ b/lib/newlib_tests/tst_res_macros.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2024 Li Wang <liwang@redhat.com> + */ + +#include "tst_test.h" + +static void do_test(void) +{ + int i = 10, j = 20; + char *str = "test"; + + TST_RES_TINFO("message"); + TST_RES_TINFO("message, i = %d", i); + TST_RES_TDEBUG("message, i = %d", i); + + TST_RES_TPASS("message"); + TST_RES_TPASS("message, i = %d, j = %d, str = %s", i, j, str); + TST_RES_TPASS_ER(TTERRNO, "message, i = %d, j = %d", i, j); + + TST_RES_TWARN("message"); + TST_RES_TWARN("message, i = %d", i); + + TST_RES_TFAIL("message"); + TST_RES_TFAIL("message, i = %d, j = %d", i, j); + + TST_RES_TFAIL_ER(TERRNO, "message"); + TST_RES_TFAIL_ER(TERRNO, "message, i = %d, j = %d", i, j); + TST_RES_TFAIL_ER(TERRNO, "message, i = %d, str = %s", i, str); + + TST_RES_TFAIL_ER(TTERRNO, "message"); + TST_RES_TFAIL_ER(TTERRNO, "message, i = %d", i); + TST_RES_TFAIL_ER(TTERRNO, "message, i = %d, str = %s", i, str); + + TST_BRK_TBROK("message"); + TST_BRK_TBROK("message, i = %d, j = %d", i, j); + + TST_BRK_TBROK_ER(TERRNO, "message"); + TST_BRK_TBROK_ER(TERRNO, "message, i = %d, str = %s", i, str); + + TST_BRK_TBROK_ER(TTERRNO, "message"); + TST_BRK_TBROK_ER(TTERRNO, "message, i = %d", i); + TST_BRK_TBROK_ER(TTERRNO, "message, i = %d, str = %s", i, str); + + TST_BRK_TCONF("message, i = %d, j = %d, str = %s", i, j, str); +} + +static struct tst_test test = { + .test_all = do_test, +};
This patch introduces new macros to encapsulate existing tst_res and tst_brk functions in the LTP library. These macros simplify the usage of tst_ functions by providing a more user-friendly interface for generating standardized test output. TST_RES_TINFO() -> tst_res(TINFO, ...) TST_RES_TWARN() -> tst_res(TINFO, "WARNING: ", ...) TST_RES_TPASS() -> tst_res(TPASS, ...) TST_RES_TDEBUG() -> tst_res(TDEBUG, ...) TST_RES_TFAIL() -> tst_res(TFAIL, ...) TST_BRK_TCONF() -> tst_brk(TCONF, ...) TST_BRK_TBROK() -> tst_brk(TBROK, ...) TST_RES_TPASS_ER() -> tst_res(TPASS | errno, ...) TST_RES_TFAIL_ER() -> tst_res(TFAIL | errno, ...) TST_BRK_TBROK_ER() -> tst_brk(TBROK | errno, ... ) The macros handle various scenarios including simple messages and messages with error flags, ensuring consistent logging of file and line information. Additionally, a new test case in tst_res_macros.c demonstrates the usage of these macros. Signed-off-by: Li Wang <liwang@redhat.com> --- Notes: At the begining I wanted to implement one macro to implement functionality like handling both messages and printing errno with the message. Still, after trying some complex macro definition usages I found that this made things too complex and confusing, so why not define more macros for special use, and then this patch was signed. Maybe something like TST_RES_TFAIL_ER is not a good name, but please help to give another, or any advise will be appreciated. include/tst_test_macros.h | 34 +++++++++++++++++++++ lib/newlib_tests/tst_res_macros.c | 50 +++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 lib/newlib_tests/tst_res_macros.c