Reviewed-by: Nicholas Pratte <npra...@iol.unh.edu>
On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
<juraj.lin...@pantheon.tech> wrote:
>
> Add decorators for functional and performance test cases. These
> decorators add attributes to the decorated test cases.
>
> With the addition of decorators, we change the test case discovery
> mechanism from looking at test case names according to a regex to simply
> checking an attribute of the function added with one of the decorators.
>
> The decorators allow us to add further variables to test cases.
>
> Also move the test case filtering to TestSuite while changing the
> mechanism to separate the logic in a more sensible manner.
>
> Bugzilla ID: 1460
>
> Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech>
> ---
> dts/framework/runner.py | 93 ++++------------
> dts/framework/test_result.py | 5 +-
> dts/framework/test_suite.py | 125 +++++++++++++++++++++-
> dts/tests/TestSuite_hello_world.py | 8 +-
> dts/tests/TestSuite_os_udp.py | 3 +-
> dts/tests/TestSuite_pmd_buffer_scatter.py | 3 +-
> dts/tests/TestSuite_smoke_tests.py | 6 +-
> 7 files changed, 160 insertions(+), 83 deletions(-)
>
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index 6b6f6a05f5..525f119ab6 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -20,11 +20,10 @@
> import importlib
> import inspect
> import os
> -import re
> import sys
> from pathlib import Path
> -from types import FunctionType
> -from typing import Iterable, Sequence
> +from types import MethodType
> +from typing import Iterable
>
> from framework.testbed_model.sut_node import SutNode
> from framework.testbed_model.tg_node import TGNode
> @@ -53,7 +52,7 @@
> TestSuiteResult,
> TestSuiteWithCases,
> )
> -from .test_suite import TestSuite
> +from .test_suite import TestCase, TestSuite
>
>
> class DTSRunner:
> @@ -232,9 +231,9 @@ def _get_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, test_suite_config.test_cases
> + test_cases: list[type[TestCase]] = []
> + func_test_cases, perf_test_cases =
> test_suite_class.get_test_cases(
> + test_suite_config.test_cases
> )
> if func:
> test_cases.extend(func_test_cases)
> @@ -309,57 +308,6 @@ def is_test_suite(object) -> bool:
> 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:
> Sequence[str]
> - ) -> tuple[list[FunctionType], list[FunctionType]]:
> - """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 matches neither the functional nor performance name
> prefix, it's an error.
> -
> - 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 = set(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}' matches neither "
> - f"a functional nor a performance test case name."
> - )
> -
> - return func_test_cases, perf_test_cases
> -
> def _connect_nodes_and_run_test_run(
> self,
> sut_nodes: dict[str, SutNode],
> @@ -607,7 +555,7 @@ def _run_test_suite(
> def _execute_test_suite(
> self,
> test_suite: TestSuite,
> - test_cases: Iterable[FunctionType],
> + test_cases: Iterable[type[TestCase]],
> test_suite_result: TestSuiteResult,
> ) -> None:
> """Execute all `test_cases` in `test_suite`.
> @@ -618,29 +566,29 @@ def _execute_test_suite(
>
> Args:
> test_suite: The test suite object.
> - test_cases: The list of test case methods.
> + test_cases: The list of test case functions.
> test_suite_result: The test suite level result object associated
> with the current test suite.
> """
> self._logger.set_stage(DtsStage.test_suite)
> - for test_case_method in test_cases:
> - test_case_name = test_case_method.__name__
> + for test_case in test_cases:
> + test_case_name = test_case.__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)
> + self._run_test_case(test_suite, test_case, 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)
> + self._run_test_case(test_suite, test_case, test_case_result)
>
> def _run_test_case(
> self,
> test_suite: TestSuite,
> - test_case_method: FunctionType,
> + test_case: type[TestCase],
> test_case_result: TestCaseResult,
> ) -> None:
> """Setup, execute and teardown `test_case_method` from `test_suite`.
> @@ -649,11 +597,11 @@ def _run_test_case(
>
> Args:
> test_suite: The test suite object.
> - test_case_method: The test case method.
> + test_case: The test case function.
> test_case_result: The test case level result object associated
> with the current test case.
> """
> - test_case_name = test_case_method.__name__
> + test_case_name = test_case.__name__
>
> try:
> # run set_up function for each case
> @@ -668,7 +616,7 @@ def _run_test_case(
>
> else:
> # run test case if setup was successful
> - self._execute_test_case(test_suite, test_case_method,
> test_case_result)
> + self._execute_test_case(test_suite, test_case, test_case_result)
>
> finally:
> try:
> @@ -686,21 +634,22 @@ def _run_test_case(
> def _execute_test_case(
> self,
> test_suite: TestSuite,
> - test_case_method: FunctionType,
> + test_case: type[TestCase],
> test_case_result: TestCaseResult,
> ) -> None:
> """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: The test case function.
> test_case_result: The test case level result object associated
> with the current test case.
> """
> - test_case_name = test_case_method.__name__
> + test_case_name = test_case.__name__
> try:
> self._logger.info(f"Starting test case execution:
> {test_case_name}")
> - test_case_method(test_suite)
> + # Explicit method binding is required, otherwise mypy complains
> + MethodType(test_case, test_suite)()
> test_case_result.update(Result.PASS)
> self._logger.info(f"Test case execution PASSED:
> {test_case_name}")
>
> diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
> index 5694a2482b..b1ca584523 100644
> --- a/dts/framework/test_result.py
> +++ b/dts/framework/test_result.py
> @@ -27,7 +27,6 @@
> from collections.abc import MutableSequence
> from dataclasses import dataclass
> from enum import Enum, auto
> -from types import FunctionType
> from typing import Union
>
> from .config import (
> @@ -44,7 +43,7 @@
> from .exception import DTSError, ErrorSeverity
> from .logger import DTSLogger
> from .settings import SETTINGS
> -from .test_suite import TestSuite
> +from .test_suite import TestCase, TestSuite
>
>
> @dataclass(slots=True, frozen=True)
> @@ -63,7 +62,7 @@ class is to hold a subset of test cases (which could be all
> test cases) because
> """
>
> test_suite_class: type[TestSuite]
> - test_cases: list[FunctionType]
> + test_cases: list[type[TestCase]]
>
> def create_config(self) -> TestSuiteConfig:
> """Generate a :class:`TestSuiteConfig` from the stored test suite
> with test cases.
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index 694b2eba65..b4ee0f9039 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -13,8 +13,11 @@
> * Test case verification.
> """
>
> +import inspect
> +from collections.abc import Callable, Sequence
> +from enum import Enum, auto
> from ipaddress import IPv4Interface, IPv6Interface, ip_interface
> -from typing import ClassVar, Union
> +from typing import ClassVar, Protocol, TypeVar, Union, cast
>
> from scapy.layers.inet import IP # type: ignore[import-untyped]
> from scapy.layers.l2 import Ether # type: ignore[import-untyped]
> @@ -27,7 +30,7 @@
> PacketFilteringConfig,
> )
>
> -from .exception import TestCaseVerifyError
> +from .exception import ConfigurationError, TestCaseVerifyError
> from .logger import DTSLogger, get_dts_logger
> from .utils import get_packet_summaries
>
> @@ -120,6 +123,68 @@ def _process_links(self) -> None:
> ):
> self._port_links.append(PortLink(sut_port=sut_port,
> tg_port=tg_port))
>
> + @classmethod
> + def get_test_cases(
> + cls, test_case_sublist: Sequence[str] | None = None
> + ) -> tuple[set[type["TestCase"]], set[type["TestCase"]]]:
> + """Filter `test_case_subset` from this class.
> +
> + Test cases are regular (or bound) methods decorated with
> :func:`func_test`
> + or :func:`perf_test`.
> +
> + Args:
> + test_case_sublist: Test case names to filter from this class.
> + If empty or :data:`None`, return all test cases.
> +
> + Returns:
> + The filtered test case functions. This method returns functions
> as opposed to methods,
> + as methods are bound to instances and this method only has
> access to the class.
> +
> + Raises:
> + ConfigurationError: If a test case from `test_case_subset` is
> not found.
> + """
> +
> + def is_test_case(function: Callable) -> bool:
> + if inspect.isfunction(function):
> + # TestCase is not used at runtime, so we can't use
> isinstance() with `function`.
> + # But function.test_type exists.
> + if hasattr(function, "test_type"):
> + return isinstance(function.test_type, TestCaseType)
> + return False
> +
> + if test_case_sublist is None:
> + test_case_sublist = []
> +
> + # the copy is needed so that the condition "elif test_case_sublist"
> doesn't
> + # change mid-cycle
> + test_case_sublist_copy = list(test_case_sublist)
> + func_test_cases = set()
> + perf_test_cases = set()
> +
> + for test_case_name, test_case_function in inspect.getmembers(cls,
> is_test_case):
> + if test_case_name in test_case_sublist_copy:
> + # if test_case_sublist_copy is non-empty, remove the found
> test case
> + # so that we can look at the remainder at the end
> + test_case_sublist_copy.remove(test_case_name)
> + elif test_case_sublist:
> + # if the original list is not empty (meaning we're filtering
> test cases),
> + # we're dealing with a test case we would've
> + # removed in the other branch; since we didn't, we don't
> want to run it
> + continue
> +
> + match test_case_function.test_type:
> + case TestCaseType.PERFORMANCE:
> + perf_test_cases.add(test_case_function)
> + case TestCaseType.FUNCTIONAL:
> + func_test_cases.add(test_case_function)
> +
> + if test_case_sublist_copy:
> + raise ConfigurationError(
> + f"Test cases {test_case_sublist_copy} not found among
> functions of {cls.__name__}."
> + )
> +
> + return func_test_cases, perf_test_cases
> +
> def set_up_suite(self) -> None:
> """Set up test fixtures common to all test cases.
>
> @@ -365,3 +430,59 @@ 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
> +
> +
> +#: The generic type for a method of an instance of TestSuite
> +TestSuiteMethodType = TypeVar("TestSuiteMethodType",
> bound=Callable[[TestSuite], None])
> +
> +
> +class TestCaseType(Enum):
> + """The types of test cases."""
> +
> + #:
> + FUNCTIONAL = auto()
> + #:
> + PERFORMANCE = auto()
> +
> +
> +class TestCase(Protocol[TestSuiteMethodType]):
> + """Definition of the test case type for static type checking purposes.
> +
> + The type is applied to test case functions through a decorator, which
> casts the decorated
> + test case function to :class:`TestCase` and sets common variables.
> + """
> +
> + #:
> + test_type: ClassVar[TestCaseType]
> + #: necessary for mypy so that it can treat this class as the function
> it's shadowing
> + __call__: TestSuiteMethodType
> +
> + @classmethod
> + def make_decorator(
> + cls, test_case_type: TestCaseType
> + ) -> Callable[[TestSuiteMethodType], type["TestCase"]]:
> + """Create a decorator for test suites.
> +
> + The decorator casts the decorated function as :class:`TestCase`,
> + sets it as `test_case_type`
> + and initializes common variables defined in
> :class:`RequiresCapabilities`.
> +
> + Args:
> + test_case_type: Either a functional or performance test case.
> +
> + Returns:
> + The decorator of a functional or performance test case.
> + """
> +
> + def _decorator(func: TestSuiteMethodType) -> type[TestCase]:
> + test_case = cast(type[TestCase], func)
> + test_case.test_type = test_case_type
> + return test_case
> +
> + return _decorator
> +
> +
> +#: The decorator for functional test cases.
> +func_test: Callable = TestCase.make_decorator(TestCaseType.FUNCTIONAL)
> +#: The decorator for performance test cases.
> +perf_test: Callable = TestCase.make_decorator(TestCaseType.PERFORMANCE)
> diff --git a/dts/tests/TestSuite_hello_world.py
> b/dts/tests/TestSuite_hello_world.py
> index d958f99030..16d064ffeb 100644
> --- a/dts/tests/TestSuite_hello_world.py
> +++ b/dts/tests/TestSuite_hello_world.py
> @@ -8,7 +8,7 @@
> """
>
> from framework.remote_session.dpdk_shell import compute_eal_params
> -from framework.test_suite import TestSuite
> +from framework.test_suite import TestSuite, func_test
> from framework.testbed_model.cpu import (
> LogicalCoreCount,
> LogicalCoreCountFilter,
> @@ -27,7 +27,8 @@ def set_up_suite(self) -> None:
> """
> self.app_helloworld_path = self.sut_node.build_dpdk_app("helloworld")
>
> - def test_hello_world_single_core(self) -> None:
> + @func_test
> + def hello_world_single_core(self) -> None:
> """Single core test case.
>
> Steps:
> @@ -46,7 +47,8 @@ def test_hello_world_single_core(self) -> None:
> f"helloworld didn't start on lcore{lcores[0]}",
> )
>
> - def test_hello_world_all_cores(self) -> None:
> + @func_test
> + def hello_world_all_cores(self) -> None:
> """All cores test case.
>
> Steps:
> diff --git a/dts/tests/TestSuite_os_udp.py b/dts/tests/TestSuite_os_udp.py
> index a78bd74139..beaa5f425d 100644
> --- a/dts/tests/TestSuite_os_udp.py
> +++ b/dts/tests/TestSuite_os_udp.py
> @@ -10,7 +10,7 @@
> from scapy.layers.inet import IP, UDP # type: ignore[import-untyped]
> from scapy.layers.l2 import Ether # type: ignore[import-untyped]
>
> -from framework.test_suite import TestSuite
> +from framework.test_suite import TestSuite, func_test
>
>
> class TestOsUdp(TestSuite):
> @@ -26,6 +26,7 @@ def set_up_suite(self) -> None:
> self.sut_node.bind_ports_to_driver(for_dpdk=False)
> self.configure_testbed_ipv4()
>
> + @func_test
> def test_os_udp(self) -> None:
> """Basic UDP IPv4 traffic test case.
>
> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py
> b/dts/tests/TestSuite_pmd_buffer_scatter.py
> index 0d8e101e5c..020fb0ab62 100644
> --- a/dts/tests/TestSuite_pmd_buffer_scatter.py
> +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
> @@ -24,7 +24,7 @@
>
> from framework.params.testpmd import SimpleForwardingModes
> from framework.remote_session.testpmd_shell import TestPmdShell
> -from framework.test_suite import TestSuite
> +from framework.test_suite import TestSuite, func_test
>
>
> class TestPmdBufferScatter(TestSuite):
> @@ -123,6 +123,7 @@ def pmd_scatter(self, mbsize: int) -> None:
> f"{offset}.",
> )
>
> + @func_test
> def test_scatter_mbuf_2048(self) -> None:
> """Run the :meth:`pmd_scatter` test with `mbsize` set to 2048."""
> self.pmd_scatter(mbsize=2048)
> diff --git a/dts/tests/TestSuite_smoke_tests.py
> b/dts/tests/TestSuite_smoke_tests.py
> index c0b0e6bb00..94f90d9327 100644
> --- a/dts/tests/TestSuite_smoke_tests.py
> +++ b/dts/tests/TestSuite_smoke_tests.py
> @@ -17,7 +17,7 @@
> from framework.config import PortConfig
> from framework.remote_session.testpmd_shell import TestPmdShell
> from framework.settings import SETTINGS
> -from framework.test_suite import TestSuite
> +from framework.test_suite import TestSuite, func_test
> from framework.utils import REGEX_FOR_PCI_ADDRESS
>
>
> @@ -47,6 +47,7 @@ def set_up_suite(self) -> None:
> self.dpdk_build_dir_path = self.sut_node.remote_dpdk_build_dir
> self.nics_in_node = self.sut_node.config.ports
>
> + @func_test
> def test_unit_tests(self) -> None:
> """DPDK meson ``fast-tests`` unit tests.
>
> @@ -63,6 +64,7 @@ def test_unit_tests(self) -> None:
> privileged=True,
> )
>
> + @func_test
> def test_driver_tests(self) -> None:
> """DPDK meson ``driver-tests`` unit tests.
>
> @@ -91,6 +93,7 @@ def test_driver_tests(self) -> None:
> privileged=True,
> )
>
> + @func_test
> def test_devices_listed_in_testpmd(self) -> None:
> """Testpmd device discovery.
>
> @@ -108,6 +111,7 @@ def test_devices_listed_in_testpmd(self) -> None:
> "please check your configuration",
> )
>
> + @func_test
> def test_device_bound_to_driver(self) -> None:
> """Device driver in OS.
>
> --
> 2.34.1
>
