Re: [PATCH bpf-next v10 07/11] selftests/bpf: Skip tests whose objects were not built

2026-04-30 Thread Ricardo B . Marlière
On Thu Apr 30, 2026 at 10:30 AM -03, bot+bpf-ci wrote:
>> diff --git a/tools/testing/selftests/bpf/test_progs.c 
>> b/tools/testing/selftests/bpf/test_progs.c
>> index cc14b13e23fe..7ba82974ee78 100644
>> --- a/tools/testing/selftests/bpf/test_progs.c
>> +++ b/tools/testing/selftests/bpf/test_progs.c
>
> [ ... ]
>
>> @@ -2046,15 +2069,20 @@ int main(int argc, char **argv)
>>  struct prog_test_def *test = &prog_test_defs[i];
>>
>>  test->test_num = i + 1;
>> -test->should_run = should_run(&env.test_selector,
>> -  test->test_num, test->test_name);
>> +test->selected = should_run(&env.test_selector,
>> +test->test_num, test->test_name);
>> +test->should_run = test->selected;
>>
>> -if ((test->run_test == NULL && test->run_serial_test == NULL) ||
>> -(test->run_test != NULL && test->run_serial_test != NULL)) {
>> +if (test->run_test && test->run_serial_test) {
>>  fprintf(stderr, "Test %d:%s must have either test_%s() 
>> or serial_test_%sl() defined.\n",
>>  test->test_num, test->test_name, 
>> test->test_name, test->test_name);
>>  exit(EXIT_ERR_SETUP_INFRA);
>>  }
>> +if (!test->run_test && !test->run_serial_test) {
>> +test->not_built = true;
>> +test->should_run = false;
>> +continue;
>> +}
>>  if (test->should_run)
>>  test->should_tmon = should_tmon(&env.tmon_selector, 
>> test->test_name);
>>  }
>> @@ -2106,9 +2134,18 @@ int main(int argc, char **argv)
>>
>>  for (i = 0; i < prog_test_cnt; i++) {
>>  struct prog_test_def *test = &prog_test_defs[i];
>> +struct test_state *state = &test_states[i];
>>
>> -if (!test->should_run)
>> +if (!test->should_run) {
>> +if (test->not_built && test->selected &&
>> +!env.get_test_cnt && !env.list_test_names) {
>> +state->tested = true;
>> +state->skip_cnt = 1;
>> +env.not_built_cnt++;
>> +print_test_result(test, state);
>> +}
>>  continue;
>> +}
>
> When test->should_run is false (which it is for unbuilt tests due to the
> code above setting test->should_run = false), the continue statement skips
> the env.get_test_cnt and env.list_test_names blocks that follow.
>
> Won't this completely omit unbuilt tests from the output of --list (-l) and
> --count modes? The code only prints unbuilt tests when !env.get_test_cnt &&
> !env.list_test_names is true.

It makes no sense to count and list tests that can't be run.

>
> If CI systems use test_progs -l to discover and distribute tests across
> parallel jobs, they won't be aware of unbuilt tests. Could this undermine
> the stated goal of keeping unbuilt tests visible in CI results?

Only if the CI in question sets BPF_STRICT_BUILD=0, which is not usually
the case and if opted-in then the user will know that some tests might not
appear and be counted.

>
> Reference: sashiko-bot raised this concern in v10 discussion at
> https://lore.kernel.org/bpf/[email protected]/
>
>>
>>  if (env.get_test_cnt) {
>>  env.succ_cnt++;
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25167006036




Re: [PATCH bpf-next v10 07/11] selftests/bpf: Skip tests whose objects were not built

2026-04-30 Thread bot+bpf-ci
> diff --git a/tools/testing/selftests/bpf/test_progs.c 
> b/tools/testing/selftests/bpf/test_progs.c
> index cc14b13e23fe..7ba82974ee78 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c

[ ... ]

> @@ -2046,15 +2069,20 @@ int main(int argc, char **argv)
>   struct prog_test_def *test = &prog_test_defs[i];
>
>   test->test_num = i + 1;
> - test->should_run = should_run(&env.test_selector,
> -   test->test_num, test->test_name);
> + test->selected = should_run(&env.test_selector,
> + test->test_num, test->test_name);
> + test->should_run = test->selected;
>
> - if ((test->run_test == NULL && test->run_serial_test == NULL) ||
> - (test->run_test != NULL && test->run_serial_test != NULL)) {
> + if (test->run_test && test->run_serial_test) {
>   fprintf(stderr, "Test %d:%s must have either test_%s() 
> or serial_test_%sl() defined.\n",
>   test->test_num, test->test_name, 
> test->test_name, test->test_name);
>   exit(EXIT_ERR_SETUP_INFRA);
>   }
> + if (!test->run_test && !test->run_serial_test) {
> + test->not_built = true;
> + test->should_run = false;
> + continue;
> + }
>   if (test->should_run)
>   test->should_tmon = should_tmon(&env.tmon_selector, 
> test->test_name);
>   }
> @@ -2106,9 +2134,18 @@ int main(int argc, char **argv)
>
>   for (i = 0; i < prog_test_cnt; i++) {
>   struct prog_test_def *test = &prog_test_defs[i];
> + struct test_state *state = &test_states[i];
>
> - if (!test->should_run)
> + if (!test->should_run) {
> + if (test->not_built && test->selected &&
> + !env.get_test_cnt && !env.list_test_names) {
> + state->tested = true;
> + state->skip_cnt = 1;
> + env.not_built_cnt++;
> + print_test_result(test, state);
> + }
>   continue;
> + }

When test->should_run is false (which it is for unbuilt tests due to the
code above setting test->should_run = false), the continue statement skips
the env.get_test_cnt and env.list_test_names blocks that follow.

Won't this completely omit unbuilt tests from the output of --list (-l) and
--count modes? The code only prints unbuilt tests when !env.get_test_cnt &&
!env.list_test_names is true.

If CI systems use test_progs -l to discover and distribute tests across
parallel jobs, they won't be aware of unbuilt tests. Could this undermine
the stated goal of keeping unbuilt tests visible in CI results?

Reference: sashiko-bot raised this concern in v10 discussion at
https://lore.kernel.org/bpf/[email protected]/

>
>   if (env.get_test_cnt) {
>   env.succ_cnt++;

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25167006036

[PATCH bpf-next v10 07/11] selftests/bpf: Skip tests whose objects were not built

2026-04-30 Thread Ricardo B . Marlière
When both run_test and run_serial_test are NULL (because the corresponding
.test.o was not compiled), mark the test as not built instead of fatally
aborting.

Report these tests as "SKIP (not built)" in per-test output and include
them in the skip count so they remain visible in CI results and JSON
output. The summary line shows the not-built count when nonzero:

  Summary: 50/55 PASSED, 5 SKIPPED (3 not built), 0 FAILED

Tests filtered out by -t/-n remain invisible as before; only genuinely
unbuilt tests are surfaced.

Signed-off-by: Ricardo B. Marlière 
---
 tools/testing/selftests/bpf/test_progs.c | 53 +++-
 tools/testing/selftests/bpf/test_progs.h |  1 +
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c 
b/tools/testing/selftests/bpf/test_progs.c
index cc14b13e23fe..7ba82974ee78 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -165,6 +165,8 @@ struct prog_test_def {
void (*run_test)(void);
void (*run_serial_test)(void);
bool should_run;
+   bool not_built;
+   bool selected;
bool need_cgroup_cleanup;
bool should_tmon;
 };
@@ -372,6 +374,8 @@ static void print_test_result(const struct prog_test_def 
*test, const struct tes
fprintf(env.stdout_saved, "#%-*d %s:", TEST_NUM_WIDTH, test->test_num, 
test->test_name);
if (test_state->error_cnt)
fprintf(env.stdout_saved, "FAIL");
+   else if (test->not_built)
+   fprintf(env.stdout_saved, "SKIP (not built)");
else if (!skipped_cnt)
fprintf(env.stdout_saved, "OK");
else if (skipped_cnt == subtests_cnt || !subtests_cnt)
@@ -1641,6 +1645,7 @@ static void calculate_summary_and_print_errors(struct 
test_env *env)
json_writer_t *w = NULL;
 
for (i = 0; i < prog_test_cnt; i++) {
+   struct prog_test_def *test = &prog_test_defs[i];
struct test_state *state = &test_states[i];
 
if (!state->tested)
@@ -1651,7 +1656,7 @@ static void calculate_summary_and_print_errors(struct 
test_env *env)
 
if (state->error_cnt)
fail_cnt++;
-   else
+   else if (!test->not_built)
succ_cnt++;
}
 
@@ -1700,8 +1705,13 @@ static void calculate_summary_and_print_errors(struct 
test_env *env)
if (env->json)
fclose(env->json);
 
-   printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
-  succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt);
+   if (env->not_built_cnt)
+   printf("Summary: %d/%d PASSED, %d SKIPPED (%d not built), %d 
FAILED\n",
+  succ_cnt, sub_succ_cnt, skip_cnt, env->not_built_cnt,
+  fail_cnt);
+   else
+   printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
+  succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt);
 
env->succ_cnt = succ_cnt;
env->sub_succ_cnt = sub_succ_cnt;
@@ -1772,6 +1782,19 @@ static void server_main(void)
run_one_test(i);
}
 
+   /* mark not-built tests as skipped */
+   for (int i = 0; i < prog_test_cnt; i++) {
+   struct prog_test_def *test = &prog_test_defs[i];
+   struct test_state *state = &test_states[i];
+
+   if (test->not_built && test->selected) {
+   state->tested = true;
+   state->skip_cnt = 1;
+   env.not_built_cnt++;
+   print_test_result(test, state);
+   }
+   }
+
/* generate summary */
fflush(stderr);
fflush(stdout);
@@ -2046,15 +2069,20 @@ int main(int argc, char **argv)
struct prog_test_def *test = &prog_test_defs[i];
 
test->test_num = i + 1;
-   test->should_run = should_run(&env.test_selector,
- test->test_num, test->test_name);
+   test->selected = should_run(&env.test_selector,
+   test->test_num, test->test_name);
+   test->should_run = test->selected;
 
-   if ((test->run_test == NULL && test->run_serial_test == NULL) ||
-   (test->run_test != NULL && test->run_serial_test != NULL)) {
+   if (test->run_test && test->run_serial_test) {
fprintf(stderr, "Test %d:%s must have either test_%s() 
or serial_test_%sl() defined.\n",
test->test_num, test->test_name, 
test->test_name, test->test_name);
exit(EXIT_ERR_SETUP_INFRA);
}
+   if (!test->run_test && !test->run_serial_test) {
+   test->not_built = true;
+   test->should_run = false;
+   contin