[v3,03/12] dts: add test case decorators
Checks
Commit Message
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.linkes@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(-)
Comments
On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
<juraj.linkes@pantheon.tech> wrote:
<snip>
> 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]] = []
If TestCase is just a class, why is the `type[]` in the annotation
required? Are these not specific instances of the TestCase class? I
figured they would need to be in order for you to run the specific
test case methods. Maybe this has something to do with the class being
a Protocol?
> + 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__}."
> )
>
<snip>
> @@ -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.
> + """
> +
<snip>
> + 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
I think this part of the comment about "we're dealing with a test case
we would've removed in the other branch" confused me a little bit. It
could just be a me thing, but I think this would have been more clear
for me if it was something more like "The original list is not empty
(meaning we're filtering test cases). Since we didn't remove this test
case in the other branch, it doesn't match the filter and we don't
want to run it."
> + # 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
> +
<snip>
> 2.34.1
>
On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš <juraj.linkes@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.linkes@pantheon.tech>
>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
<juraj.linkes@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.linkes@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
>
On 26. 8. 2024 18:50, Jeremy Spewock wrote:
> On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
> <juraj.linkes@pantheon.tech> wrote:
> <snip>
>> 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]] = []
>
> If TestCase is just a class, why is the `type[]` in the annotation
> required? Are these not specific instances of the TestCase class? I
> figured they would need to be in order for you to run the specific
> test case methods. Maybe this has something to do with the class being
> a Protocol?
>
The *_test decorators return type[TestCase]. The functions (test
methods) are cast to type[TestCase] (which kinda makes them subclasses
of TestCase).
This was a suggestion from Luca and I took it as as. Maybe the functions
could be cast as instances of TestCase, but I didn't try that.
>> + 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__}."
>> )
>>
> <snip>
>> @@ -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.
>> + """
>> +
> <snip>
>> + 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
>
> I think this part of the comment about "we're dealing with a test case
> we would've removed in the other branch" confused me a little bit. It
> could just be a me thing, but I think this would have been more clear
> for me if it was something more like "The original list is not empty
> (meaning we're filtering test cases). Since we didn't remove this test
> case in the other branch, it doesn't match the filter and we don't
> want to run it."
>
We should remove any confusion. I'll change it - your wording sound good.
>> + # 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
>> +
> <snip>
>> 2.34.1
>>
On Thu, Sep 5, 2024 at 4:07 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
>
> On 26. 8. 2024 18:50, Jeremy Spewock wrote:
> > On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
> > <juraj.linkes@pantheon.tech> wrote:
> > <snip>
> >> 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]] = []
> >
> > If TestCase is just a class, why is the `type[]` in the annotation
> > required? Are these not specific instances of the TestCase class? I
> > figured they would need to be in order for you to run the specific
> > test case methods. Maybe this has something to do with the class being
> > a Protocol?
> >
>
> The *_test decorators return type[TestCase]. The functions (test
> methods) are cast to type[TestCase] (which kinda makes them subclasses
> of TestCase).
Oh interesting, I didn't make the connection that casting them to
type[TestCase] was similar to having them be subclasses of the type,
but this actually makes a lot of sense. Thank you for the
clarification!
>
> This was a suggestion from Luca and I took it as as. Maybe the functions
> could be cast as instances of TestCase, but I didn't try that.
Right, I would think that they could be cast directly to it, but
there's no need obviously so that makes sense.
>
> >> + 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)
<snip>
>
@@ -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}")
@@ -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.
@@ -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)
@@ -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:
@@ -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.
@@ -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)
@@ -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.