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

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

[Domenico Cerasuolo]

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 subtests, the group result was marked as
SKIP. E.g.:

223/1   usdt/basic:SKIP
223/2   usdt/multispec:OK
223/3   usdt/urand_auto_attach:OK
223/4   usdt/urand_pid_attach:OK
223     usdt:SKIP

The test result of usdt in the example above
should be OK instead of SKIP, because the test
group did have passing tests and it would be
considered in "normal" state.

With this change, only if all of the subtests
were skipped, the group test is marked as SKIP.
When only some of the subtests are skipped, a
more detailed result is given, stating how
many of the subtests were skipped. E.g:

223/1   usdt/basic:SKIP
223/2   usdt/multispec:OK
223/3   usdt/urand_auto_attach:OK
223/4   usdt/urand_pid_attach:OK
223     usdt:OK (SKIP: 1/4)

changes from v1:
- added (SKIP: x/y) to OK tests that have
SKIP subtests
- merged print_test_name and test_result
functions as they were always called together

Signed-off-by: Domenico Cerasuolo <dceras@meta.com>
---
 tools/testing/selftests/bpf/test_progs.c | 38 ++++++++++++++----------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 0e9a47f97890..c34f37d7a523 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -222,6 +222,26 @@ static char *test_result(bool failed, bool skipped)
 	return failed ? "FAIL" : (skipped ? "SKIP" : "OK");
 }
 
+#define TEST_NUM_WIDTH 7
+
+static void print_test_result(const struct prog_test_def *test, const struct test_state *test_state)
+{
+	int skipped_cnt = test_state->skip_cnt;
+	int subtests_cnt = test_state->subtest_num;
+
+	fprintf(env.stdout, "#%-*d %s:", TEST_NUM_WIDTH, test->test_num, test->test_name);
+	if (test_state->error_cnt)
+		fprintf(env.stdout, "FAIL");
+	else if (!skipped_cnt)
+		fprintf(env.stdout, "OK");
+	else if (skipped_cnt == subtests_cnt || !subtests_cnt)
+		fprintf(env.stdout, "SKIP");
+	else
+		fprintf(env.stdout, "OK (SKIP: %d/%d)", skipped_cnt, subtests_cnt);
+
+	fprintf(env.stdout, "\n");
+}
+
 static void print_test_log(char *log_buf, size_t log_cnt)
 {
 	log_buf[log_cnt] = '\0';
@@ -230,18 +250,6 @@ static void print_test_log(char *log_buf, size_t log_cnt)
 		fprintf(env.stdout, "\n");
 }
 
-#define TEST_NUM_WIDTH 7
-
-static void print_test_name(int test_num, const char *test_name, char *result)
-{
-	fprintf(env.stdout, "#%-*d %s", TEST_NUM_WIDTH, test_num, test_name);
-
-	if (result)
-		fprintf(env.stdout, ":%s", result);
-
-	fprintf(env.stdout, "\n");
-}
-
 static void print_subtest_name(int test_num, int subtest_num,
 			       const char *test_name, char *subtest_name,
 			       char *result)
@@ -307,8 +315,7 @@ static void dump_test_log(const struct prog_test_def *test,
 					       subtest_state->skipped));
 	}
 
-	print_test_name(test->test_num, test->test_name,
-			test_result(test_failed, test_state->skip_cnt));
+	print_test_result(test, test_state);
 }
 
 static void stdio_restore(void);
@@ -1070,8 +1077,7 @@ static void run_one_test(int test_num)
 	state->tested = true;
 
 	if (verbose() && env.worker_id == -1)
-		print_test_name(test_num + 1, test->test_name,
-				test_result(state->error_cnt, state->skip_cnt));
+		print_test_result(test, state);
 
 	reset_affinity();
 	restore_netns();
-- 
2.30.2

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

[Yonghong Song]

On 11/9/22 10:40 AM, 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 subtests, the group result was marked as
> SKIP. E.g.:
> 
> 223/1   usdt/basic:SKIP
> 223/2   usdt/multispec:OK
> 223/3   usdt/urand_auto_attach:OK
> 223/4   usdt/urand_pid_attach:OK
> 223     usdt:SKIP
> 
> The test result of usdt in the example above
> should be OK instead of SKIP, because the test
> group did have passing tests and it would be
> considered in "normal" state.
> 
> With this change, only if all of the subtests
> were skipped, the group test is marked as SKIP.
> When only some of the subtests are skipped, a
> more detailed result is given, stating how
> many of the subtests were skipped. E.g:
> 
> 223/1   usdt/basic:SKIP
> 223/2   usdt/multispec:OK
> 223/3   usdt/urand_auto_attach:OK
> 223/4   usdt/urand_pid_attach:OK
> 223     usdt:OK (SKIP: 1/4)
> 
> changes from v1:
> - added (SKIP: x/y) to OK tests that have
> SKIP subtests
> - merged print_test_name and test_result
> functions as they were always called together
> 
> Signed-off-by: Domenico Cerasuolo <dceras@meta.com>

Acked-by: Yonghong Song <yhs@fb.com>

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Wed,  9 Nov 2022 10:40:39 -0800 you 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 subtests, the group result was marked as
> SKIP. E.g.:
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] selftests: fix test group SKIPPED result
    https://git.kernel.org/bpf/bpf-next/c/fd74b79df0d1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[John Fastabend]

Yonghong Song wrote:
> 
> 
> On 11/9/22 10:40 AM, 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 subtests, the group result was marked as
> > SKIP. E.g.:
> > 
> > 223/1   usdt/basic:SKIP
> > 223/2   usdt/multispec:OK
> > 223/3   usdt/urand_auto_attach:OK
> > 223/4   usdt/urand_pid_attach:OK
> > 223     usdt:SKIP
> > 
> > The test result of usdt in the example above
> > should be OK instead of SKIP, because the test
> > group did have passing tests and it would be
> > considered in "normal" state.
> > 
> > With this change, only if all of the subtests
> > were skipped, the group test is marked as SKIP.
> > When only some of the subtests are skipped, a
> > more detailed result is given, stating how
> > many of the subtests were skipped. E.g:
> > 
> > 223/1   usdt/basic:SKIP
> > 223/2   usdt/multispec:OK
> > 223/3   usdt/urand_auto_attach:OK
> > 223/4   usdt/urand_pid_attach:OK
> > 223     usdt:OK (SKIP: 1/4)
> > 
> > changes from v1:
> > - added (SKIP: x/y) to OK tests that have
> > SKIP subtests
> > - merged print_test_name and test_result
> > functions as they were always called together
> > 
> > Signed-off-by: Domenico Cerasuolo <dceras@meta.com>
> 
> Acked-by: Yonghong Song <yhs@fb.com>

Late but Ack from me as well this resolves my original comment.

Thanks!

Acked-by: John Fastabend <john.fastabend@gmail.com>