Re: [PATCH v1 2/3] dts: rewrite dts rst

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

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

On Tue, May 27, 2025 at 11:37 AM Dean Marx <dmarx@iol.unh.edu> wrote:

> Modify dts.rst to exclude redundant/outdated information about the project,
> and add new information regarding setup and framework design.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  doc/guides/tools/dts.rst | 310 +++++++++++++--------------------------
>  1 file changed, 99 insertions(+), 211 deletions(-)
>
> diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> index fcc6d22036..0aa6663b9f 100644
> --- a/doc/guides/tools/dts.rst
> +++ b/doc/guides/tools/dts.rst
> @@ -1,6 +1,7 @@
>  ..  SPDX-License-Identifier: BSD-3-Clause
>      Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
>      Copyright(c) 2024 Arm Limited
> +    Copyright(c) 2025 University of New Hampshire
>
>  DPDK Test Suite
>  ===============
> @@ -20,31 +21,18 @@ DTS runtime environment
>
>  DTS runtime environment node
>    A node where at least one DTS runtime environment is present.
> -  This is the node where we run DTS and from which DTS connects to other
> nodes.
>

Leave this in.


>
>  System under test
> -  An SUT is the combination of DPDK and the hardware we're testing
> -  in conjunction with DPDK (NICs, crypto and other devices).
> +  Node with DPDK and relevant hardware (NICs, crypto, etc.).
>

Maybe change to "The system which runs a DPDK application on relevant
hardware (NIC, accelerator cards, etc) and from which the DPDK behavior is
observed for tests."


>
>  System under test node
>    A node where at least one SUT is present.
>
>  Traffic generator
> -  A TG is either software or hardware capable of sending packets.
> +  Node that sends traffic; can be hardware or software-based.
>

"Node that sends traffic to the SUT;"

Sorry for being so particular. :)


>  Traffic generator node
>    A node where at least one TG is present.
> -  In case of hardware traffic generators, the TG and the node are
> literally the same.
> -
> -
> -In most cases, interchangeably referring to a runtime environment, SUT,
> TG or the node
> -they're running on (e.g. using SUT and SUT node interchangeably) doesn't
> cause confusion.
> -There could theoretically be more than of these running on the same node
> and in that case
> -it's useful to have stricter definitions.
> -An example would be two different traffic generators (such as Trex and
> Scapy)
> -running on the same node.
> -A different example would be a node containing both a DTS runtime
> environment
> -and a traffic generator, in which case it's both a DTS runtime
> environment node and a TG node.
>
>
>  DTS Environment
> @@ -195,12 +183,28 @@ These need to be set up on a Traffic Generator Node:
>  Running DTS
>  -----------
>
> -DTS needs to know which nodes to connect to and what hardware to use on
> those nodes.
> -Once that's configured, either a DPDK source code tarball or tree folder
> -need to be supplied whether these are on your DTS host machine or the SUT
> node.
> -DTS can accept a pre-compiled build placed in a subdirectory,
> -or it will compile DPDK on the SUT node,
> -and then run the tests with the newly built binaries.
> +To run DTS, use ``main.py`` with Poetry:
> +
> +.. code-block:: console
> +
> +   ```shell
> +   docker build --target dev -t dpdk-dts .
> +   docker run -v $(pwd)/..:/dpdk -v /home/dtsuser/.ssh:/root/.ssh:ro -it
> dpdk-dts bash
> +   $ poetry install
> +   $ poetry run ./main.py
> +   ```
> +
> +Common options include:
> +
> +- ``--output-dir``: Custom output location.
> +- ``--remote-source``: Use sources stored on the SUT.
> +- ``--tarball``: Specify the tarball to be tested.
> +
> +For a full list:
> +
> +.. code-block:: console
> +
> +   poetry run ./main.py --help
>

I think we should keep the full list of flags here instead of removing it
for this subset. It's a bit of a maintenance burden and it make the file
longer but it's important info. I think it's good to present it here even
if it is only "a --help away."


