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

[bot+bpf-ci]

> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 7fe16b5131b1..759e76d76c57 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c

[ ... ]

> @@ -2049,12 +2070,16 @@ int main(int argc, char **argv)
>  		test->should_run = should_run(&env.test_selector,
>  					      test->test_num, test->test_name);
>
> +		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;
> +		}

This sets not_built unconditionally, regardless of what should_run()
returned.  The commit message says:

  "Tests filtered out by -t/-n remain invisible as before"

but doesn't this do the opposite?  When should_run() already returned
false because of -t/-n filtering, the test still gets not_built = true.

> @@ -2106,9 +2131,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 && !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;
> +		}

Then here, every not-built test is printed and counted as skipped,
even ones the user excluded with -t or -n.  Running something like
"./test_progs -t my_test" on a partial build would print
"SKIP (not built)" for all unbuilt tests, and inflate both succ_cnt
and skip_cnt in calculate_summary_and_print_errors().

The same issue applies to the server_main() loop:

> @@ -1772,6 +1780,19 @@ static void server_main(void)
> +	/* 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) {

This also has no check against the original test selector, so in
parallel mode with -t, all not-built tests are surfaced as well.

Would it make sense to preserve the original should_run() result and
only set not_built when the test was actually selected?

This was also flagged in an earlier review of v5:
https://sashiko.dev/#/patchset/20260415-selftests-bpf_misconfig-v5-0-03d0a52a898a@xxxxxxxx?part=10


---
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/24478691588