Re: [PATCH v3 3/3] doc: revise coding guidelines section

[Patrick Robb <probb@iol.unh.edu>]

[-- Attachment #1: Type: text/plain, Size: 13967 bytes --]

On Fri, Jul 11, 2025 at 1:25 PM Dean Marx <dmarx@iol.unh.edu> wrote:

> The Framework Coding Guidelines section includes outdated information
> about DTS and how to write a test suite. Updated these points to include
> the new test case decorators and setup/teardown hooks.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  doc/guides/tools/dts.rst | 181 ++++++++++++++++++---------------------
>  1 file changed, 84 insertions(+), 97 deletions(-)
>
> diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> index 7f6f7c1db5..6af9f8b75c 100644
> --- a/doc/guides/tools/dts.rst
> +++ b/doc/guides/tools/dts.rst
> @@ -348,122 +348,109 @@ Adding test cases may require adding code to the
> framework as well.
>  Framework Coding Guidelines
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> -When adding code to the DTS framework, pay attention to the rest of the
> code
> -and try not to divert much from it.
> -The :ref:`DTS developer tools <dts_dev_tools>` will issue warnings
> -when some of the basics are not met.
> -You should also build the :ref:`API documentation <building_api_docs>`
> -to address any issues found during the build.
> -
> -The API documentation, which is a helpful reference when developing, may
> be accessed
> -in the code directly or generated with the :ref:`API docs build steps
> <building_api_docs>`.
> -When adding new files or modifying the directory structure,
> -the corresponding changes must be made to DTS API doc sources in
> ``doc/api/dts``.
> -
> -Speaking of which, the code must be properly documented with docstrings.
> -The style must conform to the `Google style
> -<
> https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings
> >`_.
> -See an example of the style `here
> -<
> https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html
> >`_.
> -For cases which are not covered by the Google style, refer to `PEP 257
> -<https://peps.python.org/pep-0257/>`_.
> -There are some cases which are not covered by the two style guides,
> -where we deviate or where some additional clarification is helpful:
> -
> -   * The ``__init__()`` methods of classes are documented separately
> -     from the docstring of the class itself.
> -   * The docstrings of implemented abstract methods should refer to the
> superclass's definition
> -     if there's no deviation.
> -   * Instance variables/attributes should be documented in the docstring
> of the class
> -     in the ``Attributes:`` section.
> -   * The ``dataclass.dataclass`` decorator changes how the attributes are
> processed.
> -     The dataclass attributes which result in instance
> variables/attributes
> -     should also be recorded in the ``Attributes:`` section.
> -   * Class variables/attributes and Pydantic model fields, on the other
> hand,
> -     should be documented with ``#:`` above the type annotated line.
> -     The description may be omitted if the meaning is obvious.
> -   * The ``Enum`` and ``TypedDict`` also process the attributes in
> particular ways
> -     and should be documented with ``#:`` as well.
> -     This is mainly so that the autogenerated documentation contains the
> assigned value.
> -   * When referencing a parameter of a function or a method in their
> docstring,
> -     don't use any articles and put the parameter into single backticks.
> -     This mimics the style of `Python's documentation <
> https://docs.python.org/3/index.html>`_.
> -   * When specifying a value, use double backticks::
> -
> -        def foo(greet: bool) -> None:
> -            """Demonstration of single and double backticks.
> -
> -            `greet` controls whether ``Hello World`` is printed.
> -
> -            Args:
> -               greet: Whether to print the ``Hello World`` message.
> -            """
> -            if greet:
> -               print(f"Hello World")
> -
> -   * The docstring maximum line length is the same as the code maximum
> line length.
> +When contributing code to the DTS framework, follow existing conventions
> to ensure consistency.
> +The :ref:`DTS developer tools <dts_dev_tools>` will flag basic issues.
> +Also, be sure to :ref:`build the API documentation <building_api_docs>`
> to catch any problems during the build.
> +
> +The API documentation is a helpful reference during development.
> +It can be viewed in the code directly or generated using the :ref:`API
> docs build steps <building_api_docs>`.
> +If you add new files or change the directory structure, update the
> corresponding sources in ``doc/api/dts``.
> +
> +Code must be documented with docstrings that follow the
> +`Google style <
> https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings
> >`_.
> +Additional references:
> +
> +* `Sphinx Google style example <
> https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html
> >`_
> +* `PEP 257 <https://peps.python.org/pep-0257/>`_
> +
> +Docstring and Attribute Guidelines
> +
> +* Document ``__init__()`` separately from the class docstring.
> +* If an abstract method simply implements a superclass definition without
> changes, refer to that superclass in the docstring.
> +* Document instance variables in the class docstring under an
> ``Attributes:`` section.
> +* For ``@dataclass`` classes, document instance-level attributes in
> ``Attributes:``, as they are generated from the class fields.
> +* Document class variables and Pydantic fields using ``#:``,
> +   placed above the type-annotated line. Descriptions may be omitted if
> the meaning is clear.
> +* Apply ``#:`` to ``Enum`` and ``TypedDict`` fields as well, so that
> autogenerated documentation includes their values.
> +* When referring to a parameter in a docstring, omit articles and enclose
> the parameter in single backticks (e.g., `` `param` ``),
> +   consistent with the `Python documentation style <
> https://docs.python.org/3/index.html>`_.
> +* Use double backticks (````value````) for literal values.
> +
> +Example::
> +
> +   def foo(greet: bool) -> None:
> +       """Demonstrates single vs. double backticks.
>

single AND double


> +
> +       `greet` controls whether ``Hello World`` is printed.
> +
> +       Args:
> +           greet: Whether to print the ``Hello World`` message.
> +       """
> +       if greet:
> +           print("Hello World")
> +
> +The maximum line length for docstrings must match that of the code.
>
>
>  How To Write a Test Suite
>  -------------------------
>
> -All test suites inherit from ``TestSuite`` defined in
> ``dts/framework/test_suite.py``.
> -There are four types of methods that comprise a test suite:
> +All test suites are classes that inherit from TestSuite, defined in
> dts/framework/test_suite.py. A typical suite contains:
> +
> +Test Cases
> +
> +   Test cases are defined as methods and must be decorated appropriately.
> +   Use the @func_test and/or @perf_test decorators from TestSuite above
> each test case method.
> +   For example: @func_test followed by def test_basic_link(self):
>

I think you should either not attempt to provide a literal example, or, if
you are going to do so, then show it in normal syntax i.e.:

@func_test
def test_basic_link(self):
   """your testcase docstring here"""
   #your testcase code here


> +
> +   Functional test cases should use the @func_test decorator, and
> performance test cases should use @perf_test.
> +   A test suite may include any number of functional and/or performance
> test cases.
> +   Each suite should focus on testing a single feature (one feature = one
> test suite).
> +   If a feature requires extensive testing scenarios, it's better to
> split the test cases across multiple test suites to reduce execution time.
>

Can you remove this last line about execution time? It doesn't sound true
to me (the execution time will be the same).


> +
> +Setup and Teardown Hooks
> +
> +   Setup and teardown methods can be defined at both the suite and test
> case levels.
> +
> +   Suite-level:
> +
> +   * set_up_suite() — runs once before any test cases in the suite
> +
> +   * tear_down_suite() — runs once after all test cases have completed
>
> -#. **Test cases**
> +   Case-level:
>
> -   | Test cases are methods that start with a particular prefix.
> -   | Functional test cases start with ``test_``, e.g.
> ``test_hello_world_single_core``.
> -   | Performance test cases start with ``test_perf_``, e.g.
> ``test_perf_nic_single_core``.
> -   | A test suite may have any number of functional and/or performance
> test cases.
> -     However, these test cases must test the same feature,
> -     following the rule of one feature = one test suite.
> -     Test cases for one feature don't need to be grouped in just one test
> suite, though.
> -     If the feature requires many testing scenarios to cover,
> -     the test cases would be better off spread over multiple test suites
> -     so that each test suite doesn't take too long to execute.
> +   * set_up_test_case() — runs before each individual test case
>
> -#. **Setup and Teardown methods**
> +   * tear_down_test_case() — runs after each individual test case
>
> -   | There are setup and teardown methods for the whole test suite and
> each individual test case.
> -   | Methods ``set_up_suite`` and ``tear_down_suite`` will be executed
> -     before any and after all test cases have been executed, respectively.
> -   | Methods ``set_up_test_case`` and ``tear_down_test_case`` will be
> executed
> -     before and after each test case, respectively.
> -   | These methods don't need to be implemented if there's no need for
> them in a test suite.
> -     In that case, nothing will happen when they are executed.
> +   These methods are optional. If not implemented, the framework will
> simply skip them.
>
> -#. **Configuration, traffic and other logic**
> +Configuration, Traffic, and Other Logic
>

Your section below does not say anything about configuration, so remove
configuration from the header above. Also, it's unclear (to people who
dont write/read lots of DTS) that "Traffic" is encompassed by the TestSuite
class methods (like send_packet_and_capture).


