Re: [PATCH 2/2] dts: rework test results

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

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

On Fri, Jun 27, 2025 at 11:12 AM Thomas Wilks <thomas.wilks@arm.com> wrote:

> Refactor the DTS result recording system to use a hierarchical tree
> structure based on `ResultNode` and `ResultLeaf`, replacing the prior flat
> model of DTSResult, TestRunResult, and TestSuiteResult. This improves
> clarity, composability, and enables consistent traversal and aggregation
> of test outcomes.
>

Agreed.


>
> Update all FSM states and the runner to build results directly into the
> tree, capturing setup, teardown, and test outcomes uniformly. Errors are
> now stored directly as exceptions and reduced into an exit code, and
> summaries are generated using Pydantic-based serializers for JSON and text
> output. Finally, a new textual result summary is generated showing the
> result of all the steps.
>
> Signed-off-by: Thomas Wilks <thomas.wilks@arm.com>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> ---
>  dts/framework/runner.py                      |  33 +-
>  dts/framework/test_result.py                 | 882 +++++--------------
>  dts/framework/test_run.py                    | 137 +--
>  dts/framework/testbed_model/posix_session.py |   4 +-
>  4 files changed, 337 insertions(+), 719 deletions(-)
>
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index f20aa3576a..0a3d92b0c8 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -18,16 +18,10 @@
>  from framework.test_run import TestRun
>  from framework.testbed_model.node import Node
>
> -from .config import (
> -    Configuration,
> -    load_config,
> -)
> +from .config import Configuration, load_config
>  from .logger import DTSLogger, get_dts_logger
>  from .settings import SETTINGS
> -from .test_result import (
> -    DTSResult,
> -    Result,
> -)
> +from .test_result import ResultNode, TestRunResult
>
>
>  class DTSRunner:
> @@ -35,7 +29,7 @@ class DTSRunner:
>
>      _configuration: Configuration
>      _logger: DTSLogger
> -    _result: DTSResult
> +    _result: TestRunResult
>
>      def __init__(self):
>          """Initialize the instance with configuration, logger, result and
> string constants."""
> @@ -54,7 +48,9 @@ def __init__(self):
>          if not os.path.exists(SETTINGS.output_dir):
>              os.makedirs(SETTINGS.output_dir)
>          self._logger.add_dts_root_logger_handlers(SETTINGS.verbose,
> SETTINGS.output_dir)
> -        self._result = DTSResult(SETTINGS.output_dir, self._logger)
> +
> +        test_suites_result = ResultNode(label="test_suites")
> +        self._result = TestRunResult(test_suites=test_suites_result)
>
>      def run(self) -> None:
>          """Run DTS.
> @@ -66,34 +62,30 @@ def run(self) -> None:
>          try:
>              # check the python version of the server that runs dts
>              self._check_dts_python_version()
> -            self._result.update_setup(Result.PASS)
>
>              for node_config in self._configuration.nodes:
>                  nodes.append(Node(node_config))
>
> -            test_run_result =
> self._result.add_test_run(self._configuration.test_run)
>              test_run = TestRun(
>                  self._configuration.test_run,
>                  self._configuration.tests_config,
>                  nodes,
> -                test_run_result,
> +                self._result,
>              )
>              test_run.spin()
>
>          except Exception as e:
> -            self._logger.exception("An unexpected error has occurred.")
> +            self._logger.exception("An unexpected error has occurred.", e)
>              self._result.add_error(e)
> -            # raise
>
>          finally:
>              try:
>                  self._logger.set_stage("post_run")
>                  for node in nodes:
>                      node.close()
> -                self._result.update_teardown(Result.PASS)
>              except Exception as e:
> -                self._logger.exception("The final cleanup of nodes
> failed.")
> -                self._result.update_teardown(Result.ERROR, e)
> +                self._logger.exception("The final cleanup of nodes
> failed.", e)
> +                self._result.add_error(e)
>
>          # we need to put the sys.exit call outside the finally clause to
> make sure
>          # that unexpected exceptions will propagate
> @@ -116,9 +108,6 @@ def _check_dts_python_version(self) -> None:
>
>      def _exit_dts(self) -> None:
>          """Process all errors and exit with the proper exit code."""
> -        self._result.process()
> -
>          if self._logger:
>              self._logger.info("DTS execution has ended.")
> -
> -        sys.exit(self._result.get_return_code())
> +        sys.exit(self._result.process())
> diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
> index 7f576022c7..8ce6cc8fbf 100644
> --- a/dts/framework/test_result.py
> +++ b/dts/framework/test_result.py
> @@ -7,723 +7,323 @@
>
>  The results are recorded in a hierarchical manner:
>
> -    * :class:`DTSResult` contains
>      * :class:`TestRunResult` contains
> -    * :class:`TestSuiteResult` contains
> -    * :class:`TestCaseResult`
> +    * :class:`ResultNode` may contain itself or
> +    * :class:`ResultLeaf`
>
> -Each result may contain multiple lower level results, e.g. there are
> multiple
> -:class:`TestSuiteResult`\s in a :class:`TestRunResult`.
> -The results have common parts, such as setup and teardown results,
> captured in :class:`BaseResult`,
> -which also defines some common behaviors in its methods.
> -
> -Each result class has its own idiosyncrasies which they implement in
> overridden methods.
> +Each result may contain many intermediate steps, e.g. there are multiple
> +:class:`ResultNode`\s in a :class:`ResultNode`.
>
>  The :option:`--output` command line argument and the
> :envvar:`DTS_OUTPUT_DIR` environment
>  variable modify the directory where the files with results will be stored.
>  """
>
> -import json
> -from collections.abc import MutableSequence
> -from enum import Enum, auto
> +import sys
> +from collections import Counter
> +from enum import IntEnum, auto
> +from io import StringIO
>  from pathlib import Path
> -from typing import Any, Callable, TypedDict
> +from typing import Any, ClassVar, Literal, TextIO, Union
> +
> +from pydantic import (
> +    BaseModel,
> +    ConfigDict,
> +    Field,
> +    computed_field,
> +    field_serializer,
> +    model_serializer,
> +)
> +from typing_extensions import OrderedDict
>
> -from .config.test_run import TestRunConfiguration
> -from .exception import DTSError, ErrorSeverity
> -from .logger import DTSLogger
> -from .remote_session.dpdk import DPDKBuildInfo
> -from .testbed_model.os_session import OSSessionInfo
> -from .testbed_model.port import Port
> +from framework.remote_session.dpdk import DPDKBuildInfo
> +from framework.settings import SETTINGS
> +from framework.testbed_model.os_session import OSSessionInfo
>
> +from .exception import DTSError, ErrorSeverity, InternalError
>
> -class Result(Enum):
> +
> +class Result(IntEnum):
>      """The possible states that a setup, a teardown or a test case may
> end up in."""
>
>      #:
>      PASS = auto()
>      #:
> -    FAIL = auto()
> -    #:
> -    ERROR = auto()
> +    SKIP = auto()
>      #:
>      BLOCK = auto()
>      #:
> -    SKIP = auto()
> +    FAIL = auto()
> +    #:
> +    ERROR = auto()
>
>      def __bool__(self) -> bool:
>          """Only :attr:`PASS` is True."""
>          return self is self.PASS
>
>
> -class TestCaseResultDict(TypedDict):
> -    """Represents the `TestCaseResult` results.
> -
> -    Attributes:
> -        test_case_name: The name of the test case.
> -        result: The result name of the test case.
> -    """
> -
> -    test_case_name: str
> -    result: str
> -
> -
> -class TestSuiteResultDict(TypedDict):
> -    """Represents the `TestSuiteResult` results.
> -
> -    Attributes:
> -        test_suite_name: The name of the test suite.
> -        test_cases: A list of test case results contained in this test
> suite.
> -    """
> -
> -    test_suite_name: str
> -    test_cases: list[TestCaseResultDict]
> -
> -
> -class TestRunResultDict(TypedDict, total=False):
> -    """Represents the `TestRunResult` results.
> -
> -    Attributes:
> -        compiler_version: The version of the compiler used for the DPDK
> build.
> -        dpdk_version: The version of DPDK being tested.
> -        ports: A list of ports associated with the test run.
> -        test_suites: A list of test suite results included in this test
> run.
> -        summary: A dictionary containing overall results, such as
> pass/fail counts.
> -    """
> -
> -    compiler_version: str | None
> -    dpdk_version: str | None
> -    ports: list[dict[str, Any]]
> -    test_suites: list[TestSuiteResultDict]
> -    summary: dict[str, int | float]
> -
> +class ResultLeaf(BaseModel):
> +    """Class representing a result in the results tree.
>
> -class DtsRunResultDict(TypedDict):
> -    """Represents the `DtsRunResult` results.
> +    A leaf node that can contain the results for a
> :class:`~.test_suite.TestSuite`,
> +    :class:`.test_suite.TestCase` or a DTS execution step.
>
>      Attributes:
> -        test_runs: A list of test run results.
> -        summary: A summary dictionary containing overall statistics for
> the test runs.
> +        result: The actual result.
> +        reason: The reason of the result.
>      """
>

From running some testcases that resulted in Error or Fail, I agree adding
this is a big benefit. Providing an example for any others on the mailing
list who are curious:

  rte_flow: FAIL
    test_drop_action_ETH: PASS
    test_drop_action_IP: PASS
    test_drop_action_L4: PASS
    test_drop_action_VLAN: PASS
    test_egress_rules: SKIP
      reason: flow rule failed validation.
    test_jump_action: PASS
    test_modify_actions: FAIL
      reason: Packet was never received.
    test_priority_attribute: PASS
    test_queue_action_ETH: PASS
    test_queue_action_IP: PASS
    test_queue_action_L4: PASS
    test_queue_action_VLAN: PASS


>
>
> --
> 2.43.0
>
>
Looks good, I will apply to next-dts now.

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

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