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

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Unbuilt tests bypass test filters (e.g., `-t`, `-n`) and are always printed and counted as skipped, breaking test isolation and contradicting the commit message.
--

commit 6ad669628925e7e8627a5068e116b6bbdf0f87d0
Author: Ricardo B. Marlière <rbm@suse.com>

selftests/bpf: Skip tests whose objects were not built

When tests are not compiled, this commit marks them as not built instead
of fatally aborting. It surfaces these unbuilt tests as skipped in the
output so they remain visible in CI results.

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

Does the implementation actually keep filtered tests invisible? Looking at
how the flags are set during initialization, it appears unbuilt tests
might bypass the test selector:

> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> --- 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;
> +		}

Since test->not_built is set to true regardless of what should_run() returned
for env.test_selector, won't every unbuilt test be flagged as not_built even
if the user filtered it out?

[ ... ]

> @@ -1772,6 +1780,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) {
> +			state->tested = true;
> +			state->skip_cnt = 1;
> +			env.not_built_cnt++;
> +			print_test_result(test, state);
> +		}
> +	}

Will this unconditionally print every unbuilt test in the test suite when
running in parallel mode, directly contradicting the commit message?

[ ... ]

> @@ -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;
> +		}

Could a similar issue occur here in sequential mode? If a user runs
"./test_progs -t my_test", wouldn't this logic still print "SKIP (not built)"
for all the other unbuilt tests that were supposed to be ignored?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260415-selftests-bpf_misconfig-v5-0-03d0a52a898a@suse.com?part=10