diff mbox series

[RFC,1/2] lib: add TST_RES_ serious macros

Message ID 20240626105431.746411-1-liwang@redhat.com
State Superseded
Headers show
Series [RFC,1/2] lib: add TST_RES_ serious macros | expand

Commit Message

Li Wang June 26, 2024, 10:54 a.m. UTC
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

Comments

Andrea Cervesato June 26, 2024, 11:46 a.m. UTC | #1
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,
> +};
Cyril Hrubis June 26, 2024, 11:53 a.m. UTC | #2
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.
Li Wang June 26, 2024, 1:16 p.m. UTC | #3
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 mbox series

Patch

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,
+};