> 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/[email protected]?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
