From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 40A88433AC;
	Thu, 23 Nov 2023 16:15:12 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 2FD3342FFB;
	Thu, 23 Nov 2023 16:14:07 +0100 (CET)
Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com
 [209.85.128.53]) by mails.dpdk.org (Postfix) with ESMTP id C528E42FED
 for <dev@dpdk.org>; Thu, 23 Nov 2023 16:13:58 +0100 (CET)
Received: by mail-wm1-f53.google.com with SMTP id
 5b1f17b1804b1-40b31232bf0so7566515e9.1
 for <dev@dpdk.org>; Thu, 23 Nov 2023 07:13:58 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=pantheon.tech; s=google; t=1700752438; x=1701357238; darn=dpdk.org;
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:date:subject:cc:to:from:from:to:cc:subject:date
 :message-id:reply-to;
 bh=r8ptTkrC1tsM0AxROhb6o0FU3WTzzkdfCea6YQDDbXw=;
 b=eU8PxLod17RyytYrldN9PFWYPZ3oz5tF+xIg+BBzgGzyeQqj4Syju04vbwvzitt/d1
 M1C/6glkhnjeiOLmuLyQb3QPSzwfCLf7Acz3gjeOtgfGGkByIhb9H21VdfNBPCZptJHZ
 gicHRjKTTRX898eWRrSV4ShaQA9ywRnir/LWWEpUr14FEGNN7fo7Vqu5dWb2YPEwAlUw
 pk2+n28kUbIeHut0aw4hRSuCksvkpmtjv9+AnsY0YHyHsdsX3zGk/Uf3t0h9bonBOBMX
 UvvHaETivviL1f6VNtJp1v8SUOwFBLRcz2qdKs+QtPZtCDxkC/LnzIPp+7ltaotKiA3P
 Cbrg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1700752438; x=1701357238;
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=r8ptTkrC1tsM0AxROhb6o0FU3WTzzkdfCea6YQDDbXw=;
 b=aZi3x5W26QdNp7p/f2XkRJEirnxmQ7v3+ENSTLAge1tSjkhPjcA7n6oGROvXoxbhSs
 xNIcGUgmTCMgZUm6ebMDLI8W4k8L+rMxtiw8wOKYSbI/DYu9hr5cSI4VItOgHhmTYcBY
 B7Oj5CBEh5r2E9bNGAfoLl+4F9V6990CiXGJXR5SxNm8lDjWyMzd4ewxM+KN/y0S/Vp7
 yd22TR5nUSDrdvrr/TgDwD88OiOK95wb/Mf510DvezifdNiVb9BDGJsjPi79vhMDQfNc
 e9uEIN/mgJBcvytHfQJynUCfoG8Gl9nrlswiFStlED8SQLcDTJwMz4cPcq1T8Lq2/NZX
 whCw==
X-Gm-Message-State: AOJu0Yx8IJ6xXZYBCxUmxhAQYQPuZKbKJScjsFTW4mRbMaDCwcKSSSWa
 +77RsmDOgEiIzQw/ZFRZbNwBfA==
X-Google-Smtp-Source: AGHT+IEokFc5aR5oemBwozvusQniGOTWe5LzjJT2fOt0Vp3FT+ICVvLufIR3uhtuHJNntVHGoYZXJA==
X-Received: by 2002:a05:600c:45c7:b0:40b:37ef:3671 with SMTP id
 s7-20020a05600c45c700b0040b37ef3671mr799560wmo.38.1700752438318; 
 Thu, 23 Nov 2023 07:13:58 -0800 (PST)
Received: from jlinkes-PT-Latitude-5530.. ([84.245.121.10])
 by smtp.gmail.com with ESMTPSA id
 q4-20020adfea04000000b003296b488961sm1870143wrm.31.2023.11.23.07.13.56
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Thu, 23 Nov 2023 07:13:57 -0800 (PST)
From: =?UTF-8?q?Juraj=20Linke=C5=A1?= <juraj.linkes@pantheon.tech>
To: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu,
 probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com,
 Luca.Vizzarro@arm.com
