From: Dean Marx <dmarx@iol.unh.edu>
To: "Tomáš Ďurovec" <tomas.durovec@pantheon.tech>
Cc: dev@dpdk.org
Subject: Re: [PATCH] dts: improve statistics
Date: Fri, 25 Oct 2024 13:59:53 -0400 [thread overview]
Message-ID: <CABD7UXOGNLN0s-JYj14P+nePxTkB5fCJp6BFhP0jZGLgnFH2Kg@mail.gmail.com> (raw)
In-Reply-To: <20240930162601.30317-1-tomas.durovec@pantheon.tech>
[-- 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 --]
next prev parent reply other threads:[~2024-10-25 17:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-30 16:26 Tomáš Ďurovec
2024-10-25 17:59 ` Dean Marx [this message]
-- strict thread matches above, loose matches on Subject: below --
2024-09-28 11:44 Tomáš Ďurovec
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABD7UXOGNLN0s-JYj14P+nePxTkB5fCJp6BFhP0jZGLgnFH2Kg@mail.gmail.com \
--to=dmarx@iol.unh.edu \
--cc=dev@dpdk.org \
--cc=tomas.durovec@pantheon.tech \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).