Re: [PATCH v3 4/4] perf tools: Add test for weight annotation

[email protected]
Newsgroups org.kernel.vger.linux-perf-users
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Brittle and silent test assertion due to unisolated `PERF_CONFIG` environment.
--

--- Patch [4]: [PATCH v3 4/4] perf tools: Add test for weight annotation ---
commit ae0853531b9364f64586279c41d3ca0652fe3e71
Author: Andi Kleen <[email protected]>

perf tools: Add test for weight annotation

This commit adds a new shell test case to verify that --stdio weight
annotation works correctly. It uses perf record to capture a precise PEBS
event and then tests the perf report and perf annotate tools.

> diff --git a/tools/perf/tests/shell/annotate_weight.sh b/tools/perf/tests/shell/annotate_weight.sh
> new file mode 100755
> --- /dev/null
> +++ b/tools/perf/tests/shell/annotate_weight.sh

[ ... ]

> +perf annotate --stdio -i "${perfdata}" --symbol "${testsym}" \
> +	> "${annotate_out}" 2> "${record_log}"
> +
> +grep -q 'Percent Weight' "${annotate_out}"

[Severity: Medium]
Will this test fail silently for users with a customized ~/.perfconfig?

Because set -e is active at the top of the script, if a user has settings
like annotate.show_nr_samples=true or annotate.show_total_period=true,
perf annotate might change the column header from "Percent Weight" to
something like "Samples Weight" or "Period Weight".

This would cause this standalone grep command to fail and immediately
terminate the test without providing any diagnostics. Does this test need to
run in an isolated environment or parse the output more flexibly?

> +
> +# The second numeric column is the rendered average weight. This assertion
> +# fails on the original regression because calc_percent() left it at zero.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.