Re: [RFC PATCH 2/5] perf/tests: convert check_all_patterns_found.pl to Python

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf/tests: check_all_patterns_found.py added without updating callers
--

commit 069a51b64d65753bb41f5e2b0b33fd3d38c715f2
Author: Breno Rodrigues Alves <breno3011alves@gmail.com>

perf/tests: convert check_all_patterns_found.pl to Python

This commit adds a Python 3 implementation of check_all_patterns_found.pl
under tools/perf/tests/shell/common/check_all_patterns_found.py.

>     perf/tests: convert check_all_patterns_found.pl to Python
>     
>     Faithful re-implementation of tools/perf/tests/shell/common/
>     check_all_patterns_found.pl in Python 3 (stdlib only). Same
>     regex matching, same TESTLOG_VERBOSITY/TESTLOG_ERR_MSG_MAX_LINES
>     environment handling, same exit codes.
>     
>     Verified byte-for-byte identical to the Perl version.
>     
>     Signed-off-by: Breno Rodrigues Alves <breno3011alves@gmail.com>

> diff --git a/tools/perf/tests/shell/common/check_all_patterns_found.py b/tools/perf/tests/shell/common/check_all_patterns_found.py
> new file mode 100755
> index 0000000000000..3eb907af3d6b2
> --- /dev/null
> +++ b/tools/perf/tests/shell/common/check_all_patterns_found.py
> @@ -0,0 +1,35 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +import os
> +import re
> +import sys
> +
> +
> +def main(argv):
> +    quiet = True
> +    try:
> +        if int(os.environ.get("TESTLOG_VERBOSITY", "0")) >= 2:
> +            quiet = False
> +    except ValueError:
> +        pass

[ ... ]

> +if __name__ == "__main__":
> +    sys.exit(main(sys.argv[1:]))
> \ No newline at end of file

[Severity: Medium]
Should the callers, tools/perf/Makefile.perf, and the old Perl script be
updated as part of this conversion?

This commit adds tools/perf/tests/shell/common/check_all_patterns_found.py,
but tools/perf/tests/shell/common/check_all_patterns_found.pl is not
removed, and none of the subsequent patches in this series update the
callers or build system.

All 37 call sites across the 6 test scripts in
tools/perf/tests/shell/base_probe/ (test_adding_blacklisted.sh,
test_adding_kernel.sh, test_basic.sh, test_invalid_options.sh) and
tools/perf/tests/shell/base_report/ (setup.sh, test_basic.sh) still invoke
the Perl script directly:

    "$DIR_PATH/../common/check_all_patterns_found.pl" \

Also, the install-tests target in tools/perf/Makefile.perf only installs
.sh and .pl files from tests/shell/common/:

    $(INSTALL) tests/shell/common/*.sh '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell/common'; \
    $(INSTALL) tests/shell/common/*.pl '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell/common'; \

Without updating the test scripts and Makefile.perf, the new Python script
is unused and is not installed by make install-tests.

Additionally, the commit message mentions TESTLOG_ERR_MSG_MAX_LINES
handling, which is not present in either check_all_patterns_found.pl or
check_all_patterns_found.py, and the new file is missing a trailing
newline.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018125641.23064-1-breno3011alves@gmail.com?part=2