[v7,07/21] dts: dts runner and main docstring update

Message ID 20231115130959.39420-8-juraj.linkes@pantheon.tech (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series dts: docstrings update |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Juraj Linkeš Nov. 15, 2023, 1:09 p.m. UTC
  Format according to the Google format and PEP257, with slight
deviations.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/framework/dts.py | 128 ++++++++++++++++++++++++++++++++++++-------
 dts/main.py          |   8 ++-
 2 files changed, 112 insertions(+), 24 deletions(-)
  

Comments

Jeremy Spewock Nov. 16, 2023, 9:51 p.m. UTC | #1
On Wed, Nov 15, 2023 at 8:11 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> Format according to the Google format and PEP257, with slight
> deviations.
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  dts/framework/dts.py | 128 ++++++++++++++++++++++++++++++++++++-------
>  dts/main.py          |   8 ++-
>  2 files changed, 112 insertions(+), 24 deletions(-)
>
> diff --git a/dts/framework/dts.py b/dts/framework/dts.py
> index 4c7fb0c40a..331fed7dc4 100644
> --- a/dts/framework/dts.py
> +++ b/dts/framework/dts.py
> @@ -3,6 +3,33 @@
>  # Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
>  # Copyright(c) 2022-2023 University of New Hampshire
>
> +r"""Test suite runner module.
> +
> +A DTS run is split into stages:
> +
> +    #. Execution stage,
> +    #. Build target stage,
> +    #. Test suite stage,
> +    #. Test case stage.
> +
> +The module is responsible for running tests on testbeds defined in the
> test run configuration.
> +Each setup or teardown of each stage is recorded in a
> :class:`~framework.test_result.DTSResult` or
> +one of its subclasses. The test case results are also recorded.
> +
> +If an error occurs, the current stage is aborted, the error is recorded
> and the run continues in
> +the next iteration of the same stage. The return code is the highest
> `severity` of all
> +:class:`~.framework.exception.DTSError`\s.
> +
> +Example:
> +    An error occurs in a build target setup. The current build target is
> aborted and the run
> +    continues with the next build target. If the errored build target was
> the last one in the given
> +    execution, the next execution begins.
> +
> +Attributes:
> +    dts_logger: The logger instance used in this module.
> +    result: The top level result used in the module.
> +"""
> +
>  import sys
>
>  from .config import (
> @@ -23,9 +50,38 @@
>
>
>  def run_all() -> None:
> -    """
> -    The main process of DTS. Runs all build targets in all executions
> from the main
> -    config file.
> +    """Run all build targets in all executions from the test run
> configuration.
> +
> +    Before running test suites, executions and build targets are first
> set up.
> +    The executions and build targets defined in the test run
> configuration are iterated over.
> +    The executions define which tests to run and where to run them and
> build targets define
> +    the DPDK build setup.
> +
> +    The tests suites are set up for each execution/build target tuple and
> each scheduled
> +    test case within the test suite is set up, executed and torn down.
> After all test cases
> +    have been executed, the test suite is torn down and the next build
> target will be tested.
> +
> +    All the nested steps look like this:
> +
> +        #. Execution setup
> +
> +            #. Build target setup
> +
> +                #. Test suite setup
> +
> +                    #. Test case setup
> +                    #. Test case logic
> +                    #. Test case teardown
> +
> +                #. Test suite teardown
> +
> +            #. Build target teardown
> +
> +        #. Execution teardown
> +
> +    The test cases are filtered according to the specification in the
> test run configuration and
> +    the :option:`--test-cases` command line argument or
> +    the :envvar:`DTS_TESTCASES` environment variable.
>      """
>      global dts_logger
>      global result
> @@ -87,6 +143,8 @@ def run_all() -> None:
>
>
>  def _check_dts_python_version() -> None:
> +    """Check the required Python version - v3.10."""
> +
>      def RED(text: str) -> str:
>          return f"\u001B[31;1m{str(text)}\u001B[0m"
>
> @@ -111,9 +169,16 @@ def _run_execution(
>      execution: ExecutionConfiguration,
>      result: DTSResult,
>  ) -> None:
> -    """
> -    Run the given execution. This involves running the execution setup as
> well as
> -    running all build targets in the given execution.
> +    """Run the given execution.
> +
> +    This involves running the execution setup as well as running all
> build targets
> +    in the given execution. After that, execution teardown is run.
> +
> +    Args:
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        execution: An execution's test run configuration.
> +        result: The top level result object.
>      """
>      dts_logger.info(
>          f"Running execution with SUT '{
> execution.system_under_test_node.name}'."
> @@ -150,8 +215,18 @@ def _run_build_target(
>      execution: ExecutionConfiguration,
>      execution_result: ExecutionResult,
>  ) -> None:
> -    """
> -    Run the given build target.
> +    """Run the given build target.
> +
> +    This involves running the build target setup as well as running all
> test suites
> +    in the given execution the build target is defined in.
> +    After that, build target teardown is run.
> +
> +    Args:
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        build_target: A build target's test run configuration.
> +        execution: The build target's execution's test run configuration.
> +        execution_result: The execution level result object associated
> with the execution.
>      """
>      dts_logger.info(f"Running build target '{build_target.name}'.")
>      build_target_result = execution_result.add_build_target(build_target)
> @@ -183,10 +258,17 @@ def _run_all_suites(
>      execution: ExecutionConfiguration,
>      build_target_result: BuildTargetResult,
>  ) -> None:
> -    """
> -    Use the given build_target to run execution's test suites
> -    with possibly only a subset of test cases.
> -    If no subset is specified, run all test cases.
> +    """Run the execution's (possibly a subset) test suites using the
> current build_target.
> +
> +    The function assumes the build target we're testing has already been
> built on the SUT node.
> +    The current build target thus corresponds to the current DPDK build
> present on the SUT node.
> +
> +    Args:
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        execution: The execution's test run configuration associated with
> the current build target.
> +        build_target_result: The build target level result object
> associated
> +            with the current build target.
>      """
>

Is it worth mentioning in this method or the _run_build_target method that
when a blocking suite fails that no more suites will be run on that build
target?


>      end_build_target = False
>      if not execution.skip_smoke_tests:
> @@ -215,16 +297,22 @@ def _run_single_suite(
>      build_target_result: BuildTargetResult,
>      test_suite_config: TestSuiteConfig,
>  ) -> None:
> -    """Runs a single test suite.
> +    """Run all test suite in a single test suite module.
> +
> +    The function assumes the build target we're testing has already been
> built on the SUT node.
> +    The current build target thus corresponds to the current DPDK build
> present on the SUT node.
>
>      Args:
> -        sut_node: Node to run tests on.
> -        execution: Execution the test case belongs to.
> -        build_target_result: Build target configuration test case is run
> on
> -        test_suite_config: Test suite configuration
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        execution: The execution's test run configuration associated with
> the current build target.
> +        build_target_result: The build target level result object
> associated
> +            with the current build target.
> +        test_suite_config: Test suite test run configuration specifying
> the test suite module
> +            and possibly a subset of test cases of test suites in that
> module.
>
>      Raises:
> -        BlockingTestSuiteError: If a test suite that was marked as
> blocking fails.
> +        BlockingTestSuiteError: If a blocking test suite fails.
>      """
>      try:
>          full_suite_path =
> f"tests.TestSuite_{test_suite_config.test_suite}"
> @@ -248,9 +336,7 @@ def _run_single_suite(
>
>
>  def _exit_dts() -> None:
> -    """
> -    Process all errors and exit with the proper exit code.
> -    """
> +    """Process all errors and exit with the proper exit code."""
>      result.process()
>
>      if dts_logger:
> diff --git a/dts/main.py b/dts/main.py
> index 5d4714b0c3..f703615d11 100755
> --- a/dts/main.py
> +++ b/dts/main.py
> @@ -4,9 +4,7 @@
>  # Copyright(c) 2022 PANTHEON.tech s.r.o.
>  # Copyright(c) 2022 University of New Hampshire
>
> -"""
> -A test framework for testing DPDK.
> -"""
> +"""The DTS executable."""
>
>  import logging
>
> @@ -17,6 +15,10 @@ def main() -> None:
>      """Set DTS settings, then run DTS.
>
>      The DTS settings are taken from the command line arguments and the
> environment variables.
> +    The settings object is stored in the module-level variable
> settings.SETTINGS which the entire
> +    framework uses. After importing the module (or the variable), any
> changes to the variable are
> +    not going to be reflected without a re-import. This means that the
> SETTINGS variable must
> +    be modified before the settings module is imported anywhere else in
> the framework.
>      """
>      settings.SETTINGS = settings.get_settings()
>      from framework import dts
> --
> 2.34.1
>
>
  
Juraj Linkeš Nov. 20, 2023, 4:13 p.m. UTC | #2
On Thu, Nov 16, 2023 at 10:51 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
>
>
> On Wed, Nov 15, 2023 at 8:11 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> Format according to the Google format and PEP257, with slight
>> deviations.
>>
>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>> ---
>>  dts/framework/dts.py | 128 ++++++++++++++++++++++++++++++++++++-------
>>  dts/main.py          |   8 ++-
>>  2 files changed, 112 insertions(+), 24 deletions(-)
>>
>> diff --git a/dts/framework/dts.py b/dts/framework/dts.py
>> index 4c7fb0c40a..331fed7dc4 100644
>> --- a/dts/framework/dts.py
>> +++ b/dts/framework/dts.py
>> @@ -3,6 +3,33 @@
>>  # Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
>>  # Copyright(c) 2022-2023 University of New Hampshire
>>
>> +r"""Test suite runner module.
>> +
>> +A DTS run is split into stages:
>> +
>> +    #. Execution stage,
>> +    #. Build target stage,
>> +    #. Test suite stage,
>> +    #. Test case stage.
>> +
>> +The module is responsible for running tests on testbeds defined in the test run configuration.
>> +Each setup or teardown of each stage is recorded in a :class:`~framework.test_result.DTSResult` or
>> +one of its subclasses. The test case results are also recorded.
>> +
>> +If an error occurs, the current stage is aborted, the error is recorded and the run continues in
>> +the next iteration of the same stage. The return code is the highest `severity` of all
>> +:class:`~.framework.exception.DTSError`\s.
>> +
>> +Example:
>> +    An error occurs in a build target setup. The current build target is aborted and the run
>> +    continues with the next build target. If the errored build target was the last one in the given
>> +    execution, the next execution begins.
>> +
>> +Attributes:
>> +    dts_logger: The logger instance used in this module.
>> +    result: The top level result used in the module.
>> +"""
>> +
>>  import sys
>>
>>  from .config import (
>> @@ -23,9 +50,38 @@
>>
>>
>>  def run_all() -> None:
>> -    """
>> -    The main process of DTS. Runs all build targets in all executions from the main
>> -    config file.
>> +    """Run all build targets in all executions from the test run configuration.
>> +
>> +    Before running test suites, executions and build targets are first set up.
>> +    The executions and build targets defined in the test run configuration are iterated over.
>> +    The executions define which tests to run and where to run them and build targets define
>> +    the DPDK build setup.
>> +
>> +    The tests suites are set up for each execution/build target tuple and each scheduled
>> +    test case within the test suite is set up, executed and torn down. After all test cases
>> +    have been executed, the test suite is torn down and the next build target will be tested.
>> +
>> +    All the nested steps look like this:
>> +
>> +        #. Execution setup
>> +
>> +            #. Build target setup
>> +
>> +                #. Test suite setup
>> +
>> +                    #. Test case setup
>> +                    #. Test case logic
>> +                    #. Test case teardown
>> +
>> +                #. Test suite teardown
>> +
>> +            #. Build target teardown
>> +
>> +        #. Execution teardown
>> +
>> +    The test cases are filtered according to the specification in the test run configuration and
>> +    the :option:`--test-cases` command line argument or
>> +    the :envvar:`DTS_TESTCASES` environment variable.
>>      """
>>      global dts_logger
>>      global result
>> @@ -87,6 +143,8 @@ def run_all() -> None:
>>
>>
>>  def _check_dts_python_version() -> None:
>> +    """Check the required Python version - v3.10."""
>> +
>>      def RED(text: str) -> str:
>>          return f"\u001B[31;1m{str(text)}\u001B[0m"
>>
>> @@ -111,9 +169,16 @@ def _run_execution(
>>      execution: ExecutionConfiguration,
>>      result: DTSResult,
>>  ) -> None:
>> -    """
>> -    Run the given execution. This involves running the execution setup as well as
>> -    running all build targets in the given execution.
>> +    """Run the given execution.
>> +
>> +    This involves running the execution setup as well as running all build targets
>> +    in the given execution. After that, execution teardown is run.
>> +
>> +    Args:
>> +        sut_node: The execution's SUT node.
>> +        tg_node: The execution's TG node.
>> +        execution: An execution's test run configuration.
>> +        result: The top level result object.
>>      """
>>      dts_logger.info(
>>          f"Running execution with SUT '{execution.system_under_test_node.name}'."
>> @@ -150,8 +215,18 @@ def _run_build_target(
>>      execution: ExecutionConfiguration,
>>      execution_result: ExecutionResult,
>>  ) -> None:
>> -    """
>> -    Run the given build target.
>> +    """Run the given build target.
>> +
>> +    This involves running the build target setup as well as running all test suites
>> +    in the given execution the build target is defined in.
>> +    After that, build target teardown is run.
>> +
>> +    Args:
>> +        sut_node: The execution's SUT node.
>> +        tg_node: The execution's TG node.
>> +        build_target: A build target's test run configuration.
>> +        execution: The build target's execution's test run configuration.
>> +        execution_result: The execution level result object associated with the execution.
>>      """
>>      dts_logger.info(f"Running build target '{build_target.name}'.")
>>      build_target_result = execution_result.add_build_target(build_target)
>> @@ -183,10 +258,17 @@ def _run_all_suites(
>>      execution: ExecutionConfiguration,
>>      build_target_result: BuildTargetResult,
>>  ) -> None:
>> -    """
>> -    Use the given build_target to run execution's test suites
>> -    with possibly only a subset of test cases.
>> -    If no subset is specified, run all test cases.
>> +    """Run the execution's (possibly a subset) test suites using the current build_target.
>> +
>> +    The function assumes the build target we're testing has already been built on the SUT node.
>> +    The current build target thus corresponds to the current DPDK build present on the SUT node.
>> +
>> +    Args:
>> +        sut_node: The execution's SUT node.
>> +        tg_node: The execution's TG node.
>> +        execution: The execution's test run configuration associated with the current build target.
>> +        build_target_result: The build target level result object associated
>> +            with the current build target.
>>      """
>
>
> Is it worth mentioning in this method or the _run_build_target method that when a blocking suite fails that no more suites will be run on that build target?
>

Absolutely, I'll add that. Thanks for the catch.

>>
>>      end_build_target = False
>>      if not execution.skip_smoke_tests:
>> @@ -215,16 +297,22 @@ def _run_single_suite(
>>      build_target_result: BuildTargetResult,
>>      test_suite_config: TestSuiteConfig,
>>  ) -> None:
>> -    """Runs a single test suite.
>> +    """Run all test suite in a single test suite module.
>> +
>> +    The function assumes the build target we're testing has already been built on the SUT node.
>> +    The current build target thus corresponds to the current DPDK build present on the SUT node.
>>
>>      Args:
>> -        sut_node: Node to run tests on.
>> -        execution: Execution the test case belongs to.
>> -        build_target_result: Build target configuration test case is run on
>> -        test_suite_config: Test suite configuration
>> +        sut_node: The execution's SUT node.
>> +        tg_node: The execution's TG node.
>> +        execution: The execution's test run configuration associated with the current build target.
>> +        build_target_result: The build target level result object associated
>> +            with the current build target.
>> +        test_suite_config: Test suite test run configuration specifying the test suite module
>> +            and possibly a subset of test cases of test suites in that module.
>>
>>      Raises:
>> -        BlockingTestSuiteError: If a test suite that was marked as blocking fails.
>> +        BlockingTestSuiteError: If a blocking test suite fails.
>>      """
>>      try:
>>          full_suite_path = f"tests.TestSuite_{test_suite_config.test_suite}"
>> @@ -248,9 +336,7 @@ def _run_single_suite(
>>
>>
>>  def _exit_dts() -> None:
>> -    """
>> -    Process all errors and exit with the proper exit code.
>> -    """
>> +    """Process all errors and exit with the proper exit code."""
>>      result.process()
>>
>>      if dts_logger:
>> diff --git a/dts/main.py b/dts/main.py
>> index 5d4714b0c3..f703615d11 100755
>> --- a/dts/main.py
>> +++ b/dts/main.py
>> @@ -4,9 +4,7 @@
>>  # Copyright(c) 2022 PANTHEON.tech s.r.o.
>>  # Copyright(c) 2022 University of New Hampshire
>>
>> -"""
>> -A test framework for testing DPDK.
>> -"""
>> +"""The DTS executable."""
>>
>>  import logging
>>
>> @@ -17,6 +15,10 @@ def main() -> None:
>>      """Set DTS settings, then run DTS.
>>
>>      The DTS settings are taken from the command line arguments and the environment variables.
>> +    The settings object is stored in the module-level variable settings.SETTINGS which the entire
>> +    framework uses. After importing the module (or the variable), any changes to the variable are
>> +    not going to be reflected without a re-import. This means that the SETTINGS variable must
>> +    be modified before the settings module is imported anywhere else in the framework.
>>      """
>>      settings.SETTINGS = settings.get_settings()
>>      from framework import dts
>> --
>> 2.34.1
>>
  
Yoan Picchi Nov. 20, 2023, 5:43 p.m. UTC | #3
On 11/15/23 13:09, Juraj Linkeš wrote:
> Format according to the Google format and PEP257, with slight
> deviations.
> 
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>   dts/framework/dts.py | 128 ++++++++++++++++++++++++++++++++++++-------
>   dts/main.py          |   8 ++-
>   2 files changed, 112 insertions(+), 24 deletions(-)
> 
> diff --git a/dts/framework/dts.py b/dts/framework/dts.py
> index 4c7fb0c40a..331fed7dc4 100644
> --- a/dts/framework/dts.py
> +++ b/dts/framework/dts.py
> @@ -3,6 +3,33 @@
>   # Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
>   # Copyright(c) 2022-2023 University of New Hampshire
>   
> +r"""Test suite runner module.

Is the r before the docstring intended?

> +
> +A DTS run is split into stages:
> +
> +    #. Execution stage,
> +    #. Build target stage,
> +    #. Test suite stage,
> +    #. Test case stage.
> +
> +The module is responsible for running tests on testbeds defined in the test run configuration.
> +Each setup or teardown of each stage is recorded in a :class:`~framework.test_result.DTSResult` or
> +one of its subclasses. The test case results are also recorded.
> +
> +If an error occurs, the current stage is aborted, the error is recorded and the run continues in
> +the next iteration of the same stage. The return code is the highest `severity` of all
> +:class:`~.framework.exception.DTSError`\s.

Is the . before the classname intended? considering the previous one 
doesn't have one. (I've not yet built the doc to check if it affect the 
rendered doc)

> +
> +Example:
> +    An error occurs in a build target setup. The current build target is aborted and the run
> +    continues with the next build target. If the errored build target was the last one in the given
> +    execution, the next execution begins.
> +
> +Attributes:
> +    dts_logger: The logger instance used in this module.
> +    result: The top level result used in the module.
> +"""
> +
>   import sys
>   
>   from .config import (
> @@ -23,9 +50,38 @@
>   
>   
>   def run_all() -> None:
> -    """
> -    The main process of DTS. Runs all build targets in all executions from the main
> -    config file.
> +    """Run all build targets in all executions from the test run configuration.
> +
> +    Before running test suites, executions and build targets are first set up.
> +    The executions and build targets defined in the test run configuration are iterated over.
> +    The executions define which tests to run and where to run them and build targets define
> +    the DPDK build setup.
> +
> +    The tests suites are set up for each execution/build target tuple and each scheduled
> +    test case within the test suite is set up, executed and torn down. After all test cases
> +    have been executed, the test suite is torn down and the next build target will be tested.
> +
> +    All the nested steps look like this:
> +
> +        #. Execution setup
> +
> +            #. Build target setup
> +
> +                #. Test suite setup
> +
> +                    #. Test case setup
> +                    #. Test case logic
> +                    #. Test case teardown
> +
> +                #. Test suite teardown
> +
> +            #. Build target teardown
> +
> +        #. Execution teardown
> +
> +    The test cases are filtered according to the specification in the test run configuration and
> +    the :option:`--test-cases` command line argument or
> +    the :envvar:`DTS_TESTCASES` environment variable.
>       """
>       global dts_logger
>       global result
> @@ -87,6 +143,8 @@ def run_all() -> None:
>   
>   
>   def _check_dts_python_version() -> None:
> +    """Check the required Python version - v3.10."""
> +
>       def RED(text: str) -> str:
>           return f"\u001B[31;1m{str(text)}\u001B[0m"
>   
> @@ -111,9 +169,16 @@ def _run_execution(
>       execution: ExecutionConfiguration,
>       result: DTSResult,
>   ) -> None:
> -    """
> -    Run the given execution. This involves running the execution setup as well as
> -    running all build targets in the given execution.
> +    """Run the given execution.
> +
> +    This involves running the execution setup as well as running all build targets
> +    in the given execution. After that, execution teardown is run.
> +
> +    Args:
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        execution: An execution's test run configuration.
> +        result: The top level result object.
>       """
>       dts_logger.info(
>           f"Running execution with SUT '{execution.system_under_test_node.name}'."
> @@ -150,8 +215,18 @@ def _run_build_target(
>       execution: ExecutionConfiguration,
>       execution_result: ExecutionResult,
>   ) -> None:
> -    """
> -    Run the given build target.
> +    """Run the given build target.
> +
> +    This involves running the build target setup as well as running all test suites
> +    in the given execution the build target is defined in.
> +    After that, build target teardown is run.
> +
> +    Args:
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        build_target: A build target's test run configuration.
> +        execution: The build target's execution's test run configuration.
> +        execution_result: The execution level result object associated with the execution.
>       """
>       dts_logger.info(f"Running build target '{build_target.name}'.")
>       build_target_result = execution_result.add_build_target(build_target)
> @@ -183,10 +258,17 @@ def _run_all_suites(
>       execution: ExecutionConfiguration,
>       build_target_result: BuildTargetResult,
>   ) -> None:
> -    """
> -    Use the given build_target to run execution's test suites
> -    with possibly only a subset of test cases.
> -    If no subset is specified, run all test cases.
> +    """Run the execution's (possibly a subset) test suites using the current build_target.
> +
> +    The function assumes the build target we're testing has already been built on the SUT node.
> +    The current build target thus corresponds to the current DPDK build present on the SUT node.
> +
> +    Args:
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        execution: The execution's test run configuration associated with the current build target.
> +        build_target_result: The build target level result object associated
> +            with the current build target.
>       """
>       end_build_target = False
>       if not execution.skip_smoke_tests:
> @@ -215,16 +297,22 @@ def _run_single_suite(
>       build_target_result: BuildTargetResult,
>       test_suite_config: TestSuiteConfig,
>   ) -> None:
> -    """Runs a single test suite.
> +    """Run all test suite in a single test suite module.
> +
> +    The function assumes the build target we're testing has already been built on the SUT node.
> +    The current build target thus corresponds to the current DPDK build present on the SUT node.
>   
>       Args:
> -        sut_node: Node to run tests on.
> -        execution: Execution the test case belongs to.
> -        build_target_result: Build target configuration test case is run on
> -        test_suite_config: Test suite configuration
> +        sut_node: The execution's SUT node.
> +        tg_node: The execution's TG node.
> +        execution: The execution's test run configuration associated with the current build target.
> +        build_target_result: The build target level result object associated
> +            with the current build target.
> +        test_suite_config: Test suite test run configuration specifying the test suite module
> +            and possibly a subset of test cases of test suites in that module.
>   
>       Raises:
> -        BlockingTestSuiteError: If a test suite that was marked as blocking fails.
> +        BlockingTestSuiteError: If a blocking test suite fails.
>       """
>       try:
>           full_suite_path = f"tests.TestSuite_{test_suite_config.test_suite}"
> @@ -248,9 +336,7 @@ def _run_single_suite(
>   
>   
>   def _exit_dts() -> None:
> -    """
> -    Process all errors and exit with the proper exit code.
> -    """
> +    """Process all errors and exit with the proper exit code."""
>       result.process()
>   
>       if dts_logger:
> diff --git a/dts/main.py b/dts/main.py
> index 5d4714b0c3..f703615d11 100755
> --- a/dts/main.py
> +++ b/dts/main.py
> @@ -4,9 +4,7 @@
>   # Copyright(c) 2022 PANTHEON.tech s.r.o.
>   # Copyright(c) 2022 University of New Hampshire
>   
> -"""
> -A test framework for testing DPDK.
> -"""
> +"""The DTS executable."""
>   
>   import logging
>   
> @@ -17,6 +15,10 @@ def main() -> None:
>       """Set DTS settings, then run DTS.
>   
>       The DTS settings are taken from the command line arguments and the environment variables.
> +    The settings object is stored in the module-level variable settings.SETTINGS which the entire
> +    framework uses. After importing the module (or the variable), any changes to the variable are
> +    not going to be reflected without a re-import. This means that the SETTINGS variable must
> +    be modified before the settings module is imported anywhere else in the framework.
>       """
>       settings.SETTINGS = settings.get_settings()
>       from framework import dts
  Nit: copyright notice update in main
  
Juraj Linkeš Nov. 21, 2023, 9:10 a.m. UTC | #4
On Mon, Nov 20, 2023 at 6:43 PM Yoan Picchi <yoan.picchi@foss.arm.com> wrote:
>
> On 11/15/23 13:09, Juraj Linkeš wrote:
> > Format according to the Google format and PEP257, with slight
> > deviations.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > ---
> >   dts/framework/dts.py | 128 ++++++++++++++++++++++++++++++++++++-------
> >   dts/main.py          |   8 ++-
> >   2 files changed, 112 insertions(+), 24 deletions(-)
> >
> > diff --git a/dts/framework/dts.py b/dts/framework/dts.py
> > index 4c7fb0c40a..331fed7dc4 100644
> > --- a/dts/framework/dts.py
> > +++ b/dts/framework/dts.py
> > @@ -3,6 +3,33 @@
> >   # Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
> >   # Copyright(c) 2022-2023 University of New Hampshire
> >
> > +r"""Test suite runner module.
>
> Is the r before the docstring intended?
>

Yes, this is because of :class:`~.framework.exception.DTSError`\s. Any
alphabetical characters after backticks must be escaped for Sphinx to
interpret the string correctly and the way to do that is to make the
string raw (with r before the string).

> > +
> > +A DTS run is split into stages:
> > +
> > +    #. Execution stage,
> > +    #. Build target stage,
> > +    #. Test suite stage,
> > +    #. Test case stage.
> > +
> > +The module is responsible for running tests on testbeds defined in the test run configuration.
> > +Each setup or teardown of each stage is recorded in a :class:`~framework.test_result.DTSResult` or
> > +one of its subclasses. The test case results are also recorded.
> > +
> > +If an error occurs, the current stage is aborted, the error is recorded and the run continues in
> > +the next iteration of the same stage. The return code is the highest `severity` of all
> > +:class:`~.framework.exception.DTSError`\s.
>
> Is the . before the classname intended? considering the previous one
> doesn't have one. (I've not yet built the doc to check if it affect the
> rendered doc)
>

Good catch. Not only is the dot suspect, but I looked at all
references starting with the framework dir and the ones that refer to
files in the same directory don't have to specify the full path if
starting with a dot, such as:
~framework.test_result.DTSResult -> ~.test_result.DTSResult
~.framework.exception.DTSError -> ~.exception.DTSError

test_result and exception are in the same dir as dts.py, so the above
work. I'll make these changes in other files as well.

> > +
> > +Example:
> > +    An error occurs in a build target setup. The current build target is aborted and the run
> > +    continues with the next build target. If the errored build target was the last one in the given
> > +    execution, the next execution begins.
> > +
> > +Attributes:
> > +    dts_logger: The logger instance used in this module.
> > +    result: The top level result used in the module.
> > +"""
> > +
> >   import sys
> >
> >   from .config import (
> > @@ -23,9 +50,38 @@
> >
> >
> >   def run_all() -> None:
> > -    """
> > -    The main process of DTS. Runs all build targets in all executions from the main
> > -    config file.
> > +    """Run all build targets in all executions from the test run configuration.
> > +
> > +    Before running test suites, executions and build targets are first set up.
> > +    The executions and build targets defined in the test run configuration are iterated over.
> > +    The executions define which tests to run and where to run them and build targets define
> > +    the DPDK build setup.
> > +
> > +    The tests suites are set up for each execution/build target tuple and each scheduled
> > +    test case within the test suite is set up, executed and torn down. After all test cases
> > +    have been executed, the test suite is torn down and the next build target will be tested.
> > +
> > +    All the nested steps look like this:
> > +
> > +        #. Execution setup
> > +
> > +            #. Build target setup
> > +
> > +                #. Test suite setup
> > +
> > +                    #. Test case setup
> > +                    #. Test case logic
> > +                    #. Test case teardown
> > +
> > +                #. Test suite teardown
> > +
> > +            #. Build target teardown
> > +
> > +        #. Execution teardown
> > +
> > +    The test cases are filtered according to the specification in the test run configuration and
> > +    the :option:`--test-cases` command line argument or
> > +    the :envvar:`DTS_TESTCASES` environment variable.
> >       """
> >       global dts_logger
> >       global result
> > @@ -87,6 +143,8 @@ def run_all() -> None:
> >
> >
> >   def _check_dts_python_version() -> None:
> > +    """Check the required Python version - v3.10."""
> > +
> >       def RED(text: str) -> str:
> >           return f"\u001B[31;1m{str(text)}\u001B[0m"
> >
> > @@ -111,9 +169,16 @@ def _run_execution(
> >       execution: ExecutionConfiguration,
> >       result: DTSResult,
> >   ) -> None:
> > -    """
> > -    Run the given execution. This involves running the execution setup as well as
> > -    running all build targets in the given execution.
> > +    """Run the given execution.
> > +
> > +    This involves running the execution setup as well as running all build targets
> > +    in the given execution. After that, execution teardown is run.
> > +
> > +    Args:
> > +        sut_node: The execution's SUT node.
> > +        tg_node: The execution's TG node.
> > +        execution: An execution's test run configuration.
> > +        result: The top level result object.
> >       """
> >       dts_logger.info(
> >           f"Running execution with SUT '{execution.system_under_test_node.name}'."
> > @@ -150,8 +215,18 @@ def _run_build_target(
> >       execution: ExecutionConfiguration,
> >       execution_result: ExecutionResult,
> >   ) -> None:
> > -    """
> > -    Run the given build target.
> > +    """Run the given build target.
> > +
> > +    This involves running the build target setup as well as running all test suites
> > +    in the given execution the build target is defined in.
> > +    After that, build target teardown is run.
> > +
> > +    Args:
> > +        sut_node: The execution's SUT node.
> > +        tg_node: The execution's TG node.
> > +        build_target: A build target's test run configuration.
> > +        execution: The build target's execution's test run configuration.
> > +        execution_result: The execution level result object associated with the execution.
> >       """
> >       dts_logger.info(f"Running build target '{build_target.name}'.")
> >       build_target_result = execution_result.add_build_target(build_target)
> > @@ -183,10 +258,17 @@ def _run_all_suites(
> >       execution: ExecutionConfiguration,
> >       build_target_result: BuildTargetResult,
> >   ) -> None:
> > -    """
> > -    Use the given build_target to run execution's test suites
> > -    with possibly only a subset of test cases.
> > -    If no subset is specified, run all test cases.
> > +    """Run the execution's (possibly a subset) test suites using the current build_target.
> > +
> > +    The function assumes the build target we're testing has already been built on the SUT node.
> > +    The current build target thus corresponds to the current DPDK build present on the SUT node.
> > +
> > +    Args:
> > +        sut_node: The execution's SUT node.
> > +        tg_node: The execution's TG node.
> > +        execution: The execution's test run configuration associated with the current build target.
> > +        build_target_result: The build target level result object associated
> > +            with the current build target.
> >       """
> >       end_build_target = False
> >       if not execution.skip_smoke_tests:
> > @@ -215,16 +297,22 @@ def _run_single_suite(
> >       build_target_result: BuildTargetResult,
> >       test_suite_config: TestSuiteConfig,
> >   ) -> None:
> > -    """Runs a single test suite.
> > +    """Run all test suite in a single test suite module.
> > +
> > +    The function assumes the build target we're testing has already been built on the SUT node.
> > +    The current build target thus corresponds to the current DPDK build present on the SUT node.
> >
> >       Args:
> > -        sut_node: Node to run tests on.
> > -        execution: Execution the test case belongs to.
> > -        build_target_result: Build target configuration test case is run on
> > -        test_suite_config: Test suite configuration
> > +        sut_node: The execution's SUT node.
> > +        tg_node: The execution's TG node.
> > +        execution: The execution's test run configuration associated with the current build target.
> > +        build_target_result: The build target level result object associated
> > +            with the current build target.
> > +        test_suite_config: Test suite test run configuration specifying the test suite module
> > +            and possibly a subset of test cases of test suites in that module.
> >
> >       Raises:
> > -        BlockingTestSuiteError: If a test suite that was marked as blocking fails.
> > +        BlockingTestSuiteError: If a blocking test suite fails.
> >       """
> >       try:
> >           full_suite_path = f"tests.TestSuite_{test_suite_config.test_suite}"
> > @@ -248,9 +336,7 @@ def _run_single_suite(
> >
> >
> >   def _exit_dts() -> None:
> > -    """
> > -    Process all errors and exit with the proper exit code.
> > -    """
> > +    """Process all errors and exit with the proper exit code."""
> >       result.process()
> >
> >       if dts_logger:
> > diff --git a/dts/main.py b/dts/main.py
> > index 5d4714b0c3..f703615d11 100755
> > --- a/dts/main.py
> > +++ b/dts/main.py
> > @@ -4,9 +4,7 @@
> >   # Copyright(c) 2022 PANTHEON.tech s.r.o.
> >   # Copyright(c) 2022 University of New Hampshire
> >
> > -"""
> > -A test framework for testing DPDK.
> > -"""
> > +"""The DTS executable."""
> >
> >   import logging
> >
> > @@ -17,6 +15,10 @@ def main() -> None:
> >       """Set DTS settings, then run DTS.
> >
> >       The DTS settings are taken from the command line arguments and the environment variables.
> > +    The settings object is stored in the module-level variable settings.SETTINGS which the entire
> > +    framework uses. After importing the module (or the variable), any changes to the variable are
> > +    not going to be reflected without a re-import. This means that the SETTINGS variable must
> > +    be modified before the settings module is imported anywhere else in the framework.
> >       """
> >       settings.SETTINGS = settings.get_settings()
> >       from framework import dts
>   Nit: copyright notice update in main

Nice catch again. Thanks.
  

Patch

diff --git a/dts/framework/dts.py b/dts/framework/dts.py
index 4c7fb0c40a..331fed7dc4 100644
--- a/dts/framework/dts.py
+++ b/dts/framework/dts.py
@@ -3,6 +3,33 @@ 
 # Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
 # Copyright(c) 2022-2023 University of New Hampshire
 
+r"""Test suite runner module.
+
+A DTS run is split into stages:
+
+    #. Execution stage,
+    #. Build target stage,
+    #. Test suite stage,
+    #. Test case stage.
+
+The module is responsible for running tests on testbeds defined in the test run configuration.
+Each setup or teardown of each stage is recorded in a :class:`~framework.test_result.DTSResult` or
+one of its subclasses. The test case results are also recorded.
+
+If an error occurs, the current stage is aborted, the error is recorded and the run continues in
+the next iteration of the same stage. The return code is the highest `severity` of all
+:class:`~.framework.exception.DTSError`\s.
+
+Example:
+    An error occurs in a build target setup. The current build target is aborted and the run
+    continues with the next build target. If the errored build target was the last one in the given
+    execution, the next execution begins.
+
+Attributes:
+    dts_logger: The logger instance used in this module.
+    result: The top level result used in the module.
+"""
+
 import sys
 
 from .config import (
@@ -23,9 +50,38 @@ 
 
 
 def run_all() -> None:
-    """
-    The main process of DTS. Runs all build targets in all executions from the main
-    config file.
+    """Run all build targets in all executions from the test run configuration.
+
+    Before running test suites, executions and build targets are first set up.
+    The executions and build targets defined in the test run configuration are iterated over.
+    The executions define which tests to run and where to run them and build targets define
+    the DPDK build setup.
+
+    The tests suites are set up for each execution/build target tuple and each scheduled
+    test case within the test suite is set up, executed and torn down. After all test cases
+    have been executed, the test suite is torn down and the next build target will be tested.
+
+    All the nested steps look like this:
+
+        #. Execution setup
+
+            #. Build target setup
+
+                #. Test suite setup
+
+                    #. Test case setup
+                    #. Test case logic
+                    #. Test case teardown
+
+                #. Test suite teardown
+
+            #. Build target teardown
+
+        #. Execution teardown
+
+    The test cases are filtered according to the specification in the test run configuration and
+    the :option:`--test-cases` command line argument or
+    the :envvar:`DTS_TESTCASES` environment variable.
     """
     global dts_logger
     global result
@@ -87,6 +143,8 @@  def run_all() -> None:
 
 
 def _check_dts_python_version() -> None:
+    """Check the required Python version - v3.10."""
+
     def RED(text: str) -> str:
         return f"\u001B[31;1m{str(text)}\u001B[0m"
 
@@ -111,9 +169,16 @@  def _run_execution(
     execution: ExecutionConfiguration,
     result: DTSResult,
 ) -> None:
-    """
-    Run the given execution. This involves running the execution setup as well as
-    running all build targets in the given execution.
+    """Run the given execution.
+
+    This involves running the execution setup as well as running all build targets
+    in the given execution. After that, execution teardown is run.
+
+    Args:
+        sut_node: The execution's SUT node.
+        tg_node: The execution's TG node.
+        execution: An execution's test run configuration.
+        result: The top level result object.
     """
     dts_logger.info(
         f"Running execution with SUT '{execution.system_under_test_node.name}'."
@@ -150,8 +215,18 @@  def _run_build_target(
     execution: ExecutionConfiguration,
     execution_result: ExecutionResult,
 ) -> None:
-    """
-    Run the given build target.
+    """Run the given build target.
+
+    This involves running the build target setup as well as running all test suites
+    in the given execution the build target is defined in.
+    After that, build target teardown is run.
+
+    Args:
+        sut_node: The execution's SUT node.
+        tg_node: The execution's TG node.
+        build_target: A build target's test run configuration.
+        execution: The build target's execution's test run configuration.
+        execution_result: The execution level result object associated with the execution.
     """
     dts_logger.info(f"Running build target '{build_target.name}'.")
     build_target_result = execution_result.add_build_target(build_target)
@@ -183,10 +258,17 @@  def _run_all_suites(
     execution: ExecutionConfiguration,
     build_target_result: BuildTargetResult,
 ) -> None:
-    """
-    Use the given build_target to run execution's test suites
-    with possibly only a subset of test cases.
-    If no subset is specified, run all test cases.
+    """Run the execution's (possibly a subset) test suites using the current build_target.
+
+    The function assumes the build target we're testing has already been built on the SUT node.
+    The current build target thus corresponds to the current DPDK build present on the SUT node.
+
+    Args:
+        sut_node: The execution's SUT node.
+        tg_node: The execution's TG node.
+        execution: The execution's test run configuration associated with the current build target.
+        build_target_result: The build target level result object associated
+            with the current build target.
     """
     end_build_target = False
     if not execution.skip_smoke_tests:
@@ -215,16 +297,22 @@  def _run_single_suite(
     build_target_result: BuildTargetResult,
     test_suite_config: TestSuiteConfig,
 ) -> None:
-    """Runs a single test suite.
+    """Run all test suite in a single test suite module.
+
+    The function assumes the build target we're testing has already been built on the SUT node.
+    The current build target thus corresponds to the current DPDK build present on the SUT node.
 
     Args:
-        sut_node: Node to run tests on.
-        execution: Execution the test case belongs to.
-        build_target_result: Build target configuration test case is run on
-        test_suite_config: Test suite configuration
+        sut_node: The execution's SUT node.
+        tg_node: The execution's TG node.
+        execution: The execution's test run configuration associated with the current build target.
+        build_target_result: The build target level result object associated
+            with the current build target.
+        test_suite_config: Test suite test run configuration specifying the test suite module
+            and possibly a subset of test cases of test suites in that module.
 
     Raises:
-        BlockingTestSuiteError: If a test suite that was marked as blocking fails.
+        BlockingTestSuiteError: If a blocking test suite fails.
     """
     try:
         full_suite_path = f"tests.TestSuite_{test_suite_config.test_suite}"
@@ -248,9 +336,7 @@  def _run_single_suite(
 
 
 def _exit_dts() -> None:
-    """
-    Process all errors and exit with the proper exit code.
-    """
+    """Process all errors and exit with the proper exit code."""
     result.process()
 
     if dts_logger:
diff --git a/dts/main.py b/dts/main.py
index 5d4714b0c3..f703615d11 100755
--- a/dts/main.py
+++ b/dts/main.py
@@ -4,9 +4,7 @@ 
 # Copyright(c) 2022 PANTHEON.tech s.r.o.
 # Copyright(c) 2022 University of New Hampshire
 
-"""
-A test framework for testing DPDK.
-"""
+"""The DTS executable."""
 
 import logging
 
@@ -17,6 +15,10 @@  def main() -> None:
     """Set DTS settings, then run DTS.
 
     The DTS settings are taken from the command line arguments and the environment variables.
+    The settings object is stored in the module-level variable settings.SETTINGS which the entire
+    framework uses. After importing the module (or the variable), any changes to the variable are
+    not going to be reflected without a re-import. This means that the SETTINGS variable must
+    be modified before the settings module is imported anywhere else in the framework.
     """
     settings.SETTINGS = settings.get_settings()
     from framework import dts