Cc: dev@dpdk.org, =?UTF-8?q?Juraj=20Linke=C5=A1?= <juraj.linkes@pantheon.tech>
Subject: [PATCH v8 09/21] dts: test result docstring update
Date: Thu, 23 Nov 2023 16:13:32 +0100
Message-Id: <20231123151344.162812-10-juraj.linkes@pantheon.tech>
X-Mailer: git-send-email 2.34.1
In-Reply-To: <20231123151344.162812-1-juraj.linkes@pantheon.tech>
References: <20231115130959.39420-1-juraj.linkes@pantheon.tech>
 <20231123151344.162812-1-juraj.linkes@pantheon.tech>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

Format according to the Google format and PEP257, with slight
deviations.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/framework/test_result.py | 297 ++++++++++++++++++++++++++++-------
 1 file changed, 239 insertions(+), 58 deletions(-)

diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
index 57090feb04..4467749a9d 100644
--- a/dts/framework/test_result.py
+++ b/dts/framework/test_result.py
@@ -2,8 +2,25 @@
 # Copyright(c) 2023 PANTHEON.tech s.r.o.
 # Copyright(c) 2023 University of New Hampshire
 
-"""
-Generic result container and reporters
+r"""Record and process DTS results.
+
+The results are recorded in a hierarchical manner:
+
+    * :class:`DTSResult` contains
+    * :class:`ExecutionResult` contains
+    * :class:`BuildTargetResult` contains
+    * :class:`TestSuiteResult` contains
+    * :class:`TestCaseResult`
+
+Each result may contain multiple lower level results, e.g. there are multiple
+:class:`TestSuiteResult`\s in a :class:`BuildTargetResult`.
+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.
+
+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 os.path
@@ -26,26 +43,34 @@
 
 
 class Result(Enum):
-    """
-    An Enum defining the possible states that
-    a setup, a teardown or a test case may end up in.
-    """
+    """The possible states that a setup, a teardown or a test case may end up in."""
 
+    #:
     PASS = auto()
+    #:
     FAIL = auto()
+    #:
     ERROR = auto()
+    #:
     SKIP = auto()
 
     def __bool__(self) -> bool:
+        """Only PASS is True."""
         return self is self.PASS
 
 
 class FixtureResult(object):
-    """
-    A record that stored the result of a setup or a teardown.
-    The default is FAIL because immediately after creating the object
-    the setup of the corresponding stage will be executed, which also guarantees
-    the execution of teardown.
+    """A record that stores the result of a setup or a teardown.
+
+    :attr:`~Result.FAIL` is a sensible default since it prevents false positives (which could happen
+    if the default was :attr:`~Result.PASS`).
+
+    Preventing false positives or other false results is preferable since a failure
+    is mostly likely to be investigated (the other false results may not be investigated at all).
+
+    Attributes:
+        result: The associated result.
+        error: The error in case of a failure.
     """
 
     result: Result
@@ -56,21 +81,37 @@ def __init__(
         result: Result = Result.FAIL,
         error: Exception | None = None,
     ):
+        """Initialize the constructor with the fixture result and store a possible error.
+
+        Args:
+            result: The result to store.
+            error: The error which happened when a failure occurred.
+        """
         self.result = result
         self.error = error
 
     def __bool__(self) -> bool:
+        """A wrapper around the stored :class:`Result`."""
         return bool(self.result)
 
 
 class Statistics(dict):
-    """
-    A helper class used to store the number of test cases by its result
-    along a few other basic information.
-    Using a dict provides a convenient way to format the data.
+    """How many test cases ended in which result state along some other basic information.
+
+    Subclassing :class:`dict` provides a convenient way to format the data.
+
+    The data are stored in the following keys:
+
+    * **PASS RATE** (:class:`int`) -- The FAIL/PASS ratio of all test cases.
+    * **DPDK VERSION** (:class:`str`) -- The tested DPDK version.
     """
 
     def __init__(self, dpdk_version: str | None):
+        """Extend the constructor with keys in which the data are stored.
+
+        Args:
+            dpdk_version: The version of tested DPDK.
+        """
         super(Statistics, self).__init__()
         for result in Result:
             self[result.name] = 0
