DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jeremy Spewock <jspewock@iol.unh.edu>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com,
	probb@iol.unh.edu,  paul.szczepanek@arm.com,
	Luca.Vizzarro@arm.com, dev@dpdk.org
Subject: Re: [PATCH v2 3/7] dts: filter test suites in executions
Date: Mon, 12 Feb 2024 11:44:45 -0500	[thread overview]
Message-ID: <CAAA20UT1JF-onMA7GtUnfv27EkGEdCwjJHCEvvCYypnSvqc6Uw@mail.gmail.com> (raw)
In-Reply-To: <20240206145716.71435-4-juraj.linkes@pantheon.tech>

On Tue, Feb 6, 2024 at 9:57 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> We're currently filtering which test cases to run after some setup
> steps, such as DPDK build, have already been taken. This prohibits us to
> mark the test suites and cases that were supposed to be run as blocked
> when an earlier setup fails, as that information is not available at
> that time.
>
> To remedy this, move the filtering to the beginning of each execution.
> This is the first action taken in each execution and if we can't filter
> the test cases, such as due to invalid inputs, we abort the whole
> execution. No test suites nor cases will be marked as blocked as we
> don't know which were supposed to be run.
>
> On top of that, the filtering takes place in the TestSuite class, which
> should only concern itself with test suite and test case logic, not the
> processing behind the scenes. The logic has been moved to DTSRunner
> which should do all the processing needed to run test suites.
>
> The filtering itself introduces a few changes/assumptions which are more
> sensible than before:
> 1. Assumption: There is just one TestSuite child class in each test
>    suite module. This was an implicit assumption before as we couldn't
>    specify the TestSuite classes in the test run configuration, just the
>    modules. The name of the TestSuite child class starts with "Test" and
>    then corresponds to the name of the module with CamelCase naming.
> 2. Unknown test cases specified both in the test run configuration and
>    the environment variable/command line argument are no longer silently
>    ignored. This is a quality of life improvement for users, as they
>    could easily be not aware of the silent ignoration.
>
> Also, a change in the code results in pycodestyle warning and error:
> [E] E203 whitespace before ':'
> [W] W503 line break before binary operator
>
> These two are not PEP8 compliant, so they're disabled.
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  dts/framework/config/__init__.py           |  24 +-
>  dts/framework/config/conf_yaml_schema.json |   2 +-
>  dts/framework/runner.py                    | 426 +++++++++++++++------
>  dts/framework/settings.py                  |   3 +-
>  dts/framework/test_result.py               |  34 ++
>  dts/framework/test_suite.py                |  85 +---
>  dts/pyproject.toml                         |   3 +
>  dts/tests/TestSuite_smoke_tests.py         |   2 +-
>  8 files changed, 382 insertions(+), 197 deletions(-)
>
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index 62eded7f04..c6a93b3b89 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -36,7 +36,7 @@
>  import json
>  import os.path
>  import pathlib
> -from dataclasses import dataclass
> +from dataclasses import dataclass, fields
>  from enum import auto, unique
>  from typing import Union
>
> @@ -506,6 +506,28 @@ def from_dict(
>              vdevs=vdevs,
>          )
>
> +    def copy_and_modify(self, **kwargs) -> "ExecutionConfiguration":
> +        """Create a shallow copy with any of the fields modified.
> +
> +        The only new data are those passed to this method.
> +        The rest are copied from the object's fields calling the method.
> +
> +        Args:
> +            **kwargs: The names and types of keyword arguments are defined
> +                by the fields of the :class:`ExecutionConfiguration` class.
> +
> +        Returns:
> +            The copied and modified execution configuration.
> +        """
> +        new_config = {}
> +        for field in fields(self):
> +            if field.name in kwargs:
> +                new_config[field.name] = kwargs[field.name]
> +            else:
> +                new_config[field.name] = getattr(self, field.name)
> +
> +        return ExecutionConfiguration(**new_config)
> +
>
>  @dataclass(slots=True, frozen=True)
>  class Configuration:
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index 84e45fe3c2..051b079fe4 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -197,7 +197,7 @@
>          },
>          "cases": {
>            "type": "array",
> -          "description": "If specified, only this subset of test suite's test cases will be run. Unknown test cases will be silently ignored.",
> +          "description": "If specified, only this subset of test suite's test cases will be run.",
>            "items": {
>              "type": "string"
>            },
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index 933685d638..3e95cf9e26 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -17,17 +17,27 @@
>  and the test case stage runs test cases individually.
>  """
>
> +import importlib
> +import inspect
>  import logging
> +import re
>  import sys
>  from types import MethodType
> +from typing import Iterable
>
>  from .config import (
>      BuildTargetConfiguration,
> +    Configuration,
>      ExecutionConfiguration,
>      TestSuiteConfig,
>      load_config,
>  )
> -from .exception import BlockingTestSuiteError, SSHTimeoutError, TestCaseVerifyError
> +from .exception import (
> +    BlockingTestSuiteError,
> +    ConfigurationError,
> +    SSHTimeoutError,
> +    TestCaseVerifyError,
> +)
>  from .logger import DTSLOG, getLogger
>  from .settings import SETTINGS
>  from .test_result import (
> @@ -37,8 +47,9 @@
>      Result,
>      TestCaseResult,
>      TestSuiteResult,
> +    TestSuiteWithCases,
>  )
> -from .test_suite import TestSuite, get_test_suites
> +from .test_suite import TestSuite
>  from .testbed_model import SutNode, TGNode
>
>
> @@ -59,13 +70,23 @@ class DTSRunner:
>          given execution, the next execution begins.
>      """
>
> +    _configuration: Configuration
>      _logger: DTSLOG
>      _result: DTSResult
> +    _test_suite_class_prefix: str
> +    _test_suite_module_prefix: str
> +    _func_test_case_regex: str
> +    _perf_test_case_regex: str
>
>      def __init__(self):
> -        """Initialize the instance with logger and result."""
> +        """Initialize the instance with configuration, logger, result and string constants."""
> +        self._configuration = load_config()
>          self._logger = getLogger("DTSRunner")
>          self._result = DTSResult(self._logger)
> +        self._test_suite_class_prefix = "Test"
> +        self._test_suite_module_prefix = "tests.TestSuite_"
> +        self._func_test_case_regex = r"test_(?!perf_)"
> +        self._perf_test_case_regex = r"test_perf_"
>
>      def run(self):
>          """Run all build targets in all executions from the test run configuration.
> @@ -106,29 +127,28 @@ def run(self):
>          try:
>              # check the python version of the server that runs dts
>              self._check_dts_python_version()
> +            self._result.update_setup(Result.PASS)
>
>              # for all Execution sections
> -            for execution in load_config().executions:
> -                sut_node = sut_nodes.get(execution.system_under_test_node.name)
> -                tg_node = tg_nodes.get(execution.traffic_generator_node.name)
> -
> +            for execution in self._configuration.executions:
> +                self._logger.info(
> +                    f"Running execution with SUT '{execution.system_under_test_node.name}'."
> +                )
> +                execution_result = self._result.add_execution(execution.system_under_test_node)
>                  try:
> -                    if not sut_node:
> -                        sut_node = SutNode(execution.system_under_test_node)
> -                        sut_nodes[sut_node.name] = sut_node
> -                    if not tg_node:
> -                        tg_node = TGNode(execution.traffic_generator_node)
> -                        tg_nodes[tg_node.name] = tg_node
> -                    self._result.update_setup(Result.PASS)
> +                    test_suites_with_cases = self._get_test_suites_with_cases(
> +                        execution.test_suites, execution.func, execution.perf
> +                    )
>                  except Exception as e:
> -                    failed_node = execution.system_under_test_node.name
> -                    if sut_node:
> -                        failed_node = execution.traffic_generator_node.name
> -                    self._logger.exception(f"The Creation of node {failed_node} failed.")
> -                    self._result.update_setup(Result.FAIL, e)
> +                    self._logger.exception(
> +                        f"Invalid test suite configuration found: " f"{execution.test_suites}."
> +                    )
> +                    execution_result.update_setup(Result.FAIL, e)
>
>                  else:
> -                    self._run_execution(sut_node, tg_node, execution)
> +                    self._connect_nodes_and_run_execution(
> +                        sut_nodes, tg_nodes, execution, execution_result, test_suites_with_cases
> +                    )
>
>          except Exception as e:
>              self._logger.exception("An unexpected error has occurred.")
> @@ -163,11 +183,204 @@ def _check_dts_python_version(self) -> None:
>              )
>              self._logger.warning("Please use Python >= 3.10 instead.")
>
> +    def _get_test_suites_with_cases(
> +        self,
> +        test_suite_configs: list[TestSuiteConfig],
> +        func: bool,
> +        perf: bool,
> +    ) -> list[TestSuiteWithCases]:
> +        """Test suites with test cases discovery.
> +
> +        The test suites with test cases defined in the user configuration are discovered
> +        and stored for future use so that we don't import the modules twice and so that
> +        the list of test suites with test cases is available for recording right away.
> +
> +        Args:
> +            test_suite_configs: Test suite configurations.
> +            func: Whether to include functional test cases in the final list.
> +            perf: Whether to include performance test cases in the final list.
> +
> +        Returns:
> +            The discovered test suites, each with test cases.
> +        """
> +        test_suites_with_cases = []
> +
> +        for test_suite_config in test_suite_configs:
> +            test_suite_class = self._get_test_suite_class(test_suite_config.test_suite)
> +            test_cases = []
> +            func_test_cases, perf_test_cases = self._filter_test_cases(
> +                test_suite_class, set(test_suite_config.test_cases + SETTINGS.test_cases)
> +            )
> +            if func:
> +                test_cases.extend(func_test_cases)
> +            if perf:
> +                test_cases.extend(perf_test_cases)
> +
> +            test_suites_with_cases.append(
> +                TestSuiteWithCases(test_suite_class=test_suite_class, test_cases=test_cases)
> +            )
> +
> +        return test_suites_with_cases
> +
> +    def _get_test_suite_class(self, test_suite_name: str) -> type[TestSuite]:
> +        """Find the :class:`TestSuite` class with `test_suite_name` in the corresponding module.
> +
> +        The method assumes that the :class:`TestSuite` class starts
> +        with `self._test_suite_class_prefix`,
> +        continuing with `test_suite_name` with CamelCase convention.
> +        It also assumes there's only one test suite in each module and the module name
> +        is `test_suite_name` prefixed with `self._test_suite_module_prefix`.
> +
> +        The CamelCase convention is not tested, only lowercase strings are compared.
> +
> +        Args:
> +            test_suite_name: The name of the test suite to find.
> +
> +        Returns:
> +            The found test suite.
> +
> +        Raises:
> +            ConfigurationError: If the corresponding module is not found or
> +                a valid :class:`TestSuite` is not found in the module.
> +        """
> +
> +        def is_test_suite(object) -> bool:
> +            """Check whether `object` is a :class:`TestSuite`.
> +
> +            The `object` is a subclass of :class:`TestSuite`, but not :class:`TestSuite` itself.
> +
> +            Args:
> +                object: The object to be checked.
> +
> +            Returns:
> +                :data:`True` if `object` is a subclass of `TestSuite`.
> +            """
> +            try:
> +                if issubclass(object, TestSuite) and object is not TestSuite:
> +                    return True
> +            except TypeError:
> +                return False
> +            return False
> +
> +        testsuite_module_path = f"{self._test_suite_module_prefix}{test_suite_name}"
> +        try:
> +            test_suite_module = importlib.import_module(testsuite_module_path)
> +        except ModuleNotFoundError as e:
> +            raise ConfigurationError(
> +                f"Test suite module '{testsuite_module_path}' not found."
> +            ) from e
> +
> +        lowercase_suite_name = test_suite_name.replace("_", "").lower()
> +        for class_name, class_obj in inspect.getmembers(test_suite_module, is_test_suite):
> +            if (
> +                class_name.startswith(self._test_suite_class_prefix)
> +                and lowercase_suite_name == class_name[len(self._test_suite_class_prefix) :].lower()
> +            ):

Would it be simpler to instead just make lowercase_suite_name =
f"{self._test_suite_class_prefix}{test_suite_name.replace("_",
"").lower()}" so that you can just directly compare class_name ==
lowercase_suite_name? Both ways should have the exact same result of
course so it isn't important, I was just curious.

> +                return class_obj
> +        raise ConfigurationError(
> +            f"Couldn't find any valid test suites in {test_suite_module.__name__}."
> +        )
> +
> +    def _filter_test_cases(
> +        self, test_suite_class: type[TestSuite], test_cases_to_run: set[str]
> +    ) -> tuple[list[MethodType], list[MethodType]]:
> +        """Filter `test_cases_to_run` from `test_suite_class`.
> +
> +        There are two rounds of filtering if `test_cases_to_run` is not empty.
> +        The first filters `test_cases_to_run` from all methods of `test_suite_class`.
> +        Then the methods are separated into functional and performance test cases.
> +        If a method doesn't match neither the functional nor performance name prefix, it's an error.

I think this is a double negative but could be either "if a method
doesn't match either ... or ..." or "if a method matches neither ...
nor ...". I have a small preference to the second of the two options
though because the "neither" makes the negative more clear in my mind.

> +
> +        Args:
> +            test_suite_class: The class of the test suite.
> +            test_cases_to_run: Test case names to filter from `test_suite_class`.
> +                If empty, return all matching test cases.
> +
> +        Returns:
> +            A list of test case methods that should be executed.
> +
> +        Raises:
> +            ConfigurationError: If a test case from `test_cases_to_run` is not found
> +                or it doesn't match either the functional nor performance name prefix.
> +        """
> +        func_test_cases = []
> +        perf_test_cases = []
> +        name_method_tuples = inspect.getmembers(test_suite_class, inspect.isfunction)
> +        if test_cases_to_run:
> +            name_method_tuples = [
> +                (name, method) for name, method in name_method_tuples if name in test_cases_to_run
> +            ]
> +            if len(name_method_tuples) < len(test_cases_to_run):
> +                missing_test_cases = test_cases_to_run - {name for name, _ in name_method_tuples}
> +                raise ConfigurationError(
> +                    f"Test cases {missing_test_cases} not found among methods "
> +                    f"of {test_suite_class.__name__}."
> +                )
> +
> +        for test_case_name, test_case_method in name_method_tuples:
> +            if re.match(self._func_test_case_regex, test_case_name):
> +                func_test_cases.append(test_case_method)
> +            elif re.match(self._perf_test_case_regex, test_case_name):
> +                perf_test_cases.append(test_case_method)
> +            elif test_cases_to_run:
> +                raise ConfigurationError(
> +                    f"Method '{test_case_name}' doesn't match neither "
> +                    f"a functional nor a performance test case name."

Same thing here with the double negative.



> +                )
> +
> +        return func_test_cases, perf_test_cases
> +
> +    def _connect_nodes_and_run_execution(
> +        self,
> +        sut_nodes: dict[str, SutNode],
> +        tg_nodes: dict[str, TGNode],
> +        execution: ExecutionConfiguration,
> +        execution_result: ExecutionResult,
> +        test_suites_with_cases: Iterable[TestSuiteWithCases],
> +    ) -> None:
> +        """Connect nodes, then continue to run the given execution.
> +
> +        Connect the :class:`SutNode` and the :class:`TGNode` of this `execution`.
> +        If either has already been connected, it's going to be in either `sut_nodes` or `tg_nodes`,
> +        respectively.
> +        If not, connect and add the node to the respective `sut_nodes` or `tg_nodes` :class:`dict`.
> +
> +        Args:
> +            sut_nodes: A dictionary storing connected/to be connected SUT nodes.
> +            tg_nodes: A dictionary storing connected/to be connected TG nodes.
> +            execution: An execution's test run configuration.
> +            execution_result: The execution's result.
> +            test_suites_with_cases: The test suites with test cases to run.
> +        """
> +        sut_node = sut_nodes.get(execution.system_under_test_node.name)
> +        tg_node = tg_nodes.get(execution.traffic_generator_node.name)
> +
> +        try:
> +            if not sut_node:
> +                sut_node = SutNode(execution.system_under_test_node)
> +                sut_nodes[sut_node.name] = sut_node
> +            if not tg_node:
> +                tg_node = TGNode(execution.traffic_generator_node)
> +                tg_nodes[tg_node.name] = tg_node
> +        except Exception as e:
> +            failed_node = execution.system_under_test_node.name
> +            if sut_node:
> +                failed_node = execution.traffic_generator_node.name
> +            self._logger.exception(f"The Creation of node {failed_node} failed.")
> +            execution_result.update_setup(Result.FAIL, e)
> +
> +        else:
> +            self._run_execution(
> +                sut_node, tg_node, execution, execution_result, test_suites_with_cases
> +            )
> +
>      def _run_execution(
>          self,
>          sut_node: SutNode,
>          tg_node: TGNode,
>          execution: ExecutionConfiguration,
> +        execution_result: ExecutionResult,
> +        test_suites_with_cases: Iterable[TestSuiteWithCases],
>      ) -> None:
>          """Run the given execution.
>
> @@ -178,11 +391,11 @@ def _run_execution(
>              sut_node: The execution's SUT node.
>              tg_node: The execution's TG node.
>              execution: An execution's test run configuration.
> +            execution_result: The execution's result.
> +            test_suites_with_cases: The test suites with test cases to run.
>          """
>          self._logger.info(f"Running execution with SUT '{execution.system_under_test_node.name}'.")
> -        execution_result = self._result.add_execution(sut_node.config)
>          execution_result.add_sut_info(sut_node.node_info)
> -
>          try:
>              sut_node.set_up_execution(execution)
>              execution_result.update_setup(Result.PASS)
> @@ -192,7 +405,10 @@ def _run_execution(
>
>          else:
>              for build_target in execution.build_targets:
> -                self._run_build_target(sut_node, tg_node, build_target, execution, execution_result)
> +                build_target_result = execution_result.add_build_target(build_target)
> +                self._run_build_target(
> +                    sut_node, tg_node, build_target, build_target_result, test_suites_with_cases
> +                )
>
>          finally:
>              try:
> @@ -207,8 +423,8 @@ def _run_build_target(
>          sut_node: SutNode,
>          tg_node: TGNode,
>          build_target: BuildTargetConfiguration,
> -        execution: ExecutionConfiguration,
> -        execution_result: ExecutionResult,
> +        build_target_result: BuildTargetResult,
> +        test_suites_with_cases: Iterable[TestSuiteWithCases],
>      ) -> None:
>          """Run the given build target.
>
> @@ -220,11 +436,11 @@ def _run_build_target(
>              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.
> +            build_target_result: The build target level result object associated
> +                with the current build target.
> +            test_suites_with_cases: The test suites with test cases to run.
>          """
>          self._logger.info(f"Running build target '{build_target.name}'.")
> -        build_target_result = execution_result.add_build_target(build_target)
>
>          try:
>              sut_node.set_up_build_target(build_target)
> @@ -236,7 +452,7 @@ def _run_build_target(
>              build_target_result.update_setup(Result.FAIL, e)
>
>          else:
> -            self._run_test_suites(sut_node, tg_node, execution, build_target_result)
> +            self._run_test_suites(sut_node, tg_node, build_target_result, test_suites_with_cases)
>
>          finally:
>              try:
> @@ -250,10 +466,10 @@ def _run_test_suites(
>          self,
>          sut_node: SutNode,
>          tg_node: TGNode,
> -        execution: ExecutionConfiguration,
>          build_target_result: BuildTargetResult,
> +        test_suites_with_cases: Iterable[TestSuiteWithCases],
>      ) -> None:
> -        """Run the execution's (possibly a subset of) test suites using the current build target.
> +        """Run `test_suites_with_cases` with the current build target.
>
>          The method 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.
> @@ -264,22 +480,20 @@ def _run_test_suites(
>          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.
> +            test_suites_with_cases: The test suites with test cases to run.
>          """
>          end_build_target = False
> -        if not execution.skip_smoke_tests:
> -            execution.test_suites[:0] = [TestSuiteConfig.from_dict("smoke_tests")]
> -        for test_suite_config in execution.test_suites:
> +        for test_suite_with_cases in test_suites_with_cases:
> +            test_suite_result = build_target_result.add_test_suite(
> +                test_suite_with_cases.test_suite_class.__name__
> +            )
>              try:
> -                self._run_test_suite_module(
> -                    sut_node, tg_node, execution, build_target_result, test_suite_config
> -                )
> +                self._run_test_suite(sut_node, tg_node, test_suite_result, test_suite_with_cases)
>              except BlockingTestSuiteError as e:
>                  self._logger.exception(
> -                    f"An error occurred within {test_suite_config.test_suite}. "
> +                    f"An error occurred within {test_suite_with_cases.test_suite_class.__name__}. "
>                      "Skipping build target..."
>                  )
>                  self._result.add_error(e)
> @@ -288,15 +502,14 @@ def _run_test_suites(
>              if end_build_target:
>                  break
>
> -    def _run_test_suite_module(
> +    def _run_test_suite(
>          self,
>          sut_node: SutNode,
>          tg_node: TGNode,
> -        execution: ExecutionConfiguration,
> -        build_target_result: BuildTargetResult,
> -        test_suite_config: TestSuiteConfig,
> +        test_suite_result: TestSuiteResult,
> +        test_suite_with_cases: TestSuiteWithCases,
>      ) -> None:
> -        """Set up, execute and tear down all test suites in a single test suite module.
> +        """Set up, execute and tear down `test_suite_with_cases`.
>
>          The method 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.
> @@ -306,92 +519,79 @@ def _run_test_suite_module(
>
>          Record the setup and the teardown and handle failures.
>
> -        The test cases to execute are discovered when creating the :class:`TestSuite` object.
> -
>          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.
> -            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.
> +            test_suite_result: The test suite level result object associated
> +                with the current test suite.
> +            test_suite_with_cases: The test suite with test cases to run.
>
>          Raises:
>              BlockingTestSuiteError: If a blocking test suite fails.
>          """
> +        test_suite_name = test_suite_with_cases.test_suite_class.__name__
> +        test_suite = test_suite_with_cases.test_suite_class(sut_node, tg_node)
>          try:
> -            full_suite_path = f"tests.TestSuite_{test_suite_config.test_suite}"
> -            test_suite_classes = get_test_suites(full_suite_path)
> -            suites_str = ", ".join((x.__name__ for x in test_suite_classes))
> -            self._logger.debug(f"Found test suites '{suites_str}' in '{full_suite_path}'.")
> +            self._logger.info(f"Starting test suite setup: {test_suite_name}")
> +            test_suite.set_up_suite()
> +            test_suite_result.update_setup(Result.PASS)
> +            self._logger.info(f"Test suite setup successful: {test_suite_name}")
>          except Exception as e:
> -            self._logger.exception("An error occurred when searching for test suites.")
> -            self._result.update_setup(Result.ERROR, e)
> +            self._logger.exception(f"Test suite setup ERROR: {test_suite_name}")
> +            test_suite_result.update_setup(Result.ERROR, e)
>
>          else:
> -            for test_suite_class in test_suite_classes:
> -                test_suite = test_suite_class(sut_node, tg_node, test_suite_config.test_cases)
> -
> -                test_suite_name = test_suite.__class__.__name__
> -                test_suite_result = build_target_result.add_test_suite(test_suite_name)
> -                try:
> -                    self._logger.info(f"Starting test suite setup: {test_suite_name}")
> -                    test_suite.set_up_suite()
> -                    test_suite_result.update_setup(Result.PASS)
> -                    self._logger.info(f"Test suite setup successful: {test_suite_name}")
> -                except Exception as e:
> -                    self._logger.exception(f"Test suite setup ERROR: {test_suite_name}")
> -                    test_suite_result.update_setup(Result.ERROR, e)
> -
> -                else:
> -                    self._execute_test_suite(execution.func, test_suite, test_suite_result)
> -
> -                finally:
> -                    try:
> -                        test_suite.tear_down_suite()
> -                        sut_node.kill_cleanup_dpdk_apps()
> -                        test_suite_result.update_teardown(Result.PASS)
> -                    except Exception as e:
> -                        self._logger.exception(f"Test suite teardown ERROR: {test_suite_name}")
> -                        self._logger.warning(
> -                            f"Test suite '{test_suite_name}' teardown failed, "
> -                            f"the next test suite may be affected."
> -                        )
> -                        test_suite_result.update_setup(Result.ERROR, e)
> -                    if len(test_suite_result.get_errors()) > 0 and test_suite.is_blocking:
> -                        raise BlockingTestSuiteError(test_suite_name)
> +            self._execute_test_suite(
> +                test_suite,
> +                test_suite_with_cases.test_cases,
> +                test_suite_result,
> +            )
> +        finally:
> +            try:
> +                test_suite.tear_down_suite()
> +                sut_node.kill_cleanup_dpdk_apps()
> +                test_suite_result.update_teardown(Result.PASS)
> +            except Exception as e:
> +                self._logger.exception(f"Test suite teardown ERROR: {test_suite_name}")
> +                self._logger.warning(
> +                    f"Test suite '{test_suite_name}' teardown failed, "
> +                    "the next test suite may be affected."
> +                )
> +                test_suite_result.update_setup(Result.ERROR, e)
> +            if len(test_suite_result.get_errors()) > 0 and test_suite.is_blocking:
> +                raise BlockingTestSuiteError(test_suite_name)
>
>      def _execute_test_suite(
> -        self, func: bool, test_suite: TestSuite, test_suite_result: TestSuiteResult
> +        self,
> +        test_suite: TestSuite,
> +        test_cases: Iterable[MethodType],
> +        test_suite_result: TestSuiteResult,
>      ) -> None:
> -        """Execute all discovered test cases in `test_suite`.
> +        """Execute all `test_cases` in `test_suite`.
>
>          If the :option:`--re-run` command line argument or the :envvar:`DTS_RERUN` environment
>          variable is set, in case of a test case failure, the test case will be executed again
>          until it passes or it fails that many times in addition of the first failure.
>
>          Args:
> -            func: Whether to execute functional test cases.
>              test_suite: The test suite object.
> +            test_cases: The list of test case methods.
>              test_suite_result: The test suite level result object associated
>                  with the current test suite.
>          """
> -        if func:
> -            for test_case_method in test_suite._get_functional_test_cases():
> -                test_case_name = test_case_method.__name__
> -                test_case_result = test_suite_result.add_test_case(test_case_name)
> -                all_attempts = SETTINGS.re_run + 1
> -                attempt_nr = 1
> +        for test_case_method in test_cases:
> +            test_case_name = test_case_method.__name__
> +            test_case_result = test_suite_result.add_test_case(test_case_name)
> +            all_attempts = SETTINGS.re_run + 1
> +            attempt_nr = 1
> +            self._run_test_case(test_suite, test_case_method, test_case_result)
> +            while not test_case_result and attempt_nr < all_attempts:
> +                attempt_nr += 1
> +                self._logger.info(
> +                    f"Re-running FAILED test case '{test_case_name}'. "
> +                    f"Attempt number {attempt_nr} out of {all_attempts}."
> +                )
>                  self._run_test_case(test_suite, test_case_method, test_case_result)
> -                while not test_case_result and attempt_nr < all_attempts:
> -                    attempt_nr += 1
> -                    self._logger.info(
> -                        f"Re-running FAILED test case '{test_case_name}'. "
> -                        f"Attempt number {attempt_nr} out of {all_attempts}."
> -                    )
> -                    self._run_test_case(test_suite, test_case_method, test_case_result)
>
>      def _run_test_case(
>          self,
> @@ -399,7 +599,7 @@ def _run_test_case(
>          test_case_method: MethodType,
>          test_case_result: TestCaseResult,
>      ) -> None:
> -        """Setup, execute and teardown a test case in `test_suite`.
> +        """Setup, execute and teardown `test_case_method` from `test_suite`.
>
>          Record the result of the setup and the teardown and handle failures.
>
> @@ -424,7 +624,7 @@ def _run_test_case(
>
>          else:
>              # run test case if setup was successful
> -            self._execute_test_case(test_case_method, test_case_result)
> +            self._execute_test_case(test_suite, test_case_method, test_case_result)
>
>          finally:
>              try:
> @@ -440,11 +640,15 @@ def _run_test_case(
>                  test_case_result.update(Result.ERROR)
>
>      def _execute_test_case(
> -        self, test_case_method: MethodType, test_case_result: TestCaseResult
> +        self,
> +        test_suite: TestSuite,
> +        test_case_method: MethodType,
> +        test_case_result: TestCaseResult,
>      ) -> None:
> -        """Execute one test case, record the result and handle failures.
> +        """Execute `test_case_method` from `test_suite`, record the result and handle failures.
>
>          Args:
> +            test_suite: The test suite object.
>              test_case_method: The test case method.
>              test_case_result: The test case level result object associated
>                  with the current test case.
> @@ -452,7 +656,7 @@ def _execute_test_case(
>          test_case_name = test_case_method.__name__
>          try:
>              self._logger.info(f"Starting test case execution: {test_case_name}")
> -            test_case_method()
> +            test_case_method(test_suite)
>              test_case_result.update(Result.PASS)
>              self._logger.info(f"Test case execution PASSED: {test_case_name}")
>
> diff --git a/dts/framework/settings.py b/dts/framework/settings.py
> index 609c8d0e62..2b8bfbe0ed 100644
> --- a/dts/framework/settings.py
> +++ b/dts/framework/settings.py
> @@ -253,8 +253,7 @@ def _get_parser() -> argparse.ArgumentParser:
>          "--test-cases",
>          action=_env_arg("DTS_TESTCASES"),
>          default="",
> -        help="[DTS_TESTCASES] Comma-separated list of test cases to execute. "
> -        "Unknown test cases will be silently ignored.",
> +        help="[DTS_TESTCASES] Comma-separated list of test cases to execute.",
>      )
>
>      parser.add_argument(
> diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
> index 4467749a9d..075195fd5b 100644
> --- a/dts/framework/test_result.py
> +++ b/dts/framework/test_result.py
> @@ -25,7 +25,9 @@
>
>  import os.path
>  from collections.abc import MutableSequence
> +from dataclasses import dataclass
>  from enum import Enum, auto
> +from types import MethodType
>
>  from .config import (
>      OS,
> @@ -36,10 +38,42 @@
>      CPUType,
>      NodeConfiguration,
>      NodeInfo,
> +    TestSuiteConfig,
>  )
>  from .exception import DTSError, ErrorSeverity
>  from .logger import DTSLOG
>  from .settings import SETTINGS
> +from .test_suite import TestSuite
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class TestSuiteWithCases:
> +    """A test suite class with test case methods.
> +
> +    An auxiliary class holding a test case class with test case methods. The intended use of this
> +    class is to hold a subset of test cases (which could be all test cases) because we don't have
> +    all the data to instantiate the class at the point of inspection. The knowledge of this subset
> +    is needed in case an error occurs before the class is instantiated and we need to record
> +    which test cases were blocked by the error.
> +
> +    Attributes:
> +        test_suite_class: The test suite class.
> +        test_cases: The test case methods.
> +    """
> +
> +    test_suite_class: type[TestSuite]
> +    test_cases: list[MethodType]
> +
> +    def create_config(self) -> TestSuiteConfig:
> +        """Generate a :class:`TestSuiteConfig` from the stored test suite with test cases.
> +
> +        Returns:
> +            The :class:`TestSuiteConfig` representation.
> +        """
> +        return TestSuiteConfig(
> +            test_suite=self.test_suite_class.__name__,
> +            test_cases=[test_case.__name__ for test_case in self.test_cases],
> +        )
>
>
>  class Result(Enum):
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index b02fd36147..f9fe88093e 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -11,25 +11,17 @@
>      * Testbed (SUT, TG) configuration,
>      * Packet sending and verification,
>      * Test case verification.
> -
> -The module also defines a function, :func:`get_test_suites`,
> -for gathering test suites from a Python module.
>  """
>
> -import importlib
> -import inspect
> -import re
>  from ipaddress import IPv4Interface, IPv6Interface, ip_interface
> -from types import MethodType
> -from typing import Any, ClassVar, Union
> +from typing import ClassVar, Union
>
>  from scapy.layers.inet import IP  # type: ignore[import]
>  from scapy.layers.l2 import Ether  # type: ignore[import]
>  from scapy.packet import Packet, Padding  # type: ignore[import]
>
> -from .exception import ConfigurationError, TestCaseVerifyError
> +from .exception import TestCaseVerifyError
>  from .logger import DTSLOG, getLogger
> -from .settings import SETTINGS
>  from .testbed_model import Port, PortLink, SutNode, TGNode
>  from .utils import get_packet_summaries
>
> @@ -37,7 +29,6 @@
>  class TestSuite(object):
>      """The base class with building blocks needed by most test cases.
>
> -        * Test case filtering and collection,
>          * Test suite setup/cleanup methods to override,
>          * Test case setup/cleanup methods to override,
>          * Test case verification,
> @@ -71,7 +62,6 @@ class TestSuite(object):
>      #: will block the execution of all subsequent test suites in the current build target.
>      is_blocking: ClassVar[bool] = False
>      _logger: DTSLOG
> -    _test_cases_to_run: list[str]
>      _port_links: list[PortLink]
>      _sut_port_ingress: Port
>      _sut_port_egress: Port
> @@ -86,24 +76,19 @@ def __init__(
>          self,
>          sut_node: SutNode,
>          tg_node: TGNode,
> -        test_cases: list[str],
>      ):
>          """Initialize the test suite testbed information and basic configuration.
>
> -        Process what test cases to run, find links between ports and set up
> -        default IP addresses to be used when configuring them.
> +        Find links between ports and set up default IP addresses to be used when
> +        configuring them.
>
>          Args:
>              sut_node: The SUT node where the test suite will run.
>              tg_node: The TG node where the test suite will run.
> -            test_cases: The list of test cases to execute.
> -                If empty, all test cases will be executed.
>          """
>          self.sut_node = sut_node
>          self.tg_node = tg_node
>          self._logger = getLogger(self.__class__.__name__)
> -        self._test_cases_to_run = test_cases
> -        self._test_cases_to_run.extend(SETTINGS.test_cases)
>          self._port_links = []
>          self._process_links()
>          self._sut_port_ingress, self._tg_port_egress = (
> @@ -364,65 +349,3 @@ def _verify_l3_packet(self, received_packet: IP, expected_packet: IP) -> bool:
>          if received_packet.src != expected_packet.src or received_packet.dst != expected_packet.dst:
>              return False
>          return True
> -
> -    def _get_functional_test_cases(self) -> list[MethodType]:
> -        """Get all functional test cases defined in this TestSuite.
> -
> -        Returns:
> -            The list of functional test cases of this TestSuite.
> -        """
> -        return self._get_test_cases(r"test_(?!perf_)")
> -
> -    def _get_test_cases(self, test_case_regex: str) -> list[MethodType]:
> -        """Return a list of test cases matching test_case_regex.
> -
> -        Returns:
> -            The list of test cases matching test_case_regex of this TestSuite.
> -        """
> -        self._logger.debug(f"Searching for test cases in {self.__class__.__name__}.")
> -        filtered_test_cases = []
> -        for test_case_name, test_case in inspect.getmembers(self, inspect.ismethod):
> -            if self._should_be_executed(test_case_name, test_case_regex):
> -                filtered_test_cases.append(test_case)
> -        cases_str = ", ".join((x.__name__ for x in filtered_test_cases))
> -        self._logger.debug(f"Found test cases '{cases_str}' in {self.__class__.__name__}.")
> -        return filtered_test_cases
> -
> -    def _should_be_executed(self, test_case_name: str, test_case_regex: str) -> bool:
> -        """Check whether the test case should be scheduled to be executed."""
> -        match = bool(re.match(test_case_regex, test_case_name))
> -        if self._test_cases_to_run:
> -            return match and test_case_name in self._test_cases_to_run
> -
> -        return match
> -
> -
> -def get_test_suites(testsuite_module_path: str) -> list[type[TestSuite]]:
> -    r"""Find all :class:`TestSuite`\s in a Python module.
> -
> -    Args:
> -        testsuite_module_path: The path to the Python module.
> -
> -    Returns:
> -        The list of :class:`TestSuite`\s found within the Python module.
> -
> -    Raises:
> -        ConfigurationError: The test suite module was not found.
> -    """
> -
> -    def is_test_suite(object: Any) -> bool:
> -        try:
> -            if issubclass(object, TestSuite) and object is not TestSuite:
> -                return True
> -        except TypeError:
> -            return False
> -        return False
> -
> -    try:
> -        testcase_module = importlib.import_module(testsuite_module_path)
> -    except ModuleNotFoundError as e:
> -        raise ConfigurationError(f"Test suite '{testsuite_module_path}' not found.") from e
> -    return [
> -        test_suite_class
> -        for _, test_suite_class in inspect.getmembers(testcase_module, is_test_suite)
> -    ]
> diff --git a/dts/pyproject.toml b/dts/pyproject.toml
> index 28bd970ae4..8eb92b4f11 100644
> --- a/dts/pyproject.toml
> +++ b/dts/pyproject.toml
> @@ -51,6 +51,9 @@ linters = "mccabe,pycodestyle,pydocstyle,pyflakes"
>  format = "pylint"
>  max_line_length = 100
>
> +[tool.pylama.linter.pycodestyle]
> +ignore = "E203,W503"
> +
>  [tool.pylama.linter.pydocstyle]
>  convention = "google"
>
> diff --git a/dts/tests/TestSuite_smoke_tests.py b/dts/tests/TestSuite_smoke_tests.py
> index 5e2bac14bd..7b2a0e97f8 100644
> --- a/dts/tests/TestSuite_smoke_tests.py
> +++ b/dts/tests/TestSuite_smoke_tests.py
> @@ -21,7 +21,7 @@
>  from framework.utils import REGEX_FOR_PCI_ADDRESS
>
>
> -class SmokeTests(TestSuite):
> +class TestSmokeTests(TestSuite):
>      """DPDK and infrastructure smoke test suite.
>
>      The test cases validate the most basic DPDK functionality needed for all other test suites.
> --
> 2.34.1
>

  reply	other threads:[~2024-02-12 16:45 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-20 10:33 [RFC PATCH v1 0/5] test case blocking and logging Juraj Linkeš
2023-12-20 10:33 ` [RFC PATCH v1 1/5] dts: convert dts.py methods to class Juraj Linkeš
2023-12-20 10:33 ` [RFC PATCH v1 2/5] dts: move test suite execution logic to DTSRunner Juraj Linkeš
2023-12-20 10:33 ` [RFC PATCH v1 3/5] dts: process test suites at the beginning of run Juraj Linkeš
2023-12-20 10:33 ` [RFC PATCH v1 4/5] dts: block all testcases when earlier setup fails Juraj Linkeš
2023-12-20 10:33 ` [RFC PATCH v1 5/5] dts: refactor logging configuration Juraj Linkeš
2024-01-08 18:47 ` [RFC PATCH v1 0/5] test case blocking and logging Jeremy Spewock
2024-02-06 14:57 ` [PATCH v2 0/7] " Juraj Linkeš
2024-02-06 14:57   ` [PATCH v2 1/7] dts: convert dts.py methods to class Juraj Linkeš
2024-02-06 14:57   ` [PATCH v2 2/7] dts: move test suite execution logic to DTSRunner Juraj Linkeš
2024-02-06 14:57   ` [PATCH v2 3/7] dts: filter test suites in executions Juraj Linkeš
2024-02-12 16:44     ` Jeremy Spewock [this message]
2024-02-14  9:55       ` Juraj Linkeš
2024-02-06 14:57   ` [PATCH v2 4/7] dts: reorganize test result Juraj Linkeš
2024-02-06 14:57   ` [PATCH v2 5/7] dts: block all test cases when earlier setup fails Juraj Linkeš
2024-02-06 14:57   ` [PATCH v2 6/7] dts: refactor logging configuration Juraj Linkeš
2024-02-12 16:45     ` Jeremy Spewock
2024-02-14  7:49       ` Juraj Linkeš
2024-02-14 16:51         ` Jeremy Spewock
2024-02-06 14:57   ` [PATCH v2 7/7] dts: improve test suite and case filtering Juraj Linkeš
2024-02-23  7:54 ` [PATCH v3 0/7] test case blocking and logging Juraj Linkeš
2024-02-23  7:54   ` [PATCH v3 1/7] dts: convert dts.py methods to class Juraj Linkeš
2024-02-23  7:54   ` [PATCH v3 2/7] dts: move test suite execution logic to DTSRunner Juraj Linkeš
2024-02-23  7:54   ` [PATCH v3 3/7] dts: filter test suites in executions Juraj Linkeš
2024-02-27 21:21     ` Jeremy Spewock
2024-02-28  9:16       ` Juraj Linkeš
2024-02-23  7:54   ` [PATCH v3 4/7] dts: reorganize test result Juraj Linkeš
2024-02-23  7:55   ` [PATCH v3 5/7] dts: block all test cases when earlier setup fails Juraj Linkeš
2024-02-23  7:55   ` [PATCH v3 6/7] dts: refactor logging configuration Juraj Linkeš
2024-02-23  7:55   ` [PATCH v3 7/7] dts: improve test suite and case filtering Juraj Linkeš
2024-02-27 21:24   ` [PATCH v3 0/7] test case blocking and logging Jeremy Spewock
2024-03-01 10:55 ` [PATCH v4 " Juraj Linkeš
2024-03-01 10:55   ` [PATCH v4 1/7] dts: convert dts.py methods to class Juraj Linkeš
2024-03-01 10:55   ` [PATCH v4 2/7] dts: move test suite execution logic to DTSRunner Juraj Linkeš
2024-03-01 10:55   ` [PATCH v4 3/7] dts: filter test suites in executions Juraj Linkeš
2024-03-01 10:55   ` [PATCH v4 4/7] dts: reorganize test result Juraj Linkeš
2024-03-01 10:55   ` [PATCH v4 5/7] dts: block all test cases when earlier setup fails Juraj Linkeš
2024-03-01 10:55   ` [PATCH v4 6/7] dts: refactor logging configuration Juraj Linkeš
2024-03-01 10:55   ` [PATCH v4 7/7] dts: improve test suite and case filtering Juraj Linkeš
2024-03-01 17:41     ` Jeremy Spewock
2024-03-01 17:43       ` Jeremy Spewock
2024-03-07 11:04         ` Thomas Monjalon
2024-03-01 16:11   ` [PATCH v4 0/7] test case blocking and logging Patrick Robb
2024-03-07 14:55   ` Thomas Monjalon

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=CAAA20UT1JF-onMA7GtUnfv27EkGEdCwjJHCEvvCYypnSvqc6Uw@mail.gmail.com \
    --to=jspewock@iol.unh.edu \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Luca.Vizzarro@arm.com \
    --cc=dev@dpdk.org \
    --cc=juraj.linkes@pantheon.tech \
    --cc=paul.szczepanek@arm.com \
    --cc=probb@iol.unh.edu \
    --cc=thomas@monjalon.net \
    /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).