[LTP] [PATCH] Disable failure hints before we actually run the test

[LTP] [PATCH] Disable failure hints before we actually run the test

[Cyril Hrubis]

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();
-- 
2.44.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] Disable failure hints before we actually run the test

[Andrea Cervesato via 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

Re: [LTP] [PATCH] Disable failure hints before we actually run the test

[Andrea Cervesato via 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

Re: [LTP] [PATCH] Disable failure hints before we actually run the test

[Martin Doucha]

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

Re: [LTP] [PATCH] Disable failure hints before we actually run the test

[Cyril Hrubis]

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

Re: [LTP] [PATCH] Disable failure hints before we actually run the test

[Martin Doucha]

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