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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent failures on intermediate commands cause test logs to be deleted without error output.
--

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

perf tools: Add test for weight annotation

This commit adds a shell script test case to verify if the --stdio weight
annotation functions correctly.

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

[ ... ]

> +if ! awk '$1 ~ /^[0-9]/ && ($1 + 0) > 0 { found = 1 } END { exit !found }' \
> +	"${report_out}"; then
> +	echo "[SKIP] PEBS weight sampling produced no nonzero weights"
> +	exit 2
> +fi
> +
> +perf annotate --stdio -i "${perfdata}" --symbol "${testsym}" \
> +	> "${annotate_out}" 2> "${record_log}"
> +
> +grep -q 'Percent Weight' "${annotate_out}"

[Severity: Medium]
Because the script runs under set -e, will a failure in perf annotate or
grep cause the script to exit immediately and trigger the cleanup trap?

If the script aborts here, it appears it will delete the log files without
printing any error output, which might mask the failure reason and make
debugging difficult.

> +
> +# The second numeric column is the rendered average weight. This assertion
> +# fails on the original regression because calc_percent() left it at zero.
> +if ! awk '$1 ~ /^[0-9]/ && $2 ~ /^[0-9]/ && ($2 + 0) > 0 { found = 1 } END { exit !found }' \
> +	"${annotate_out}"; then
> +	echo "Annotation output contained no nonzero weight"

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4