>
> -   The ``TestSuite`` class contains a variety of methods for anything that
> -   a test suite setup, a teardown, or a test case may need to do.
> +   The TestSuite class provides a variety of methods for setup, teardown,
> and test logic.
> +   Test suites often use DPDK applications (e.g., testpmd) in interactive
> mode and interact with them via shell instances.
>
> -   The test suites also frequently use a DPDK app, such as testpmd, in
> interactive mode
> -   and use the interactive shell instances directly.
> +Framework logic should be used in one of two ways:
>

I think what this section is trying to say is that there is an API surface
for the testsuite writer, and that is the TestSuite classes and DPDK apps.


>
> -   These are the two main ways to call the framework logic in test suites.
> -   If there's any functionality or logic missing from the framework,
> -   it should be implemented so that the test suites can use one of these
> two ways.
> +* Using built-in methods provided by TestSuite or its base classes
>
> -   Test suites may also be configured individually using a file provided
> at the command line.
> -   The file is a simple mapping of test suite names to their
> corresponding configurations.
> +* Interacting directly with tools or shell interfaces
>
> -   Any test suite can be designed to require custom configuration
> attributes or optional ones.
> -   Any optional attributes should supply a default value for the test
> suite to use.
> +If any required functionality is missing, it should be implemented in a
> way that supports one of these two approaches.
>
>
Should this be indented since it is under the section "Framework logic
should be used in one of two ways:" (although that section name should be
renamed per the earlier comment).