@@ -78,8 +119,17 @@ def __init__(self, dpdk_version: str | None):
         self["DPDK VERSION"] = dpdk_version
 
     def __iadd__(self, other: Result) -> "Statistics":
-        """
-        Add a Result to the final count.
+        """Add a Result to the final count.
+
+        Example:
+            stats: Statistics = Statistics()  # empty Statistics
+            stats += Result.PASS  # add a Result to `stats`
+
+        Args:
+            other: The Result to add to this statistics object.
+
+        Returns:
+            The modified statistics object.
         """
         self[other.name] += 1
         self["PASS RATE"] = (
@@ -88,9 +138,7 @@ def __iadd__(self, other: Result) -> "Statistics":
         return self
 
     def __str__(self) -> str:
-        """
-        Provide a string representation of the data.
-        """
+        """Each line contains the formatted key = value pair."""
         stats_str = ""
         for key, value in self.items():
             stats_str += f"{key:<12} = {value}\n"
@@ -100,10 +148,16 @@ def __str__(self) -> str:
 
 
 class BaseResult(object):
-    """
-    The Base class for all results. Stores the results of
-    the setup and teardown portions of the corresponding stage
-    and a list of results from each inner stage in _inner_results.
+    """Common data and behavior of DTS results.
+
+    Stores the results of the setup and teardown portions of the corresponding stage.
+    The hierarchical nature of DTS results is captured recursively in an internal list.
+    A stage is each level in this particular hierarchy (pre-execution or the top-most level,
+    execution, build target, test suite and test case.)
+
+    Attributes:
+        setup_result: The result of the setup of the particular stage.
+        teardown_result: The results of the teardown of the particular stage.
     """
 
     setup_result: FixtureResult
@@ -111,15 +165,28 @@ class BaseResult(object):
     _inner_results: MutableSequence["BaseResult"]
 
     def __init__(self):
+        """Initialize the constructor."""
         self.setup_result = FixtureResult()
         self.teardown_result = FixtureResult()
         self._inner_results = []
 
     def update_setup(self, result: Result, error: Exception | None = None) -> None:
+        """Store the setup result.
+
+        Args:
+            result: The result of the setup.
+            error: The error that occurred in case of a failure.
+        """
         self.setup_result.result = result
         self.setup_result.error = error
 
     def update_teardown(self, result: Result, error: Exception | None = None) -> None:
+        """Store the teardown result.
+
+        Args:
+            result: The result of the teardown.
+            error: The error that occurred in case of a failure.
+        """
         self.teardown_result.result = result
         self.teardown_result.error = error
 
@@ -137,27 +204,55 @@ def _get_inner_errors(self) -> list[Exception]:
         ]
 
     def get_errors(self) -> list[Exception]:
+        """Compile errors from the whole result hierarchy.
+
+        Returns:
+            The errors from setup, teardown and all errors found in the whole result hierarchy.
+        """
         return self._get_setup_teardown_errors() + self._get_inner_errors()
 
     def add_stats(self, statistics: Statistics) -> None:
+        """Collate stats from the whole result hierarchy.
+
+        Args:
+            statistics: The :class:`Statistics` object where the stats will be collated.
+        """
         for inner_result in self._inner_results:
             inner_result.add_stats(statistics)
 
 
 class TestCaseResult(BaseResult, FixtureResult):
