From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3E8EE458A9; Fri, 30 Aug 2024 17:50:27 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C0E6642E9D; Fri, 30 Aug 2024 17:50:26 +0200 (CEST) Received: from mail-lj1-f173.google.com (mail-lj1-f173.google.com [209.85.208.173]) by mails.dpdk.org (Postfix) with ESMTP id E078C42E9A for ; Fri, 30 Aug 2024 17:50:25 +0200 (CEST) Received: by mail-lj1-f173.google.com with SMTP id 38308e7fff4ca-2f402830d19so3437751fa.0 for ; Fri, 30 Aug 2024 08:50:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1725033025; x=1725637825; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=0PZ2tUfWs8Wirm3nf7ZF4z0dLdH8jkFXndr9Ng7uFUw=; b=ALueq+HF4ZcLfLHEKYyjpUCd9wg6JFG7TmIRb0yWZx/GSXRRQySNsqBNvPOlpQ3c4x ncNNucqJFAwI614aUjYmzKtiQYb1iAcRn9sJoJ997wrbpId7GCAI4d6qcYXaLoSwQ0CW /pxqXAYo1ZNmmyQkqn80dclECzUQIZjAEfcq4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725033025; x=1725637825; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0PZ2tUfWs8Wirm3nf7ZF4z0dLdH8jkFXndr9Ng7uFUw=; b=wHtRxYV4+hP9d1Om/3ouaITD83eIR3in8p1p6NJGZB4oX49lOOYdaqf5dqnWDFVmw0 zMi+LKjnco2d1FgYDeVBJ0Soc3Q508CHhcVIjGckMkEQmUM1VtX+uERr3If3UkqwAlor 6V7GRhCR+yKfn7N3rlb/SYiChHxyWet9klxDnBasq+8vYJIlTq4/Q6OskuLWgOVIwTzZ 874OKX7Em6NXbzBB/PNwhk4mRNUhB3EK9RBLVgK3/fLfw5HZ3uBC3br013blx3BdUXPL 6IOw4bmJ5KmknjG0Ko2Rrk5lnBas2wRKI06qrhl+PIa4PezGwWXntrIlTektzfAcQ9lM TT7g== X-Forwarded-Encrypted: i=1; AJvYcCWPdYlSzH6cD/qD3x2Ejv26gyDz9cCwyB2mKzIFvM9t49ldi58DnTYoUD9Z8Udu5VDbQPg=@dpdk.org X-Gm-Message-State: AOJu0YzXDzO3HIsdkJ3czAFe+O8B6KVWvR/shuSlEVgO0eZ1q+9qy2lA xuSmgJlmycKlYS3RdD6EVAt24esYF+gQGQsV2mF4oJcshJDUjz/3jWCuyX1wuw2Y0sFqGLXWptP ZiTlsTZdC5Li1jTs1PZi3DTbOHOQKvgp8g2XPmg== X-Google-Smtp-Source: AGHT+IHUAbj6YEcuY0XzewAxZBtM1g4WAUhOn9LuY5ak+XK2flSku0xIokfJ03inSS2XVVBp284J3bGqFyQqXIdbtII= X-Received: by 2002:a05:651c:2115:b0:2ef:2373:5f90 with SMTP id 38308e7fff4ca-2f61df9f4b8mr11612731fa.0.1725033024832; Fri, 30 Aug 2024 08:50:24 -0700 (PDT) MIME-Version: 1.0 References: <20240301155416.96960-1-juraj.linkes@pantheon.tech> <20240821145315.97974-1-juraj.linkes@pantheon.tech> <20240821145315.97974-4-juraj.linkes@pantheon.tech> In-Reply-To: <20240821145315.97974-4-juraj.linkes@pantheon.tech> From: Nicholas Pratte Date: Fri, 30 Aug 2024 11:50:13 -0400 Message-ID: Subject: Re: [PATCH v3 03/12] dts: add test case decorators To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, dmarx@iol.unh.edu, alex.chapman@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Reviewed-by: Nicholas Pratte On Wed, Aug 21, 2024 at 10:53=E2=80=AFAM Juraj Linke=C5=A1 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=C5=A1 > --- > 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 =3D self._get_test_suite_class(test_suite_c= onfig.test_suite) > - test_cases =3D [] > - func_test_cases, perf_test_cases =3D self._filter_test_cases= ( > - test_suite_class, test_suite_config.test_cases > + test_cases: list[type[TestCase]] =3D [] > + func_test_cases, perf_test_cases =3D test_suite_class.get_te= st_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: Sequ= ence[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 t= est 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_suit= e_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 performanc= e name prefix. > - """ > - func_test_cases =3D [] > - perf_test_cases =3D [] > - name_method_tuples =3D inspect.getmembers(test_suite_class, insp= ect.isfunction) > - if test_cases_to_run: > - name_method_tuples =3D [ > - (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 =3D set(test_cases_to_run) - { > - name for name, _ in name_method_tuples > - } > - raise ConfigurationError( > - f"Test cases {missing_test_cases} not found among me= thods " > - 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 associ= ated > with the current test suite. > """ > self._logger.set_stage(DtsStage.test_suite) > - for test_case_method in test_cases: > - test_case_name =3D test_case_method.__name__ > + for test_case in test_cases: > + test_case_name =3D test_case.__name__ > test_case_result =3D test_suite_result.add_test_case(test_ca= se_name) > all_attempts =3D SETTINGS.re_run + 1 > attempt_nr =3D 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 +=3D 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_c= ase_result) > + self._run_test_case(test_suite, test_case, test_case_res= ult) > > 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_sui= te`. > @@ -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 associat= ed > with the current test case. > """ > - test_case_name =3D test_case_method.__name__ > + test_case_name =3D 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_c= ase_result) > + self._execute_test_case(test_suite, test_case, test_case_res= ult) > > 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 resu= lt 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 associat= ed > with the current test case. > """ > - test_case_name =3D test_case_method.__name__ > + test_case_name =3D 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 compla= ins > + MethodType(test_case, test_suite)() > test_case_result.update(Result.PASS) > self._logger.info(f"Test case execution PASSED: {test_case_n= ame}") > > 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=3DTrue, frozen=3DTrue) > @@ -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 suit= e 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=3Dsut_port= , tg_port=3Dtg_port)) > > + @classmethod > + def get_test_cases( > + cls, test_case_sublist: Sequence[str] | None =3D 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 functi= ons as opposed to methods, > + as methods are bound to instances and this method only has a= ccess to the class. > + > + Raises: > + ConfigurationError: If a test case from `test_case_subset` i= s not found. > + """ > + > + def is_test_case(function: Callable) -> bool: > + if inspect.isfunction(function): > + # TestCase is not used at runtime, so we can't use isins= tance() 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 =3D [] > + > + # the copy is needed so that the condition "elif test_case_subli= st" doesn't > + # change mid-cycle > + test_case_sublist_copy =3D list(test_case_sublist) > + func_test_cases =3D set() > + perf_test_cases =3D 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 fou= nd 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 filte= ring 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 fu= nctions 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, exp= ected_packet: IP) -> bool: > if received_packet.src !=3D expected_packet.src or received_pack= et.dst !=3D expected_packet.dst: > return False > return True > + > + > +#: The generic type for a method of an instance of TestSuite > +TestSuiteMethodType =3D TypeVar("TestSuiteMethodType", bound=3DCallable[= [TestSuite], None]) > + > + > +class TestCaseType(Enum): > + """The types of test cases.""" > + > + #: > + FUNCTIONAL =3D auto() > + #: > + PERFORMANCE =3D auto() > + > + > +class TestCase(Protocol[TestSuiteMethodType]): > + """Definition of the test case type for static type checking purpose= s. > + > + The type is applied to test case functions through a decorator, whic= h 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 functio= n 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:`RequiresCapa= bilities`. > + > + 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 =3D cast(type[TestCase], func) > + test_case.test_type =3D test_case_type > + return test_case > + > + return _decorator > + > + > +#: The decorator for functional test cases. > +func_test: Callable =3D TestCase.make_decorator(TestCaseType.FUNCTIONAL) > +#: The decorator for performance test cases. > +perf_test: Callable =3D TestCase.make_decorator(TestCaseType.PERFORMANCE= ) > diff --git a/dts/tests/TestSuite_hello_world.py b/dts/tests/TestSuite_hel= lo_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 =3D self.sut_node.build_dpdk_app("hello= world") > > - 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.p= y > 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=3DFalse) > 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/TestSu= ite_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=3D2048) > diff --git a/dts/tests/TestSuite_smoke_tests.py b/dts/tests/TestSuite_smo= ke_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 =3D self.sut_node.remote_dpdk_build_dir > self.nics_in_node =3D 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=3DTrue, > ) > > + @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=3DTrue, > ) > > + @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 >