Re: [PATCH v3 4/4] perf tools: Add test for weight annotation
| 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