-    """
-    The test case specific result.
-    Stores the result of the actual test case.
-    Also stores the test case name.
+    r"""The test case specific result.
+
+    Stores the result of the actual test case. This is done by adding an extra superclass
+    in :class:`FixtureResult`. The setup and teardown results are :class:`FixtureResult`\s and
+    the class is itself a record of the test case.
+
+    Attributes:
+        test_case_name: The test case name.
     """
 
     test_case_name: str
 
     def __init__(self, test_case_name: str):
+        """Extend the constructor with `test_case_name`.
+
+        Args:
+            test_case_name: The test case's name.
+        """
         super(TestCaseResult, self).__init__()
         self.test_case_name = test_case_name
 
     def update(self, result: Result, error: Exception | None = None) -> None:
+        """Update the test case result.
+
+        This updates the result of the test case itself and doesn't affect
+        the results of the setup and teardown steps in any way.
+
+        Args:
+            result: The result of the test case.
+            error: The error that occurred in case of a failure.
+        """
         self.result = result
         self.error = error
 
@@ -167,36 +262,64 @@ def _get_inner_errors(self) -> list[Exception]:
         return []
 
     def add_stats(self, statistics: Statistics) -> None:
+        r"""Add the test case result to statistics.
+
+        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.
+
+        Args:
+            statistics: The :class:`Statistics` object where the stats will be added.
+        """
         statistics += self.result
 
     def __bool__(self) -> bool:
+        """The test case passed only if setup, teardown and the test case itself passed."""
         return bool(self.setup_result) and bool(self.teardown_result) and bool(self.result)
 
 
 class TestSuiteResult(BaseResult):
-    """
-    The test suite specific result.
-    The _inner_results list stores results of test cases in a given test suite.
-    Also stores the test suite name.
+    """The test suite specific result.
+
+    The internal list stores the results of all test cases in a given test suite.
+
+    Attributes:
+        suite_name: The test suite name.
     """
 
     suite_name: str
 
     def __init__(self, suite_name: str):
+        """Extend the constructor with `suite_name`.
+
+        Args:
+            suite_name: The test suite's name.
+        """
         super(TestSuiteResult, self).__init__()
         self.suite_name = suite_name
 
     def add_test_case(self, test_case_name: str) -> TestCaseResult:
+        """Add and return the inner result (test case).
+
+        Returns:
+            The test case's result.
+        """
         test_case_result = TestCaseResult(test_case_name)
         self._inner_results.append(test_case_result)
         return test_case_result
 
 
 class BuildTargetResult(BaseResult):
-    """
-    The build target specific result.
-    The _inner_results list stores results of test suites in a given build target.
-    Also stores build target specifics, such as compiler used to build DPDK.
+    """The build target specific result.
+
+    The internal list stores the results of all test suites in a given build target.
+
+    Attributes:
+        arch: The DPDK build target architecture.
+        os: The DPDK build target operating system.
+        cpu: The DPDK build target CPU.
+        compiler: The DPDK build target compiler.
+        compiler_version: The DPDK build target compiler version.
+        dpdk_version: The built DPDK version.
     """
 
     arch: Architecture
@@ -207,6 +330,11 @@ class BuildTargetResult(BaseResult):
     dpdk_version: str | None
 
     def __init__(self, build_target: BuildTargetConfiguration):
+        """Extend the constructor with the `build_target`'s build target config.
+
+        Args:
+            build_target: The build target's test run configuration.
+        """
         super(BuildTargetResult, self).__init__()
         self.arch = build_target.arch
         self.os = build_target.os
@@ -216,20 +344,35 @@ def __init__(self, build_target: BuildTargetConfiguration):
         self.dpdk_version = None
 
     def add_build_target_info(self, versions: BuildTargetInfo) -> None:
+        """Add information about the build target gathered at runtime.
+
+        Args:
+            versions: The additional information.
+        """
         self.compiler_version = versions.compiler_version
         self.dpdk_version = versions.dpdk_version
 
     def add_test_suite(self, test_suite_name: str) -> TestSuiteResult:
+        """Add and return the inner result (test suite).
+
+        Returns:
+            The test suite's result.
+        """
         test_suite_result = TestSuiteResult(test_suite_name)
         self._inner_results.append(test_suite_result)
         return test_suite_result
 
 
 class ExecutionResult(BaseResult):
-    """
-    The execution specific result.
-    The _inner_results list stores results of build targets in a given execution.
-    Also stores the SUT node configuration.
+    """The execution specific result.
+
+    The internal list stores the results of all build targets in a given execution.
+
+    Attributes:
+        sut_node: The SUT node used in the execution.
+        sut_os_name: The operating system of the SUT node.
+        sut_os_version: The operating system version of the SUT node.
+        sut_kernel_version: The operating system kernel version of the SUT node.
     """
 
     sut_node: NodeConfiguration
@@ -238,34 +381,53 @@ class ExecutionResult(BaseResult):
     sut_kernel_version: str
 
     def __init__(self, sut_node: NodeConfiguration):