>
>
>  Configuring DTS
> @@ -220,71 +224,6 @@ The user must have :ref:`administrator privileges
> <sut_admin_user>`
>  which don't require password authentication.
>
>
> -DTS Execution
> -~~~~~~~~~~~~~
> -
> -DTS is run with ``main.py`` located in the ``dts`` directory after
> entering Poetry shell:
> -
> -.. code-block:: console
> -
> -   (dts-py3.10) $ ./main.py --help
> -   usage: main.py [-h] [--test-run-config-file FILE_PATH]
> [--nodes-config-file FILE_PATH] [--tests-config-file FILE_PATH]
> -                  [--output-dir DIR_PATH] [-t SECONDS] [-v] [--dpdk-tree
> DIR_PATH | --tarball FILE_PATH] [--remote-source]
> -                  [--precompiled-build-dir DIR_NAME] [--compile-timeout
> SECONDS] [--test-suite TEST_SUITE [TEST_CASES ...]]
> -                  [--re-run N_TIMES] [--random-seed NUMBER]
> -
> -   Run DPDK test suites. All options may be specified with the
> environment variables provided in brackets. Command line arguments have
> higher
> -   priority.
> -
> -   options:
> -     -h, --help            show this help message and exit
> -     --test-run-config-file FILE_PATH
> -                           [DTS_TEST_RUN_CFG_FILE] The configuration file
> that describes the test cases and DPDK build options. (default:
> test-run.conf.yaml)
> -     --nodes-config-file FILE_PATH
> -                           [DTS_NODES_CFG_FILE] The configuration file
> that describes the SUT and TG nodes. (default: nodes.conf.yaml)
> -     --tests-config-file FILE_PATH
> -                           [DTS_TESTS_CFG_FILE] Configuration file used
> to override variable values inside specific test suites. (default: None)
> -     --output-dir DIR_PATH, --output DIR_PATH
> -                           [DTS_OUTPUT_DIR] Output directory where DTS
> logs and results are saved. (default: output)
> -     -t SECONDS, --timeout SECONDS
> -                           [DTS_TIMEOUT] The default timeout for all DTS
> operations except for compiling DPDK. (default: 15)
> -     -v, --verbose         [DTS_VERBOSE] Specify to enable verbose
> output, logging all messages to the console. (default: False)
> -     --compile-timeout SECONDS
> -                           [DTS_COMPILE_TIMEOUT] The timeout for
> compiling DPDK. (default: 1200)
> -     --test-suite TEST_SUITE [TEST_CASES ...]
> -                           [DTS_TEST_SUITES] A list containing a test
> suite with test cases. The first parameter is the test suite name, and
> -                           the rest are test case names, which are
> optional. May be specified multiple times. To specify multiple test suites
> -                           in the environment variable, join the lists
> with a comma. Examples: --test-suite suite case case --test-suite
> -                           suite case ... | DTS_TEST_SUITES='suite case
> case, suite case, ...' | --test-suite suite --test-suite suite case
> -                           ... | DTS_TEST_SUITES='suite, suite case, ...'
> (default: [])
> -     --re-run N_TIMES, --re_run N_TIMES
> -                           [DTS_RERUN] Re-run each test case the
> specified number of times if a test failure occurs. (default: 0)
> -     --random-seed NUMBER  [DTS_RANDOM_SEED] The seed to use with the
> pseudo-random generator. If not specified, the configuration value is
> -                           used instead. If that's also not specified, a
> random seed is generated. (default: None)
> -
> -   DPDK Build Options:
> -     Arguments in this group (and subgroup) will be applied to a
> DPDKLocation when the DPDK tree, tarball or revision will be provided,
> -     other arguments like remote source and build dir are optional. A
> DPDKLocation from settings are used instead of from config if
> -     construct successful.
> -
> -     --dpdk-tree DIR_PATH  [DTS_DPDK_TREE] The path to the DPDK source
> tree directory to test. Cannot be used in conjunction with --tarball.
> -                           (default: None)
> -     --tarball FILE_PATH, --snapshot FILE_PATH
> -                           [DTS_DPDK_TARBALL] The path to the DPDK source
> tarball to test. DPDK must be contained in a folder with the same
> -                           name as the tarball file. Cannot be used in
> conjunction with --dpdk-tree. (default: None)
> -     --remote-source       [DTS_REMOTE_SOURCE] Set this option if either
> the DPDK source tree or tarball to be used are located on the SUT
> -                           node. Can only be used with --dpdk-tree or
> --tarball. (default: False)
> -     --precompiled-build-dir DIR_NAME
> -                           [DTS_PRECOMPILED_BUILD_DIR] Define the
> subdirectory under the DPDK tree root directory or tarball where the pre-
> -                           compiled binaries are located. (default: None)
> -
> -
> -The brackets contain the names of environment variables that set the same
> thing.
> -The minimum DTS needs is a config file and a pre-built DPDK
> -or DPDK sources location which can be specified in said config file
> -or on the command line or environment variables.
> -
> -
>  DTS Results
>  ~~~~~~~~~~~
>
> @@ -308,140 +247,89 @@ 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.
> -
> -
> -How To Write a Test Suite
> --------------------------
> -
> -All test suites inherit from ``TestSuite`` defined in
> ``dts/framework/test_suite.py``.
>

