[PATCH v1] dts: add test case docstring checks to format script

[PATCH v1] dts: add test case docstring checks to format script

[Dean Marx]

Add a python script that checks all test cases
to ensure each docstring contains a steps and
verify section in accordance with coding guidelines.
Add a section within the check-format script which
calls the docstring script after linting with ruff.

Bugzilla ID: 1623

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 devtools/check-docstrings.py | 52 ++++++++++++++++++++++++++++++++++++
 devtools/dts-check-format.sh |  7 +++++
 2 files changed, 59 insertions(+)
 create mode 100755 devtools/check-docstrings.py

diff --git a/devtools/check-docstrings.py b/devtools/check-docstrings.py
new file mode 100755
index 0000000000..0baee6e383
--- /dev/null
+++ b/devtools/check-docstrings.py
@@ -0,0 +1,52 @@
+# 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/
+TESTS_DIR = BASE_DIR.parent / "dts" / "tests"  # 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"}:
+            return True
+    return False
+
+
+def check_file(path: Path) -> bool:
+    """Return True if file has steps and verify sections in each test case docstring."""
+    source = path.read_text(encoding="utf-8")
+    tree = parse(source, filename=str(path))
+    ok = True
+
+    for node in walk(tree):
+        if isinstance(node, FunctionDef):
+            if has_test_decorator(node):
+                doc = get_docstring(node)
+                if not doc:
+                    print(f"{path}:{node.lineno} missing docstring for test case")
+                    ok = False
+                else:
+                    if "Steps:" not in doc:
+                        print(f"{path}:{node.lineno} missing 'Steps:' section")
+                        ok = False
+                    if "Verify:" not in doc:
+                        print(f"{path}:{node.lineno} missing 'Verify:' section")
+                        ok = False
+    return ok
+
+
+def main():
+    all_ok = True
+    for path in TESTS_DIR.rglob("*.py"):
+        if not check_file(path):
+            all_ok = False
+    sys.exit(0 if all_ok else 1)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
index 907eed1293..5f93a11a16 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/check-docstrings.py"
+		python "$docstring_script"
+		errors=$((errors + $?))
+
 		git update-index --refresh
+
 		retval=$?
 		if [ $retval -ne 0 ]; then
 			echo 'The "needs update" files have been fixed by the linter.'
-- 
2.50.1

[PATCH v2] dts: add test case docstring checks to format script

[Dean Marx]

Add a python script that checks all test cases
to ensure each docstring contains a steps and
verify section in accordance with coding guidelines.
Add a section within the check-format script which
calls the docstring script after linting with ruff.

Bugzilla ID: 1623

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 devtools/dts-check-docstrings.py | 52 ++++++++++++++++++++++++++++++++
 devtools/dts-check-format.sh     |  7 +++++
 2 files changed, 59 insertions(+)
 create mode 100755 devtools/dts-check-docstrings.py

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 @@
+# 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/
+TESTS_DIR = BASE_DIR.parent / "dts" / "tests"  # 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"}:
+            return True
+    return False
+
+
+def check_file(path: Path) -> bool:
+    """Return True if file has steps and verify sections in each test case docstring."""
+    source = path.read_text(encoding="utf-8")
+    tree = parse(source, filename=str(path))
+    ok = True
+
+    for node in walk(tree):
+        if isinstance(node, FunctionDef):
+            if has_test_decorator(node):
+                doc = get_docstring(node)
+                if not doc:
+                    print(f"{path}:{node.lineno} missing docstring for test case")
+                    ok = False
+                else:
+                    if "Steps:" not in doc:
+                        print(f"{path}:{node.lineno} missing 'Steps:' section")
+                        ok = False
+                    if "Verify:" not in doc:
+                        print(f"{path}:{node.lineno} missing 'Verify:' section")
+                        ok = False
+    return ok
+
+
+def main():
+    all_ok = True
+    for path in TESTS_DIR.rglob("*.py"):
+        if not check_file(path):
+            all_ok = False
+    sys.exit(0 if all_ok else 1)
+
+
+if __name__ == "__main__":
+    main()
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 + $?))
+
 		git update-index --refresh
