Re: [PATCH] dts: improve statistics

[Dean Marx <dmarx@iol.unh.edu>]

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

Hi Tomáš,

This all looks great, one thing I did notice when running locally is that
the "test_suites" section of results.json contains null values instead of
the names of the suites that were run. Could just be something strange
happening on my side, but I would double check on your end just in case.
Otherwise:

Reviewed-by: Dean Marx <dmarx@iol.unh.edu>

On Mon, Sep 30, 2024 at 12:26 PM Tomáš Ďurovec <tomas.durovec@pantheon.tech>
wrote:

> The previous version of statistics store only the last test run
> attribute and result. In this patch we are adding header for each
> test run and overall summary of test runs at the end. This is
> represented as textual summaries with basic information and json
> result with more detailed information.
>
> Signed-off-by: Tomáš Ďurovec <tomas.durovec@pantheon.tech>
>
> Depends-on: series-33184 ("DTS external DPDK build")
> ---
>  dts/framework/runner.py      |   7 +-
>  dts/framework/test_result.py | 409 ++++++++++++++++++++++++++++-------
>  2 files changed, 332 insertions(+), 84 deletions(-)
>
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index 7d463c1fa1..be615ccace 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -84,7 +84,7 @@ 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(self._logger)
> +        self._result = DTSResult(SETTINGS.output_dir, self._logger)
>          self._test_suite_class_prefix = "Test"
>          self._test_suite_module_prefix = "tests.TestSuite_"
>          self._func_test_case_regex = r"test_(?!perf_)"
> @@ -421,11 +421,12 @@ def _run_test_run(
>          self._logger.info(
>              f"Running test run with SUT '{
> test_run_config.system_under_test_node.name}'."
>          )
> -        test_run_result.add_sut_info(sut_node.node_info)
> +        test_run_result.ports = sut_node.ports
> +        test_run_result.sut_info = sut_node.node_info
>          try:
>              dpdk_location = SETTINGS.dpdk_location or
> test_run_config.dpdk_config.dpdk_location
>              sut_node.set_up_test_run(test_run_config, dpdk_location)
> -
> test_run_result.add_dpdk_build_info(sut_node.get_dpdk_build_info())
> +            test_run_result.dpdk_build_info =
> sut_node.get_dpdk_build_info()
>              tg_node.set_up_test_run(test_run_config, dpdk_location)
>              test_run_result.update_setup(Result.PASS)
>          except Exception as e:
> diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
> index 0a10723098..bf148a6b45 100644
> --- a/dts/framework/test_result.py
> +++ b/dts/framework/test_result.py
> @@ -22,18 +22,19 @@
>  variable modify the directory where the files with results will be stored.
>  """
>
> -import os.path
> +import json
>  from collections.abc import MutableSequence
> -from dataclasses import dataclass
> +from dataclasses import asdict, dataclass
>  from enum import Enum, auto
> +from pathlib import Path
>  from types import FunctionType
> -from typing import Union
> +from typing import Any, Callable, TypedDict
>
>  from .config import DPDKBuildInfo, NodeInfo, TestRunConfiguration,
> TestSuiteConfig
>  from .exception import DTSError, ErrorSeverity
>  from .logger import DTSLogger
> -from .settings import SETTINGS
>  from .test_suite import TestSuite
> +from .testbed_model.port import Port
>
>
>  @dataclass(slots=True, frozen=True)
> @@ -85,6 +86,60 @@ def __bool__(self) -> bool:
>          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 DtsRunResultDict(TypedDict):
> +    """Represents the `DtsRunResult` results.
> +
> +    Attributes:
> +        test_runs: A list of test run results.
> +        summary: A summary dictionary containing overall statistics for
> the test runs.
> +    """
> +
> +    test_runs: list[TestRunResultDict]
> +    summary: dict[str, int | float]
> +
> +
>  class FixtureResult:
>      """A record that stores the result of a setup or a teardown.
>
> @@ -198,14 +253,34 @@ def get_errors(self) -> list[Exception]:
>          """
>          return self._get_setup_teardown_errors() +
> self._get_child_errors()
>
> -    def add_stats(self, statistics: "Statistics") -> None:
> -        """Collate stats from the whole result hierarchy.
> +    def to_dict(self):
> +        """Convert the results hierarchy into a dictionary
> representation."""
> +
> +    def add_result(self, results: dict[str, int]):
> +        """Collate the test case result from the result hierarchy.
>
>          Args:
> -            statistics: The :class:`Statistics` object where the stats
> will be collated.
> +            results: The dictionary to which results will be collated.
>          """
>          for child_result in self.child_results:
> -            child_result.add_stats(statistics)
> +            child_result.add_result(results)
> +
> +    def generate_pass_rate_dict(self, test_run_summary) -> dict[str,
> float]:
> +        """Generate a dictionary with the FAIL/PASS ratio of all test
> cases.
> +
> +        Args:
> +            test_run_summary: The summary dictionary containing test
> result counts.
> +
> +        Returns:
> +            A dictionary with the FAIL/PASS ratio of all test cases.
> +        """
> +        return {
> +            "PASS_RATE": (
> +                float(test_run_summary[Result.PASS.name])
> +                * 100
> +                / sum(test_run_summary[result.name] for result in Result)
> +            )
> +        }
>
>
>  class DTSResult(BaseResult):
> @@ -220,31 +295,25 @@ class DTSResult(BaseResult):
>      and as such is where the data form the whole hierarchy is collated or
> processed.
>
>      The internal list stores the results of all test runs.
> -
> -    Attributes:
> -        dpdk_version: The DPDK version to record.
>      """
>
> -    dpdk_version: str | None
> +    _output_dir: str
>      _logger: DTSLogger
>      _errors: list[Exception]
>      _return_code: ErrorSeverity
> -    _stats_result: Union["Statistics", None]
> -    _stats_filename: str
>
> -    def __init__(self, logger: DTSLogger):
> +    def __init__(self, output_dir: str, logger: DTSLogger):
>          """Extend the constructor with top-level specifics.
>
>          Args:
> +            output_dir: The directory where DTS logs and results are
> saved.
>              logger: The logger instance the whole result will use.
>          """
>          super().__init__()
> -        self.dpdk_version = None
> +        self._output_dir = output_dir
>          self._logger = logger
>          self._errors = []
>          self._return_code = ErrorSeverity.NO_ERR
> -        self._stats_result = None
> -        self._stats_filename = os.path.join(SETTINGS.output_dir,
> "statistics.txt")
>
>      def add_test_run(self, test_run_config: TestRunConfiguration) ->
> "TestRunResult":
>          """Add and return the child result (test run).
> @@ -281,10 +350,8 @@ def process(self) -> None:
>              for error in self._errors:
>                  self._logger.debug(repr(error))
>
> -        self._stats_result = Statistics(self.dpdk_version)
> -        self.add_stats(self._stats_result)
> -        with open(self._stats_filename, "w+") as stats_file:
> -            stats_file.write(str(self._stats_result))
> +        TextSummary(self).save(Path(self._output_dir,
> "results_summary.txt"))
> +        JsonResults(self).save(Path(self._output_dir, "results.json"))
>
>      def get_return_code(self) -> int:
>          """Go through all stored Exceptions and return the final DTS
> error code.
> @@ -302,6 +369,37 @@ def get_return_code(self) -> int:
>
>          return int(self._return_code)
>
> +    def to_dict(self) -> DtsRunResultDict:
> +        """Convert DTS result into a dictionary format.
> +
> +        The dictionary contains test runs and summary of test runs.
> +
> +        Returns:
> +            A dictionary representation of the DTS result
> +        """
> +
> +        def merge_test_run_summaries(test_run_summaries: list[dict[str,
> int]]) -> dict[str, int]:
> +            """Merge multiple test run summaries into one dictionary.
> +
> +            Args:
> +                test_run_summaries: List of test run summary dictionaries.
> +
> +            Returns:
> +                A merged dictionary containing the aggregated summary.
> +            """
> +            return {
> +                key.name: sum(test_run_summary[key.name] for
> test_run_summary in test_run_summaries)
> +                for key in Result
> +            }
> +
> +        test_runs = [child.to_dict() for child in self.child_results]
> +        test_run_summary = merge_test_run_summaries([test_run["summary"]
> for test_run in test_runs])
> +
> +        return {
> +            "test_runs": test_runs,
> +            "summary": test_run_summary |
> self.generate_pass_rate_dict(test_run_summary),
> +        }
> +
>
>  class TestRunResult(BaseResult):
>      """The test run specific result.
> @@ -316,13 +414,11 @@ class TestRunResult(BaseResult):
>          sut_kernel_version: The operating system kernel version of the
> SUT node.
>      """
>
> -    compiler_version: str | None
> -    dpdk_version: str | None
> -    sut_os_name: str
> -    sut_os_version: str
> -    sut_kernel_version: str
>      _config: TestRunConfiguration
>      _test_suites_with_cases: list[TestSuiteWithCases]
> +    _ports: list[Port]
> +    _sut_info: NodeInfo | None
> +    _dpdk_build_info: DPDKBuildInfo | None
>
>      def __init__(self, test_run_config: TestRunConfiguration):
>          """Extend the constructor with the test run's config.
> @@ -331,10 +427,11 @@ def __init__(self, test_run_config:
> TestRunConfiguration):
>              test_run_config: A test run configuration.
>          """
>          super().__init__()
> -        self.compiler_version = None
> -        self.dpdk_version = None
>          self._config = test_run_config
>          self._test_suites_with_cases = []
> +        self._ports = []
> +        self._sut_info = None
> +        self._dpdk_build_info = None
>
>      def add_test_suite(
>          self,
> @@ -374,24 +471,96 @@ def test_suites_with_cases(self,
> test_suites_with_cases: list[TestSuiteWithCases
>              )
>          self._test_suites_with_cases = test_suites_with_cases
>
> -    def add_sut_info(self, sut_info: NodeInfo) -> None:
> -        """Add SUT information gathered at runtime.
> +    @property
> +    def ports(self) -> list[Port]:
> +        """Get the list of ports associated with this test run."""
> +        return self._ports
> +
> +    @ports.setter
> +    def ports(self, ports: list[Port]) -> None:
> +        """Set the list of ports associated with this test run.
> +
> +        Args:
> +            ports: The list of ports to associate with this test run.
> +
> +        Raises:
> +            ValueError: If the ports have already been assigned to this
> test run.
> +        """
> +        if self._ports:
> +            raise ValueError(
> +                "Attempted to assign `ports` to a test run result which
> already has `ports`."
> +            )
> +        self._ports = ports
> +
> +    @property
> +    def sut_info(self) -> NodeInfo | None:
> +        """Get the SUT node information associated with this test run."""
> +        return self._sut_info
> +
> +    @sut_info.setter
> +    def sut_info(self, sut_info: NodeInfo) -> None:
> +        """Set the SUT node information associated with this test run.
>
>          Args:
> -            sut_info: The additional SUT node information.
> +            sut_info: The SUT node information to associate with this
> test run.
> +
> +        Raises:
> +            ValueError: If the SUT information has already been assigned
> to this test run.
>          """
> -        self.sut_os_name = sut_info.os_name
> -        self.sut_os_version = sut_info.os_version
> -        self.sut_kernel_version = sut_info.kernel_version
> +        if self._sut_info:
> +            raise ValueError(
> +                "Attempted to assign `sut_info` to a test run result
> which already has `sut_info`."
> +            )
> +        self._sut_info = sut_info
>
> -    def add_dpdk_build_info(self, versions: DPDKBuildInfo) -> None:
> -        """Add information about the DPDK build gathered at runtime.
> +    @property
> +    def dpdk_build_info(self) -> DPDKBuildInfo | None:
> +        """Get the DPDK build information associated with this test
> run."""
> +        return self._dpdk_build_info
> +
> +    @dpdk_build_info.setter
> +    def dpdk_build_info(self, dpdk_build_info: DPDKBuildInfo) -> None:
> +        """Set the DPDK build information associated with this test run.
>
>          Args:
> -            versions: The additional information.
> +            dpdk_build_info: The DPDK build information to associate with
> this test run.
> +
> +        Raises:
> +            ValueError: If the DPDK build information has already been
> assigned to this test run.
>          """
> -        self.compiler_version = versions.compiler_version
> -        self.dpdk_version = versions.dpdk_version
> +        if self._dpdk_build_info:
> +            raise ValueError(
> +                "Attempted to assign `dpdk_build_info` to a test run
> result which already "
> +                "has `dpdk_build_info`."
> +            )
> +        self._dpdk_build_info = dpdk_build_info
> +
> +    def to_dict(self) -> TestRunResultDict:
> +        """Convert the test run result into a dictionary.
> +
> +        The dictionary contains test suites in this test run, and a
> summary of the test run and
> +        information about the DPDK version, compiler version and
> associated ports.
> +
> +        Returns:
> +            TestRunResultDict: A dictionary representation of the test
> run result.
> +        """
> +        results = {result.name: 0 for result in Result}
> +        self.add_result(results)
> +
> +        compiler_version = None
> +        dpdk_version = None
> +
> +        if self.dpdk_build_info:
> +            compiler_version = self.dpdk_build_info.compiler_version
> +            dpdk_version = self.dpdk_build_info.dpdk_version
> +
> +        return {
> +            "compiler_version": compiler_version,
> +            "dpdk_version": dpdk_version,
> +            "ports": [asdict(port) for port in self.ports],
> +            "test_suites": [child.to_dict() for child in
> self.child_results],
> +            "summary": results | self.generate_pass_rate_dict(results),
> +        }
>
>      def _block_result(self) -> None:
>          r"""Mark the result as :attr:`~Result.BLOCK`\ed."""
> @@ -436,6 +605,16 @@ def add_test_case(self, test_case_name: str) ->
> "TestCaseResult":
>          self.child_results.append(result)
>          return result
>
> +    def to_dict(self) -> TestSuiteResultDict:
> +        """Convert the test suite result into a dictionary.
> +
> +        The dictionary contains a test suite name and test cases given in
> this test suite.
> +        """
> +        return {
> +            "test_suite_name": self.test_suite_name,
> +            "test_cases": [child.to_dict() for child in
> self.child_results],
> +        }
> +
>      def _block_result(self) -> None:
>          r"""Mark the result as :attr:`~Result.BLOCK`\ed."""
>          for test_case_method in self._test_suite_with_cases.test_cases:
> @@ -483,16 +662,23 @@ def _get_child_errors(self) -> list[Exception]:
>              return [self.error]
>          return []
>
> -    def add_stats(self, statistics: "Statistics") -> None:
> -        r"""Add the test case result to statistics.
> +    def to_dict(self) -> TestCaseResultDict:
> +        """Convert the test case result into a dictionary.
> +
> +        The dictionary contains a test case name and the result name.
> +        """
> +        return {"test_case_name": self.test_case_name, "result":
> self.result.name}
> +
> +    def add_result(self, results: dict[str, int]):
> +        r"""Add the test case result to the results.
>
>          The base method goes through the hierarchy recursively and this
> method is here to stop
> -        the recursion, as the :class:`TestCaseResult`\s are the leaves of
> the hierarchy tree.
> +        the recursion, as the :class:`TestCaseResult` are the leaves of
> the hierarchy tree.
>
>          Args:
> -            statistics: The :class:`Statistics` object where the stats
> will be added.
> +            results: The dictionary to which results will be collated.
>          """
> -        statistics += self.result
> +        results[self.result.name] += 1
>
>      def _block_result(self) -> None:
>          r"""Mark the result as :attr:`~Result.BLOCK`\ed."""
> @@ -503,53 +689,114 @@ def __bool__(self) -> bool:
>          return bool(self.setup_result) and bool(self.teardown_result) and
> bool(self.result)
>
>
> -class Statistics(dict):
> -    """How many test cases ended in which result state along some other
> basic information.
> +class TextSummary:
> +    """Generates and saves textual summaries of DTS run results.
>
> -    Subclassing :class:`dict` provides a convenient way to format the
> data.
> +    The summary includes:
> +    * Results of test run test cases,
> +    * Compiler version of the DPDK build,
> +    * DPDK version of the DPDK source tree,
> +    * Overall summary of results when multiple test runs are present.
> +    """
>
> -    The data are stored in the following keys:
> +    _dict_result: DtsRunResultDict
> +    _summary: dict[str, int | float]
> +    _text: str
>
> -    * **PASS RATE** (:class:`int`) -- The FAIL/PASS ratio of all test
> cases.
> -    * **DPDK VERSION** (:class:`str`) -- The tested DPDK version.
> -    """
> +    def __init__(self, dts_run_result: DTSResult):
> +        """Initializes with a DTSResult object and converts it to a
> dictionary format.
>
> -    def __init__(self, dpdk_version: str | None):
> -        """Extend the constructor with keys in which the data are stored.
> +        Args:
> +            dts_run_result: The DTS result.
> +        """
> +        self._dict_result = dts_run_result.to_dict()
> +        self._summary = self._dict_result["summary"]
> +        self._text = ""
> +
> +    @property
> +    def _outdent(self) -> str:
> +        """Appropriate indentation based on multiple test run results."""
> +        return "\t" if len(self._dict_result["test_runs"]) > 1 else ""
> +
> +    def save(self, output_path: Path):
> +        """Generate and save text statistics to a file.
>
>          Args:
> -            dpdk_version: The version of tested DPDK.
> +            output_path: The path where the text file will be saved.
>          """
> -        super().__init__()
> -        for result in Result:
> -            self[result.name] = 0
> -        self["PASS RATE"] = 0.0
> -        self["DPDK VERSION"] = dpdk_version
> +        if self._dict_result["test_runs"]:
> +            with open(f"{output_path}", "w") as fp:
> +
> self._add_test_runs_dict_decorator(self._add_test_run_dict)
> +                fp.write(self._text)
>
> -    def __iadd__(self, other: Result) -> "Statistics":
> -        """Add a Result to the final count.
> +    def _add_test_runs_dict_decorator(self, func: Callable):
> +        """Handles multiple test runs and appends results to the summary.
>
> -        Example:
> -            stats: Statistics = Statistics()  # empty Statistics
> -            stats += Result.PASS  # add a Result to `stats`
> +        Adds headers for each test run and overall result when multiple
> +        test runs are provided.
>
>          Args:
> -            other: The Result to add to this statistics object.
> +            func: Function to process and add results from each test run.
> +        """
> +        if len(self._dict_result["test_runs"]) > 1:
> +            for idx, test_run_result in
> enumerate(self._dict_result["test_runs"]):
> +                self._text += f"TEST_RUN_{idx}\n"
> +                func(test_run_result)
>
> -        Returns:
> -            The modified statistics object.
> +            self._add_overall_results()
> +        else:
> +            func(self._dict_result["test_runs"][0])
> +
> +    def _add_test_run_dict(self, test_run_dict: TestRunResultDict):
> +        """Adds the results and the test run attributes of a single test
> run to the summary.
> +
> +        Args:
> +            test_run_dict: Dictionary containing the test run results.
>          """
> -        self[other.name] += 1
> -        self["PASS RATE"] = (
> -            float(self[Result.PASS.name]) * 100 / sum(self[result.name]
> for result in Result)
> +        self._add_column(
> +            DPDK_VERSION=test_run_dict["dpdk_version"],
> +            COMPILER_VERSION=test_run_dict["compiler_version"],
> +            **test_run_dict["summary"],
>          )
> -        return self
> -
> -    def __str__(self) -> str:
> -        """Each line contains the formatted key = value pair."""
> -        stats_str = ""
> -        for key, value in self.items():
> -            stats_str += f"{key:<12} = {value}\n"
> -            # according to docs, we should use \n when writing to text
> files
> -            # on all platforms
> -        return stats_str
> +        self._text += "\n"
> +
> +    def _add_column(self, **rows):
> +        """Formats and adds key-value pairs to the summary text.
> +
> +        Handles cases where values might be None by replacing them with
> "N/A".
> +
> +        Args:
> +            **rows: Arbitrary key-value pairs representing the result
> data.
> +        """
> +        rows = {k: "N/A" if v is None else v for k, v in rows.items()}
> +        max_length = len(max(rows, key=len))
> +        for key, value in rows.items():
> +            self._text += f"{self._outdent}{key:<{max_length}} =
> {value}\n"
> +
> +    def _add_overall_results(self):
> +        """Add overall summary of test runs."""
> +        self._text += "OVERALL\n"
> +        self._add_column(**self._summary)
> +
> +
> +class JsonResults:
> +    """Save DTS run result in JSON format."""
> +
> +    _dict_result: DtsRunResultDict
> +
> +    def __init__(self, dts_run_result: DTSResult):
> +        """Initializes with a DTSResult object and converts it to a
> dictionary format.
> +
> +        Args:
> +            dts_run_result: The DTS result.
> +        """
> +        self._dict_result = dts_run_result.to_dict()
> +
> +    def save(self, output_path: Path):
> +        """Save the result to a file as JSON.
> +
> +        Args:
> +            output_path: The path where the JSON file will be saved.
> +        """
> +        with open(f"{output_path}", "w") as fp:
> +            json.dump(self._dict_result, fp, indent=4)
> --
> 2.46.1
>
>

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