"All test suites are a class which inherits from"


> -There are four types of methods that comprise a test suite:
> -
> -#. **Test cases**
> -
> -   | 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``.
>

Now decorator based.


> -   | 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.
> -
> -#. **Setup and Teardown methods**
> -
> -   | 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.
> -
> -#. **Configuration, traffic and other logic**
> -
> -   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 test suites also frequently use a DPDK app, such as testpmd, in
> interactive mode
> -   and use the interactive shell instances directly.
> -
> -   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.
> -
> -#. **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.
> -
> -#. **Other methods**
>

I see that some of the content under "Other methods" is now false and
should be removed - thanks for doing so. However, I do think there was a
lot of good within the original "Test Cases," "Setup and Teardown Methods,"
and "Configuration, traffic and other logic" which has been removed. For
this one I prefer if we just sit down and hash it out in person when you're
in next week.


> -
> -   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.
> +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``.
>
> -.. _dts_dev_tools:
> +Code must be documented with docstrings that follow the
> +`Google style <
> https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings
> >`_.
> +Additional references:
>
> -DTS Developer Tools
> --------------------
> +* `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
>
> -There are two tools used in DTS to help with code checking, style and
> formatting:
> +* 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.
>
> -* `ruff <https://astral.sh/ruff/>`_
> +Example::
>
> -  An extremely fast all-in-one linting and formatting solution,
> -  which covers most if not all the major rules such as:
> -  pylama, flake8, pylint etc.
> -  Its built-in formatter is also Black-compatible
> -  and is able to sort imports automatically like isort would.
> +   def foo(greet: bool) -> None:
> +       """Demonstrates single vs. double backticks.
>
> -* `mypy <https://github.com/python/mypy>`_
> +       `greet` controls whether ``Hello World`` is printed.
>
> -  Enables static typing for Python, exploiting the type hints in the
> source code.
> +       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.
> +
> +
> +Creating a Test Suite
> +---------------------
> +
> +All test suites inherit from ``TestSuite`` in
> ``dts/framework/test_suite.py``. A typical suite contains:
> +
> +- Test Cases
> +  - Functional: start with ``test_``, e.g., ``test_basic_link``
> +  - Performance: start with ``test_perf_``, e.g.,
> ``test_perf_throughput``


I realize you go on to describe the decorators after this, but I think the
test_func and test_perf naming convention is no longer required. Example:

    @requires(NicCapability.FLOW_CTRL)
    @func_test
    def flow_ctrl_port_configuration_persistence(self) -> None:
        """Flow control port configuration persistency test.

        Steps:
            For each port enable flow control for RX and TX individually.
        Verify:
            The configuration persists after the port is restarted.
        """



> +  - Import the ``func_test`` and/or ``perf_test`` decorators from
> ``TestSuite`` and add them above each test case method,
> +   e.g., ``@func_test`` followed by ``test_basic_link``
> +
> +- Setup/Teardown Hooks
> +  - Suite-level: ``set_up_suite()``, ``tear_down_suite()``
> +  - Case-level: ``set_up_test_case()``, ``tear_down_test_case()``
> +
> +- Verification
> +  Use ``self.verify(condition, message)`` to record test results.
>

I think the important part of "verify" to explain to people is that you are
setting the testcase assertion condition, not the recording aspect.


> +
> +- Support Methods
> +  Helper logic (e.g., traffic handling, config) should be in private
> methods or delegated to ``sut_node``.
>

I realize this is just a rewording that crept in, but this is wrong for a
couple reasons:

1. We no longer import node/sutnode when writing testsuites. Node is purely
framework code now, and is not exposed to testsuites.
2. sut_node (and tg_node) no longer exists.


> +
> +
> +.. _dts_dev_tools:
> +
> +Developer Tools
> +---------------
> +
> +- ruff
> +  - Linter and formatter (replaces flake8, pylint, isort, etc.)
> +  - Compatible with Black
> +
> +- mypy
> +  - Performs static type checking
> +
> +Run checks using:
> +
> +.. code-block:: console
>
> -These two tools are all used in ``devtools/dts-check-format.sh``,
> -the DTS code check and format script.
> -Refer to the script for usage: ``devtools/dts-check-format.sh -h``.
> +   devtools/dts-check-format.sh
>
>
>  .. _building_api_docs:
> --
> 2.49.0
>
>
Lots of improvements overall - keep up the good work! A productive Summer
ahead.

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

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