+        """Extend the constructor with the `sut_node`'s config.
+
+        Args:
+            sut_node: The SUT node's test run configuration used in the execution.
+        """
         super(ExecutionResult, self).__init__()
         self.sut_node = sut_node
 
     def add_build_target(self, build_target: BuildTargetConfiguration) -> BuildTargetResult:
+        """Add and return the inner result (build target).
+
+        Args:
+            build_target: The build target's test run configuration.
+
+        Returns:
+            The build target's result.
+        """
         build_target_result = BuildTargetResult(build_target)
         self._inner_results.append(build_target_result)
         return build_target_result
 
     def add_sut_info(self, sut_info: NodeInfo) -> None:
+        """Add SUT information gathered at runtime.
+
+        Args:
+            sut_info: The additional SUT node information.
+        """
         self.sut_os_name = sut_info.os_name
         self.sut_os_version = sut_info.os_version
         self.sut_kernel_version = sut_info.kernel_version
 
 
 class DTSResult(BaseResult):
-    """
-    Stores environment information and test results from a DTS run, which are:
-    * Execution level information, such as SUT and TG hardware.
-    * Build target level information, such as compiler, target OS and cpu.
-    * Test suite results.
-    * All errors that are caught and recorded during DTS execution.
+    """Stores environment information and test results from a DTS run.
 
-    The information is stored in nested objects.
+        * Execution level information, such as testbed and the test suite list,
+        * Build target level information, such as compiler, target OS and cpu,
+        * Test suite and test case results,
+        * All errors that are caught and recorded during DTS execution.
 
-    The class is capable of computing the return code used to exit DTS with
-    from the stored error.
+    The information is stored hierarchically. This is the first level of the hierarchy
+    and as such is where the data form the whole hierarchy is collated or processed.
 
-    It also provides a brief statistical summary of passed/failed test cases.
+    The internal list stores the results of all executions.
+
+    Attributes:
+        dpdk_version: The DPDK version to record.
     """
 
     dpdk_version: str | None
@@ -276,6 +438,11 @@ class DTSResult(BaseResult):
     _stats_filename: str
 
     def __init__(self, logger: DTSLOG):
+        """Extend the constructor with top-level specifics.
+
+        Args:
+            logger: The logger instance the whole result will use.
+        """
         super(DTSResult, self).__init__()
         self.dpdk_version = None
         self._logger = logger
@@ -285,21 +452,33 @@ def __init__(self, logger: DTSLOG):
         self._stats_filename = os.path.join(SETTINGS.output_dir, "statistics.txt")
 
     def add_execution(self, sut_node: NodeConfiguration) -> ExecutionResult:
+        """Add and return the inner result (execution).
+
+        Args:
+            sut_node: The SUT node's test run configuration.
+
+        Returns:
+            The execution's result.
+        """
         execution_result = ExecutionResult(sut_node)
         self._inner_results.append(execution_result)
         return execution_result
 
     def add_error(self, error: Exception) -> None:
+        """Record an error that occurred outside any execution.
+
+        Args:
+            error: The exception to record.
+        """
         self._errors.append(error)
 
     def process(self) -> None:
-        """
-        Process the data after a DTS run.
-        The data is added to nested objects during runtime and this parent object
-        is not updated at that time. This requires us to process the nested data
-        after it's all been gathered.
+        """Process the data after a whole DTS run.
+
+        The data is added to inner objects during runtime and this object is not updated
+        at that time. This requires us to process the inner data after it's all been gathered.
 
-        The processing gathers all errors and the result statistics of test cases.
+        The processing gathers all errors and the statistics of test case results.
         """
         self._errors += self.get_errors()
         if self._errors and self._logger:
@@ -313,8 +492,10 @@ def process(self) -> None:
             stats_file.write(str(self._stats_result))
 
     def get_return_code(self) -> int:
-        """
-        Go through all stored Exceptions and return the highest error code found.
+        """Go through all stored Exceptions and return the final DTS error code.
+
+        Returns:
+            The highest error code found.
         """
         for error in self._errors:
             error_return_code = ErrorSeverity.GENERIC_ERR
-- 
2.34.1