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 95A7D45A84; Tue, 1 Oct 2024 22:45:15 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5CB824026C; Tue, 1 Oct 2024 22:45:15 +0200 (CEST) Received: from mail-lf1-f48.google.com (mail-lf1-f48.google.com [209.85.167.48]) by mails.dpdk.org (Postfix) with ESMTP id 8A04640268 for ; Tue, 1 Oct 2024 22:45:14 +0200 (CEST) Received: by mail-lf1-f48.google.com with SMTP id 2adb3069b0e04-539899c1febso402927e87.2 for ; Tue, 01 Oct 2024 13:45:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1727815514; x=1728420314; 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=sIbYp/PItwoy/gi6Zv1SBCgSxtV+0ij3wcu6Plwk1U4=; b=eD12ECnftcKwaj0FJLnxfaMESfhybDb1aANHS3Wj5U+zpQvdyRFxGRsMm3nUOA/kt4 35a9XSMIMpdlHuebSt1w6PS3XArWpXRnXGNr02OKRpDAh+ODOi6+GtDrFhPYYmv66FVU ihIO7u8EYlNQrsVJx/GfY0hw04jlcU0pEWHwg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727815514; x=1728420314; 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=sIbYp/PItwoy/gi6Zv1SBCgSxtV+0ij3wcu6Plwk1U4=; b=dpcwtytFPNfi0z3G7MXX2rl8m+5bISm9IOZwJPVa9HqNsHrs0SkkUEYuqDchu1HI+K Xpamc4P/Z3UMBpBFa2HkWCWNKtDB4YAMF0ybPG5RSQdtHKEPIJkQIUCmuEbjvvcRLIYF KCQ2T+RayR2f2R3EsxKTGrzMJ2O8wrZTVolg63cmURuabTtNck2f5+L8eAn4BnubaksM apgVuZOUo4cw+cKj0ijgylbqmVc7YJ2LXaq3cyHTswYcnKuA1hYZOywKWGVEGp/xAn/M y2GjCQxEN4C2Cq7Cs69ANP715AR3AFc+x3fvAevcFjeHv8Y2nco2SDPCle4xd6djTesS beIQ== X-Gm-Message-State: AOJu0Yw09PWhlmdCC/lOV2beR2RNGEViRA0ta9SJrtCgnWdqm7TpcLDM jDQ7qpMIUggDAFHiivf7uzO72clp0CtAwQyIpFolayPdJ/Lv6y4pCNhFvzFTE97Ntqk6DeRov2I t7UBhg0EbU7xJ92dR5LIQ4D95omZwNAnRWAcWfQ== X-Google-Smtp-Source: AGHT+IHEF3cP7Nmr3aODHf5HwX8ZD6kRKCPb8wRoH42fogapBGgHkR9YqTKYC/otDZy8kNKyJJZroq3opF+3K0jaW00= X-Received: by 2002:a2e:a543:0:b0:2ee:d55c:255f with SMTP id 38308e7fff4ca-2fae10a9041mr1822241fa.7.1727815513671; Tue, 01 Oct 2024 13:45:13 -0700 (PDT) MIME-Version: 1.0 References: <20240822163941.1390326-1-luca.vizzarro@arm.com> <20240822163941.1390326-5-luca.vizzarro@arm.com> In-Reply-To: <20240822163941.1390326-5-luca.vizzarro@arm.com> From: Nicholas Pratte Date: Tue, 1 Oct 2024 16:45:02 -0400 Message-ID: Subject: Re: [PATCH 4/5] dts: use TestSuiteSpec class imports To: Luca Vizzarro Cc: dev@dpdk.org, Honnappa Nagarahalli , =?UTF-8?Q?Juraj_Linke=C5=A1?= , Paul Szczepanek 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 The code you have here makes sense, and I like the implementation as it removes a lot of fluff in DTSRunner. I know Jurja mentioned in an earlier patch in this series that this functionality intersects with the capabilities series, but I'm missing a lot of context to understand that fully. Maybe you could provide some insight? I'll make sure to analyse this deeper in my own time as well. Beyond that: Reviewed-by: Nicholas Pratte On Thu, Aug 22, 2024 at 12:40=E2=80=AFPM Luca Vizzarro wrote: > > The introduction of TestSuiteSpec adds auto-discovery of test suites, > which are also automatically imported. This causes double imports as the > runner loads the test suites. This changes the behaviour of the runner > to load the imported classes from TestSuiteSpec instead of importing > them again. > > Signed-off-by: Luca Vizzarro > Reviewed-by: Paul Szczepanek > --- > dts/framework/runner.py | 167 +++++++--------------------------------- > 1 file changed, 27 insertions(+), 140 deletions(-) > > diff --git a/dts/framework/runner.py b/dts/framework/runner.py > index 14e405aced..00b63cc292 100644 > --- a/dts/framework/runner.py > +++ b/dts/framework/runner.py > @@ -2,6 +2,7 @@ > # Copyright(c) 2010-2019 Intel Corporation > # Copyright(c) 2022-2023 PANTHEON.tech s.r.o. > # Copyright(c) 2022-2023 University of New Hampshire > +# Copyright(c) 2024 Arm Limited > > """Test suite runner module. > > @@ -17,14 +18,11 @@ > and the test case stage runs test cases individually. > """ > > -import importlib > -import inspect > import os > -import re > import sys > from pathlib import Path > from types import FunctionType > -from typing import Iterable, Sequence > +from typing import Iterable > > from framework.testbed_model.sut_node import SutNode > from framework.testbed_model.tg_node import TGNode > @@ -38,12 +36,7 @@ > TGNodeConfiguration, > load_config, > ) > -from .exception import ( > - BlockingTestSuiteError, > - ConfigurationError, > - SSHTimeoutError, > - TestCaseVerifyError, > -) > +from .exception import BlockingTestSuiteError, SSHTimeoutError, TestCase= VerifyError > from .logger import DTSLogger, DtsStage, get_dts_logger > from .settings import SETTINGS > from .test_result import ( > @@ -55,7 +48,7 @@ > TestSuiteResult, > TestSuiteWithCases, > ) > -from .test_suite import TestSuite > +from .test_suite import TestCase, TestCaseVariant, TestSuite > > > class DTSRunner: > @@ -217,11 +210,10 @@ def _get_test_suites_with_cases( > func: bool, > perf: bool, > ) -> list[TestSuiteWithCases]: > - """Test suites with test cases discovery. > + """Get test suites with selected cases. > > - The test suites with test cases defined in the user configuratio= n are discovered > - and stored for future use so that we don't import the modules tw= ice and so that > - the list of test suites with test cases is available for recordi= ng right away. > + The test suites with test cases defined in the user configuratio= n are selected > + and the corresponding functions and classes are gathered. > > Args: > test_suite_configs: Test suite configurations. > @@ -229,139 +221,34 @@ def _get_test_suites_with_cases( > perf: Whether to include performance test cases in the final= list. > > Returns: > - The discovered test suites, each with test cases. > + The test suites, each with test cases. > """ > test_suites_with_cases =3D [] > > for test_suite_config in test_suite_configs: > - test_suite_class =3D self._get_test_suite_class(test_suite_c= onfig.test_suite_name) > - test_cases =3D [] > - func_test_cases, perf_test_cases =3D self._filter_test_cases= ( > - test_suite_class, test_suite_config.test_cases_names > - ) > - 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=3Dtest_suite_class, = test_cases=3Dtest_cases) > - ) > - > - return test_suites_with_cases > - > - def _get_test_suite_class(self, module_name: str) -> type[TestSuite]= : > - """Find the :class:`TestSuite` class in `module_name`. > - > - The full module name is `module_name` prefixed with `self._test_= suite_module_prefix`. > - The module name is a standard filename with words separated with= underscores. > - Search the `module_name` for a :class:`TestSuite` class which st= arts > - with `self._test_suite_class_prefix`, continuing with CamelCase = `module_name`. > - The first matching class is returned. > - > - The CamelCase convention applies to abbreviations, acronyms, ini= tialisms and so on:: > - > - OS -> Os > - TCP -> Tcp > - > - Args: > - module_name: The module name without prefix where to search = for the test suite. > - > - Returns: > - The found test suite class. > - > - 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 :c= lass:`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 TestS= uite: > - return True > - except TypeError: > - return False > - return False > - > - testsuite_module_path =3D f"{self._test_suite_module_prefix}{mod= ule_name}" > - try: > - test_suite_module =3D importlib.import_module(testsuite_modu= le_path) > - except ModuleNotFoundError as e: > - raise ConfigurationError( > - f"Test suite module '{testsuite_module_path}' not found.= " > - ) from e > - > - camel_case_suite_name =3D "".join( > - [suite_word.capitalize() for suite_word in module_name.split= ("_")] > - ) > - full_suite_name_to_find =3D f"{self._test_suite_class_prefix}{ca= mel_case_suite_name}" > - for class_name, class_obj in inspect.getmembers(test_suite_modul= e, is_test_suite): > - if class_name =3D=3D full_suite_name_to_find: > - 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: 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. > + test_suite_spec =3D test_suite_config.test_suite_spec > + test_suite_class =3D test_suite_spec.class_type > + > + filtered_test_cases: list[TestCase] =3D [ > + test_case > + for test_case in test_suite_spec.test_cases > + if not test_suite_config.test_cases_names > + or test_case.name in test_suite_config.test_cases_names > + ] > > - 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 > + selected_test_cases: list[FunctionType] =3D [ > + test_case.function_type # type: ignore[misc] > + for test_case in filtered_test_cases > + if (func and test_case.variant =3D=3D TestCaseVariant.FU= NCTIONAL) > + or (perf and test_case.variant =3D=3D TestCaseVariant.PE= RFORMANCE) > ] > - 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." > + test_suites_with_cases.append( > + TestSuiteWithCases( > + test_suite_class=3Dtest_suite_class, test_cases=3Dse= lected_test_cases > ) > - > - return func_test_cases, perf_test_cases > + ) > + return test_suites_with_cases > > def _connect_nodes_and_run_test_run( > self, > -- > 2.34.1 >