From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Jeremy Spewock <jspewock@iol.unh.edu>
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: Wed, 14 Feb 2024 10:55:05 +0100 [thread overview]
Message-ID: <CAOb5WZYuvNRYD==TH9DmjQqFP+VVCKEEpHd29VxsEZt8BwJ7Gg@mail.gmail.com> (raw)
In-Reply-To: <CAAA20UT1JF-onMA7GtUnfv27EkGEdCwjJHCEvvCYypnSvqc6Uw@mail.gmail.com>
On Mon, Feb 12, 2024 at 5:44 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> 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.
>
I've looked at how the code looks and it is better. I also changed
some of the variable names (test_suite_name -> module_name and
lowercase_suite_name -> lowercase_suite_to_find), updated the
docstring and now I'm much happier with the result.
> > + 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.
>
I'll change this, thanks for the grammar fix.
> > +
> > + 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
> >
next prev parent reply other threads:[~2024-02-14 9:55 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
2024-02-14 9:55 ` Juraj Linkeš [this message]
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='CAOb5WZYuvNRYD==TH9DmjQqFP+VVCKEEpHd29VxsEZt8BwJ7Gg@mail.gmail.com' \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=Luca.Vizzarro@arm.com \
--cc=dev@dpdk.org \
--cc=jspewock@iol.unh.edu \
--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).