Re: [PATCH bpf-next v2 7/7] selftests/bpf: Skip compare_text subtest when stdout is not saved

[KaFai Wan <kafai.wan@linux.dev>]

On Wed, 2026-10-14 at 12:08 +0000, bot+bpf-ci@kernel.org wrote:
> > 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?
> 
just one line for this, we can print why the test skipped. and short comment for patch#6
> 
> ---
> 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

-- 
Thanks,
KaFai