Re: [PATCH v3 3/3] perf tests: Add NVMe PMU event parsing test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf tests: suite__nvme_pmu has NULL test_cases when libnvme is
  disabled
--

commit 606f3f12b454f7dd2fdaeea08ae54a0e801abbdb
Author: Ian Rogers <irogers@google.com>

perf tests: Add NVMe PMU event parsing test

This commit adds a unit test suite for the NVMe tool PMU event parser and
registers a helper to create a test NVMe PMU.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 0b8897fd42cac..fbcdcbf92aded 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -110,6 +110,7 @@ static struct test_suite *generic_tests[] = {
>  	&suite__pmu,
>  	&suite__pmu_events,
>  	&suite__hwmon_pmu,
> +	&suite__nvme_pmu,
>  	&suite__tool_pmu,
>  	&suite__dso_data,
>  	&suite__perf_evsel__roundtrip_name_test,
> diff --git a/tools/perf/tests/nvme_pmu.c b/tools/perf/tests/nvme_pmu.c
> new file mode 100644
> index 0000000000000..c3c17ccc9ea0b
> --- /dev/null
> +++ b/tools/perf/tests/nvme_pmu.c

[ ... ]

> +#else
> +
> +struct test_suite suite__nvme_pmu = {
> +	.desc = "NVMe PMU",
> +	.test_cases = NULL,
> +};
> +
> +#endif

[Severity: Low]
When HAVE_LIBNVME_SUPPORT is not defined, does setting
suite__nvme_pmu.test_cases to NULL cause __cmd_test() to silently drop the
suite during execution?

generic_tests[] registers suite__nvme_pmu unconditionally, so perf test
list assigns it a suite number, but __cmd_test() only invokes start_test()
and print_test_result() inside test_suite__for_each_test_case(), which
runs zero iterations when test_cases is NULL.  On builds without libnvme,
perf test skips over that suite number without printing a Skip line,
counting it in the skipped summary, or emitting it in JUnit output.
Providing a stub test case that returns TEST_SKIP with a TEST_CASE_REASON
of "not compiled in" (like tools/perf/tests/pfm.c), or guarding
&suite__nvme_pmu in generic_tests[] with HAVE_LIBNVME_SUPPORT, would keep
the suite list and test runner in sync.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113191234.2509964-1-irogers@google.com?part=3