RE: [PATCH bpf-next] selftests: fix test group SKIPPED result

[John Fastabend <john.fastabend@gmail.com>]

Domenico Cerasuolo wrote:
> From: Domenico Cerasuolo <dceras@meta.com>
> 
> When showing the result of a test group, if one
> of the subtests was skipped, while still having
> passing subtets, the group result was marked as
> SKIPPED.
> 
> #223/1   usdt/basic:SKIP
> #223/2   usdt/multispec:OK
> #223     usdt:SKIP
> 
> With this change only if all of the subtests
> were skipped the group test is marked as SKIPPED.
> 
> #223/1   usdt/basic:SKIP
> #223/2   usdt/multispec:OK
> #223     usdt:OK

I'm not sure don't you want to know that some of the tests
were skipped? With this change its not knowable from output
if everything passed or one passed.

I would prefer the behavior: If anything fails return
FAIL, else if anything is skipped SKIP and if _everything_
passes mark it OK.

My preference is to drop this change.

> 
> Signed-off-by: Domenico Cerasuolo <dceras@meta.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 0e9a47f97890..14b70393018b 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -222,6 +222,11 @@ static char *test_result(bool failed, bool skipped)
>  	return failed ? "FAIL" : (skipped ? "SKIP" : "OK");
>  }
>  
> +static char *test_group_result(int tests_count, bool failed, int skipped)
> +{
> +	return failed ? "FAIL" : (skipped == tests_count ? "SKIP" : "OK");
> +}
> +
>  static void print_test_log(char *log_buf, size_t log_cnt)
>  {
>  	log_buf[log_cnt] = '\0';
> @@ -308,7 +313,8 @@ static void dump_test_log(const struct prog_test_def *test,
>  	}
>  
>  	print_test_name(test->test_num, test->test_name,
> -			test_result(test_failed, test_state->skip_cnt));
> +			test_group_result(test_state->subtest_num,
> +				test_failed, test_state->skip_cnt));
>  }
>  
>  static void stdio_restore(void);
> @@ -1071,7 +1077,8 @@ static void run_one_test(int test_num)
>  
>  	if (verbose() && env.worker_id == -1)
>  		print_test_name(test_num + 1, test->test_name,
> -				test_result(state->error_cnt, state->skip_cnt));
> +				test_group_result(state->subtest_num,
> +					state->error_cnt, state->skip_cnt));
>  
>  	reset_affinity();
>  	restore_netns();
> -- 
> 2.30.2
>