> selftests/bpf: Skip compare_text subtest when stdout is not saved
TL;DR: Nit only: the new test__skip() in the compare_text_mismatch subtest
gives no reason for skipping; a short comment or stderr message might help.
The skip logic itself looks correct.
> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> index b4cd7c7ff439..ddb6cdc1579f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> +++ b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
[ ... ]
> @@ -203,14 +203,18 @@ void test_prog_tests_framework_compare_text(void)
> }
>
> if (test__start_subtest("compare_text_mismatch")) {
> - err = compare_text_to_expected("line two\n", "line one\n");
> - fflush(stdout);
> + if (!env.subtest_state->stdout_saved) {
> + test__skip();
> + } else {
> + err = compare_text_to_expected("line two\n", "line
> one\n");
> + fflush(stdout);
>
> - ASSERT_EQ(err, -1, "mismatch_rc");
> - ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "-line one",
> - "diff_has_expected");
> - ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "+line two",
> - "diff_has_actual");
> + ASSERT_EQ(err, -1, "mismatch_rc");
> + ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "-line
> one",
> + "diff_has_expected");
> + ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "+line
> two",
> + "diff_has_actual");
> + }
> test__end_subtest();
> }
> }
This isn't a bug, but could the skip in the compare_text_mismatch subtest
carry an explanation?
if (!env.subtest_state->stdout_saved) {
test__skip();
}
A subsystem guideline flags selftests that skip silently with no message,
and test__skip() here prints nothing about why the subtest was skipped.
The same condition covers -v in non-parallel mode, a libc without
open_memstream() support, and an open_memstream() failure, so the reason is
not obvious from the test output alone.
For context, the skip itself looks correct: env.subtest_state is non-NULL
inside the subtest, test__skip() sets subtest_state->skipped, and
test__end_subtest() handles a NULL stdout_saved.
The other skips in this file are also bare test__skip() calls, and
test__skip() takes no reason argument, so the guideline may not map cleanly
onto test_progs. If a reason is wanted, would a short comment above the
test__skip() call, or a separate message printed to env.stderr_saved, be
appropriate?
---
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/38559567568