> -#. **Test case verification**
> +Test Case Verification
>
> -   Test case verification should be done with the ``verify`` method,
> which records the result.
> -   The method should be called at the end of each test case.
> +   Use the verify method to assert conditions and record test results.
> +   This should typically be called at the end of each test case.
> +   Example: self.verify(link_up, "Link should be up after configuration.")
>
> -#. **Other methods**
> +Other Methods
>
> -   Of course, all test suite code should adhere to coding standards.
> -   Only the above methods will be treated specially and any other methods
> may be defined
> -   (which should be mostly private methods needed by each particular test
> suite).
> -   Any specific features (such as NIC configuration) required by a test
> suite
> -   should be implemented in the ``SutNode`` class (and the underlying
> classes that ``SutNode`` uses)
> -   and used by the test suite via the ``sut_node`` field.
> +   All test suite code should follow the project's coding standards.
> +   Only test cases, setup/teardown hooks, and verification methods are
> treated specially by the framework.
> +   Additional methods may be defined as needed (ideally private).
> +   Any specific features (such as NIC configuration) should be
> implemented in the SutNode class or its supporting classes, and accessed
> using the sut_node field.
>

There's no such class as SutNode anymore.


>
>
>  .. _dts_dev_tools:
> --
> 2.49.0
>
>
Also, this series should adjust the poetry section to explain that "poetry
shell" is deprecated now and one must use "poetry run" instead.

Reviewed-by: Patrick Robb <probb@iol.unh.edu>

[-- Attachment #2: Type: text/html, Size: 17279 bytes --]