[PATCH v2] dts: add test case docstring checks to format script
Luca Vizzarro
luca.vizzarro at arm.com
Wed Aug 27 17:46:20 CEST 2025
Hi Dean,
thank you for your patch! Looks mostly good, a couple of comments
though.
On Tue, Aug 26, 2025 at 04:07:20PM +0000, Dean Marx wrote:
> diff --git a/devtools/dts-check-docstrings.py b/devtools/dts-check-docstrings.py
> new file mode 100755
> index 0000000000..0baee6e383
> --- /dev/null
> +++ b/devtools/dts-check-docstrings.py
> @@ -0,0 +1,52 @@
Consider adding a shebang here, like it's already done with the other
scripts.
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2025 University of New Hampshire
> +
> +import sys
> +from ast import FunctionDef, Name, walk, get_docstring, parse
> +from pathlib import Path
> +
> +BASE_DIR = Path(__file__).resolve().parent # dpdk/
The hint is actually wrong, this is dpdk/devtools. I think you meant to
make this:
BASE_DIR = Path(__file__).resolve().parent.parent
> +TESTS_DIR = BASE_DIR.parent / "dts" / "tests" # dts/tests/
and this would just be:
TESTS_DIR = BASE_DIR / "dts" / "tests"
> +
> +
> +def has_test_decorator(node: FunctionDef) -> bool:
> + """Return True if function has @func_test or @perf_test decorator."""
> + for decorator in node.decorator_list:
> + if isinstance(decorator, Name) and decorator.id in {"func_test", "perf_test"}:
I wouldn't hard code the decorator names like this. Put this in a
constant at the top instead:
DECORATOR_NAMES = {"func_test", "perf_test"}
> + return True
> + return False
<snip>
> diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
> index 907eed1293..da6e6f34ee 100755
> --- a/devtools/dts-check-format.sh
> +++ b/devtools/dts-check-format.sh
> @@ -86,7 +86,14 @@ if $lint; then
> ruff check --fix
> errors=$((errors + $?))
>
> + docstring_script_path=$(dirname "$0")
> + docstring_script_path=$(cd "$docstring_script_path" && pwd)
> + docstring_script="$docstring_script_path/dts-check-docstrings.py"
> + python "$docstring_script"
> + errors=$((errors + $?))
> +
I wouldn't insert this with the ruff if scope. This can still stay under
the lint if block, but give it its own.
> git update-index --refresh
> +
Best regards,
Luca
More information about the dev
mailing list