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 771934338D; Tue, 21 Nov 2023 10:10:54 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E711440298; Tue, 21 Nov 2023 10:10:53 +0100 (CET) Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by mails.dpdk.org (Postfix) with ESMTP id 4BDBA4025F for ; Tue, 21 Nov 2023 10:10:52 +0100 (CET) Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-a002562bd8bso253944866b.0 for ; Tue, 21 Nov 2023 01:10:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1700557852; x=1701162652; 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=hzJhAg1YqEq3ESqRTdtoH6yCUPIfvBD3DBPSwi6tMbY=; b=d89QqdM5G9VO1PDlxu7ifdUAdM+FTOiqVZf0UOYpXY3rL1oO3FaZvgm6y0yFRtsNYn MHNo8juYDcimxEG/LQuW+0riyUzYeau5l+61SkIQN0JP2jdkBLzsXoiOsgoqMEqFDHtg cPZcW2NAA4MhpPh8Zsbig9D1Cfr2pFhBe/lCppYHfsUKuvoNvJoeQbFxy37+goE4LXZK e/aZl0QNbCx+SQvk3Am8u7i2gWF9Pn1BFEHg4qTWfAlK0+5ddcCydrHGeWwxZM43d0av RLSj+34G5kz7SlCLl/uDg0QVrmSWyA+S/xZGWTi0v9yNQdN2vWyrLXjQqR0An4tyEdyI Mgaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700557852; x=1701162652; 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=hzJhAg1YqEq3ESqRTdtoH6yCUPIfvBD3DBPSwi6tMbY=; b=rI8VAZecKyW8eypYQKshCen04gpMhgUICzQjx01V8JYYGaSXfukP6Jp+UimY7N+OQm 5ecgEcnX6ex5RHMLukR5XwTkEjrvSb2nWFBaQ81OI7O6jlrSAJj4vobipYpW3Hkc8gch PqZuk7pg7rf+oTGZ4/Dg46EU6Q8tMgqeJAZs5T69U5IYtl1Syrm6jYMeCqdI6/MPPn9/ umwqwiqf3grADX5KhqESnTrnDisOm/ce/HmOZb3797a296SZSz6fpv605YZ/RfIWZ+u/ tkd/14B0Ny76J31WxKGqQJD0HpzkZsuLrWRE9XL+7SHjLb0sQbck46Bi3PQYd0fJ0iuI rFrw== X-Gm-Message-State: AOJu0YyONLQnBYeeijE9jJWYDW1fJSa9EnKl+RN2K7Xf6c02CIyRJfTW bzf+4DyribG4xr8dvxhZnfFnYA+0XY+Q/sLsruROLw== X-Google-Smtp-Source: AGHT+IHKrIFxlmJR1tOCjV3fEcoVOFsOS0nBIC5HomFt2aoPdjhUWiyb+dMykFFppOIqWhLv/iUjqf4FXXqHrNvxtkk= X-Received: by 2002:a17:907:15d5:b0:9f2:6fac:e266 with SMTP id cj21-20020a17090715d500b009f26face266mr1767559ejc.1.1700557851739; Tue, 21 Nov 2023 01:10:51 -0800 (PST) MIME-Version: 1.0 References: <20231108125324.191005-23-juraj.linkes@pantheon.tech> <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231115130959.39420-8-juraj.linkes@pantheon.tech> In-Reply-To: From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Tue, 21 Nov 2023 10:10:40 +0100 Message-ID: Subject: Re: [PATCH v7 07/21] dts: dts runner and main docstring update To: Yoan Picchi Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@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 On Mon, Nov 20, 2023 at 6:43=E2=80=AFPM Yoan Picchi wrote: > > On 11/15/23 13:09, Juraj Linke=C5=A1 wrote: > > Format according to the Google format and PEP257, with slight > > deviations. > > > > Signed-off-by: Juraj Linke=C5=A1 > > --- > > dts/framework/dts.py | 128 ++++++++++++++++++++++++++++++++++++------= - > > dts/main.py | 8 ++- > > 2 files changed, 112 insertions(+), 24 deletions(-) > > > > diff --git a/dts/framework/dts.py b/dts/framework/dts.py > > index 4c7fb0c40a..331fed7dc4 100644 > > --- a/dts/framework/dts.py > > +++ b/dts/framework/dts.py > > @@ -3,6 +3,33 @@ > > # Copyright(c) 2022-2023 PANTHEON.tech s.r.o. > > # Copyright(c) 2022-2023 University of New Hampshire > > > > +r"""Test suite runner module. > > Is the r before the docstring intended? > Yes, this is because of :class:`~.framework.exception.DTSError`\s. Any alphabetical characters after backticks must be escaped for Sphinx to interpret the string correctly and the way to do that is to make the string raw (with r before the string). > > + > > +A DTS run is split into stages: > > + > > + #. Execution stage, > > + #. Build target stage, > > + #. Test suite stage, > > + #. Test case stage. > > + > > +The module is responsible for running tests on testbeds defined in the= test run configuration. > > +Each setup or teardown of each stage is recorded in a :class:`~framewo= rk.test_result.DTSResult` or > > +one of its subclasses. The test case results are also recorded. > > + > > +If an error occurs, the current stage is aborted, the error is recorde= d and the run continues in > > +the next iteration of the same stage. The return code is the highest `= severity` of all > > +:class:`~.framework.exception.DTSError`\s. > > Is the . before the classname intended? considering the previous one > doesn't have one. (I've not yet built the doc to check if it affect the > rendered doc) > Good catch. Not only is the dot suspect, but I looked at all references starting with the framework dir and the ones that refer to files in the same directory don't have to specify the full path if starting with a dot, such as: ~framework.test_result.DTSResult -> ~.test_result.DTSResult ~.framework.exception.DTSError -> ~.exception.DTSError test_result and exception are in the same dir as dts.py, so the above work. I'll make these changes in other files as well. > > + > > +Example: > > + An error occurs in a build target setup. The current build target = is aborted and the run > > + continues with the next build target. If the errored build target = was the last one in the given > > + execution, the next execution begins. > > + > > +Attributes: > > + dts_logger: The logger instance used in this module. > > + result: The top level result used in the module. > > +""" > > + > > import sys > > > > from .config import ( > > @@ -23,9 +50,38 @@ > > > > > > def run_all() -> None: > > - """ > > - The main process of DTS. Runs all build targets in all executions = from the main > > - config file. > > + """Run all build targets in all executions from the test run confi= guration. > > + > > + Before running test suites, executions and build targets are first= set up. > > + The executions and build targets defined in the test run configura= tion are iterated over. > > + The executions define which tests to run and where to run them and= build targets define > > + the DPDK build setup. > > + > > + The tests suites are set up for each execution/build target tuple = and each scheduled > > + test case within the test suite is set up, executed and torn down.= After all test cases > > + have been executed, the test suite is torn down and the next build= target will be tested. > > + > > + All the nested steps look like this: > > + > > + #. Execution setup > > + > > + #. Build target setup > > + > > + #. Test suite setup > > + > > + #. Test case setup > > + #. Test case logic > > + #. Test case teardown > > + > > + #. Test suite teardown > > + > > + #. Build target teardown > > + > > + #. Execution teardown > > + > > + The test cases are filtered according to the specification in the = test run configuration and > > + the :option:`--test-cases` command line argument or > > + the :envvar:`DTS_TESTCASES` environment variable. > > """ > > global dts_logger > > global result > > @@ -87,6 +143,8 @@ def run_all() -> None: > > > > > > def _check_dts_python_version() -> None: > > + """Check the required Python version - v3.10.""" > > + > > def RED(text: str) -> str: > > return f"\u001B[31;1m{str(text)}\u001B[0m" > > > > @@ -111,9 +169,16 @@ def _run_execution( > > execution: ExecutionConfiguration, > > result: DTSResult, > > ) -> None: > > - """ > > - Run the given execution. This involves running the execution setup= as well as > > - running all build targets in the given execution. > > + """Run the given execution. > > + > > + This involves running the execution setup as well as running all b= uild targets > > + in the given execution. After that, execution teardown is run. > > + > > + Args: > > + sut_node: The execution's SUT node. > > + tg_node: The execution's TG node. > > + execution: An execution's test run configuration. > > + result: The top level result object. > > """ > > dts_logger.info( > > f"Running execution with SUT '{execution.system_under_test_no= de.name}'." > > @@ -150,8 +215,18 @@ def _run_build_target( > > execution: ExecutionConfiguration, > > execution_result: ExecutionResult, > > ) -> None: > > - """ > > - Run the given build target. > > + """Run the given build target. > > + > > + This involves running the build target setup as well as running al= l test suites > > + in the given execution the build target is defined in. > > + After that, build target teardown is run. > > + > > + Args: > > + sut_node: The execution's SUT node. > > + tg_node: The execution's TG node. > > + build_target: A build target's test run configuration. > > + execution: The build target's execution's test run configurati= on. > > + execution_result: The execution level result object associated= with the execution. > > """ > > dts_logger.info(f"Running build target '{build_target.name}'.") > > build_target_result =3D execution_result.add_build_target(build_t= arget) > > @@ -183,10 +258,17 @@ def _run_all_suites( > > execution: ExecutionConfiguration, > > build_target_result: BuildTargetResult, > > ) -> None: > > - """ > > - Use the given build_target to run execution's test suites > > - with possibly only a subset of test cases. > > - If no subset is specified, run all test cases. > > + """Run the execution's (possibly a subset) test suites using the c= urrent build_target. > > + > > + The function assumes the build target we're testing has already be= en built on the SUT node. > > + The current build target thus corresponds to the current DPDK buil= d present on the SUT node. > > + > > + Args: > > + sut_node: The execution's SUT node. > > + tg_node: The execution's TG node. > > + execution: The execution's test run configuration associated w= ith the current build target. > > + build_target_result: The build target level result object asso= ciated > > + with the current build target. > > """ > > end_build_target =3D False > > if not execution.skip_smoke_tests: > > @@ -215,16 +297,22 @@ def _run_single_suite( > > build_target_result: BuildTargetResult, > > test_suite_config: TestSuiteConfig, > > ) -> None: > > - """Runs a single test suite. > > + """Run all test suite in a single test suite module. > > + > > + The function assumes the build target we're testing has already be= en built on the SUT node. > > + The current build target thus corresponds to the current DPDK buil= d present on the SUT node. > > > > Args: > > - sut_node: Node to run tests on. > > - execution: Execution the test case belongs to. > > - build_target_result: Build target configuration test case is r= un on > > - test_suite_config: Test suite configuration > > + sut_node: The execution's SUT node. > > + tg_node: The execution's TG node. > > + execution: The execution's test run configuration associated w= ith the current build target. > > + build_target_result: The build target level result object asso= ciated > > + with the current build target. > > + test_suite_config: Test suite test run configuration specifyin= g the test suite module > > + and possibly a subset of test cases of test suites in that= module. > > > > Raises: > > - BlockingTestSuiteError: If a test suite that was marked as blo= cking fails. > > + BlockingTestSuiteError: If a blocking test suite fails. > > """ > > try: > > full_suite_path =3D f"tests.TestSuite_{test_suite_config.test= _suite}" > > @@ -248,9 +336,7 @@ def _run_single_suite( > > > > > > def _exit_dts() -> None: > > - """ > > - Process all errors and exit with the proper exit code. > > - """ > > + """Process all errors and exit with the proper exit code.""" > > result.process() > > > > if dts_logger: > > diff --git a/dts/main.py b/dts/main.py > > index 5d4714b0c3..f703615d11 100755 > > --- a/dts/main.py > > +++ b/dts/main.py > > @@ -4,9 +4,7 @@ > > # Copyright(c) 2022 PANTHEON.tech s.r.o. > > # Copyright(c) 2022 University of New Hampshire > > > > -""" > > -A test framework for testing DPDK. > > -""" > > +"""The DTS executable.""" > > > > import logging > > > > @@ -17,6 +15,10 @@ def main() -> None: > > """Set DTS settings, then run DTS. > > > > The DTS settings are taken from the command line arguments and th= e environment variables. > > + The settings object is stored in the module-level variable setting= s.SETTINGS which the entire > > + framework uses. After importing the module (or the variable), any = changes to the variable are > > + not going to be reflected without a re-import. This means that the= SETTINGS variable must > > + be modified before the settings module is imported anywhere else i= n the framework. > > """ > > settings.SETTINGS =3D settings.get_settings() > > from framework import dts > Nit: copyright notice update in main Nice catch again. Thanks.