+
 		retval=$?
 		if [ $retval -ne 0 ]; then
 			echo 'The "needs update" files have been fixed by the linter.'
-- 
2.50.1

Re: [PATCH v2] dts: add test case docstring checks to format script

[Luca Vizzarro]

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

Re: [PATCH v2] dts: add test case docstring checks to format script

[Dean Marx]

On Wed, Aug 27, 2025 at 11:46 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> 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.

Heard

>
> > +# 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"

Thanks for catching this, I hadn't realized

>
> > +
> > +
> > +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"}

I'll add this to the new version

>
> > +            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.

Okay, I'll add a docstringcheck variable in the check-format script to
give it its own if block

[PATCH v3] dts: add test case docstring checks to format script

[Dean Marx]

Add a python script that checks all test cases
to ensure each docstring contains a steps and
verify section in accordance with coding guidelines.
Add a section within the check-format script which
calls the docstring script after linting with ruff.

Bugzilla ID: 1623

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 devtools/dts-check-docstrings.py | 54 ++++++++++++++++++++++++++++++++
 devtools/dts-check-format.sh     |  8 +++++
 2 files changed, 62 insertions(+)
 create mode 100755 devtools/dts-check-docstrings.py

diff --git a/devtools/dts-check-docstrings.py b/devtools/dts-check-docstrings.py
new file mode 100755
index 0000000000..3b0d2a42a1
--- /dev/null
+++ b/devtools/dts-check-docstrings.py
@@ -0,0 +1,54 @@
+#!/usr/bin/env python3
+# 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.parent  # dpdk/
+TESTS_DIR = BASE_DIR / "dts" / "tests"  # dts/tests/
+DECORATOR_NAMES = {"func_test", "perf_test"}
+
+
+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 DECORATOR_NAMES:
+            return True
+    return False
+
+
+def check_file(path: Path) -> bool:
+    """Return True if file has steps and verify sections in each test case docstring."""
+    source = path.read_text(encoding="utf-8")
+    tree = parse(source, filename=str(path))
+    ok = True
+
+    for node in walk(tree):
+        if isinstance(node, FunctionDef):
+            if has_test_decorator(node):
+                doc = get_docstring(node)
+                if not doc:
+                    print(f"{path}:{node.lineno} missing docstring for test case")
+                    ok = False
+                else:
+                    if "Steps:" not in doc:
+                        print(f"{path}:{node.lineno} missing 'Steps:' section")
+                        ok = False
+                    if "Verify:" not in doc:
+                        print(f"{path}:{node.lineno} missing 'Verify:' section")
+                        ok = False
+    return ok
+
+
+def main():
+    all_ok = True
+    for path in TESTS_DIR.rglob("*.py"):
+        if not check_file(path):
+            all_ok = False
+    sys.exit(0 if all_ok else 1)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
index 907eed1293..f2cd5a56a1 100755
--- a/devtools/dts-check-format.sh
+++ b/devtools/dts-check-format.sh
@@ -13,6 +13,7 @@ usage() {
 format=true
 lint=true
 typecheck=true
+docstringcheck=true
 
 # Comments after args serve as documentation; must be present
 while getopts "hflt" arg; do
@@ -97,6 +98,13 @@ if $lint; then
 		echo "ruff not found, unable to run linter"
 		errors=$((errors + 1))
 	fi
+	if $docstringcheck; then
+		docstring_script_path=$(dirname "$0")
+		docstring_script_path=$(cd "$docstring_script_path" && pwd)
+		docstring_script="$docstring_script_path/dts-check-docstrings.py"
+		$docstring_script
+		errors=$((errors + $?))
+	fi
 fi
 
 if $typecheck; then
-- 
2.51.0