* Re: [LTP] [PATCH] Disable failure hints before we actually run the test
2024-09-17 8:25 [LTP] [PATCH] Disable failure hints before we actually run the test Cyril Hrubis
@ 2024-09-17 12:22 ` Andrea Cervesato via ltp
2024-09-17 12:23 ` Andrea Cervesato via ltp
2024-09-17 14:36 ` Martin Doucha
2 siblings, 0 replies; 6+ messages in thread
From: Andrea Cervesato via ltp @ 2024-09-17 12:22 UTC (permalink / raw)
To: ltp
Hi Cyril,
LGTM
Acked-by: Andrea Cervesato <andrea.cervesato@mailbox.org>
On 9/17/24 10:25, Cyril Hrubis wrote:
> This is patch based on a suggestion from Peter Vorel:
>
> http://patchwork.ozlabs.org/project/ltp/patch/20240527202858.350200-2-pvorel@suse.cz/
>
> Peter proposed not to print failure hints (the tags) if we fail to
> acquire the device. This patch extends it for the whole test library
> intialization and enables the failure hints right before we jump into
> the test function.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> lib/tst_test.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 918bee2a1..3a71330b8 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -899,6 +899,8 @@ static void print_failure_hint(const char *tag, const char *hint,
> }
> }
>
> +static int show_failure_hints;
> +
> /* update also docparse/testinfo.pl */
> static void print_failure_hints(void)
> {
> @@ -919,7 +921,8 @@ static void do_exit(int ret)
>
> if (results->failed) {
> ret |= TFAIL;
> - print_failure_hints();
> + if (show_failure_hints)
> + print_failure_hints();
> }
>
> if (results->skipped && !results->passed)
> @@ -930,7 +933,8 @@ static void do_exit(int ret)
>
> if (results->broken) {
> ret |= TBROK;
> - print_failure_hints();
> + if (show_failure_hints)
> + print_failure_hints();
> }
>
> fprintf(stderr, "\nSummary:\n");
> @@ -1871,6 +1875,8 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
> if (tst_test->test_variants)
> test_variants = tst_test->test_variants;
>
> + show_failure_hints = 1;
> +
> for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
> if (tst_test->all_filesystems || count_fs_descs() > 1)
> ret |= run_tcases_per_fs();
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH] Disable failure hints before we actually run the test
2024-09-17 8:25 [LTP] [PATCH] Disable failure hints before we actually run the test Cyril Hrubis
2024-09-17 12:22 ` Andrea Cervesato via ltp
@ 2024-09-17 12:23 ` Andrea Cervesato via ltp
2024-09-17 14:36 ` Martin Doucha
2 siblings, 0 replies; 6+ messages in thread
From: Andrea Cervesato via ltp @ 2024-09-17 12:23 UTC (permalink / raw)
To: ltp
Hi Cyril,
LGTM
Acked-by: Andrea Cervesato <andrea.cervesato@suse.com>
On 9/17/24 10:25, Cyril Hrubis wrote:
> This is patch based on a suggestion from Peter Vorel:
>
> http://patchwork.ozlabs.org/project/ltp/patch/20240527202858.350200-2-pvorel@suse.cz/
>
> Peter proposed not to print failure hints (the tags) if we fail to
> acquire the device. This patch extends it for the whole test library
> intialization and enables the failure hints right before we jump into
> the test function.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> lib/tst_test.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 918bee2a1..3a71330b8 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -899,6 +899,8 @@ static void print_failure_hint(const char *tag, const char *hint,
> }
> }
>
> +static int show_failure_hints;
> +
> /* update also docparse/testinfo.pl */
> static void print_failure_hints(void)
> {
> @@ -919,7 +921,8 @@ static void do_exit(int ret)
>
> if (results->failed) {
> ret |= TFAIL;
> - print_failure_hints();
> + if (show_failure_hints)
> + print_failure_hints();
> }
>
> if (results->skipped && !results->passed)
> @@ -930,7 +933,8 @@ static void do_exit(int ret)
>
> if (results->broken) {
> ret |= TBROK;
> - print_failure_hints();
> + if (show_failure_hints)
> + print_failure_hints();
> }
>
> fprintf(stderr, "\nSummary:\n");
> @@ -1871,6 +1875,8 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
> if (tst_test->test_variants)
> test_variants = tst_test->test_variants;
>
> + show_failure_hints = 1;
> +
> for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
> if (tst_test->all_filesystems || count_fs_descs() > 1)
> ret |= run_tcases_per_fs();
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH] Disable failure hints before we actually run the test
2024-09-17 8:25 [LTP] [PATCH] Disable failure hints before we actually run the test Cyril Hrubis
2024-09-17 12:22 ` Andrea Cervesato via ltp
2024-09-17 12:23 ` Andrea Cervesato via ltp
@ 2024-09-17 14:36 ` Martin Doucha
2024-09-17 15:50 ` Cyril Hrubis
2 siblings, 1 reply; 6+ messages in thread
From: Martin Doucha @ 2024-09-17 14:36 UTC (permalink / raw)
To: Cyril Hrubis, ltp
Hi,
does it make sense to print failure hints after setup() failure? I can
think of a few cases where bug effects can be delayed until cleanup()
but it seems a bit early to me to cover setup() as well. I think the
flag could be moved to shared memory, enabled immediately before calling
run_tests() and disabled immediately after handling child's exit status
in fork_testrun().
On 17. 09. 24 10:25, Cyril Hrubis wrote:
> This is patch based on a suggestion from Peter Vorel:
>
> http://patchwork.ozlabs.org/project/ltp/patch/20240527202858.350200-2-pvorel@suse.cz/
>
> Peter proposed not to print failure hints (the tags) if we fail to
> acquire the device. This patch extends it for the whole test library
> intialization and enables the failure hints right before we jump into
> the test function.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> lib/tst_test.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 918bee2a1..3a71330b8 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -899,6 +899,8 @@ static void print_failure_hint(const char *tag, const char *hint,
> }
> }
>
> +static int show_failure_hints;
> +
> /* update also docparse/testinfo.pl */
> static void print_failure_hints(void)
> {
> @@ -919,7 +921,8 @@ static void do_exit(int ret)
>
> if (results->failed) {
> ret |= TFAIL;
> - print_failure_hints();
> + if (show_failure_hints)
> + print_failure_hints();
> }
>
> if (results->skipped && !results->passed)
> @@ -930,7 +933,8 @@ static void do_exit(int ret)
>
> if (results->broken) {
> ret |= TBROK;
> - print_failure_hints();
> + if (show_failure_hints)
> + print_failure_hints();
> }
>
> fprintf(stderr, "\nSummary:\n");
> @@ -1871,6 +1875,8 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
> if (tst_test->test_variants)
> test_variants = tst_test->test_variants;
>
> + show_failure_hints = 1;
> +
> for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
> if (tst_test->all_filesystems || count_fs_descs() > 1)
> ret |= run_tcases_per_fs();
--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH] Disable failure hints before we actually run the test
2024-09-17 14:36 ` Martin Doucha
@ 2024-09-17 15:50 ` Cyril Hrubis
2024-09-17 15:56 ` Martin Doucha
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2024-09-17 15:50 UTC (permalink / raw)
To: Martin Doucha; +Cc: ltp
Hi!
> does it make sense to print failure hints after setup() failure? I can
> think of a few cases where bug effects can be delayed until cleanup()
> but it seems a bit early to me to cover setup() as well. I think the
> flag could be moved to shared memory, enabled immediately before calling
> run_tests() and disabled immediately after handling child's exit status
> in fork_testrun().
My idea here was that we should always show the hints once we executed a
test function at least once. The rationale is that if the run_test()
corrupted kernel memory it may manifest at any point e.g. when we
attempt to setup a next loop device for the next filesytem (for
.all_filesystems).
So the idea is not to print them during the test library setup i.e.
do_setup() function but enable them right after that and keeping them
on.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] Disable failure hints before we actually run the test
2024-09-17 15:50 ` Cyril Hrubis
@ 2024-09-17 15:56 ` Martin Doucha
0 siblings, 0 replies; 6+ messages in thread
From: Martin Doucha @ 2024-09-17 15:56 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
On 17. 09. 24 17:50, Cyril Hrubis wrote:
> Hi!
>> does it make sense to print failure hints after setup() failure? I can
>> think of a few cases where bug effects can be delayed until cleanup()
>> but it seems a bit early to me to cover setup() as well. I think the
>> flag could be moved to shared memory, enabled immediately before calling
>> run_tests() and disabled immediately after handling child's exit status
>> in fork_testrun().
>
> My idea here was that we should always show the hints once we executed a
> test function at least once. The rationale is that if the run_test()
> corrupted kernel memory it may manifest at any point e.g. when we
> attempt to setup a next loop device for the next filesytem (for
> .all_filesystems).
>
> So the idea is not to print them during the test library setup i.e.
> do_setup() function but enable them right after that and keeping them
> on.
Yes, that makes sense. Though the enablement is still quite deep in
library setup. You could at least move it immediately before the fork()
in fork_testrun(), if you don't want to use shared memory.
--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread