From patchwork Mon Jan 22 18:26:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Luca Vizzarro X-Patchwork-Id: 136041 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E07664399C; Mon, 22 Jan 2024 19:26:40 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 440E040DDA; Mon, 22 Jan 2024 19:26:36 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id BC83640298 for ; Mon, 22 Jan 2024 19:26:34 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5BE57FEC; Mon, 22 Jan 2024 10:27:20 -0800 (PST) Received: from localhost.localdomain (unknown [10.57.90.210]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 91B1E3F762; Mon, 22 Jan 2024 10:26:33 -0800 (PST) From: Luca Vizzarro To: dev@dpdk.org Cc: Luca Vizzarro , =?utf-8?q?Juraj_Linke=C5=A1?= , Paul Szczepanek Subject: [PATCH 1/4] dts: constrain DPDK source flag Date: Mon, 22 Jan 2024 18:26:08 +0000 Message-Id: <20240122182611.1904974-2-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240122182611.1904974-1-luca.vizzarro@arm.com> References: <20240122182611.1904974-1-luca.vizzarro@arm.com> MIME-Version: 1.0 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 DTS needs an input to gather the DPDK source code from. This is then built on the remote target. This commit makes sure that this input is more constrained, separating the Git revision ID – used to create a tarball using Git – and providing tarballed source code directly, while retaining mutual exclusion. This makes the code more readable and easier to handle for input validation, of which this commit introduces a basic one based on the pre-existing code. Moreover it ensures that these flags are explicitly required to be set by the user, dropping a default value. It also aids the user understand how to use the DTS in the scenario it is ran without any arguments set. Reviewed-by: Paul Szczepanek Signed-off-by: Luca Vizzarro --- doc/guides/tools/dts.rst | 8 +++-- dts/framework/settings.py | 64 ++++++++++++++++++++++++++++----------- dts/framework/utils.py | 43 ++++++++++++++++---------- 3 files changed, 79 insertions(+), 36 deletions(-) diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst index 846696e14e..6e2da317e8 100644 --- a/doc/guides/tools/dts.rst +++ b/doc/guides/tools/dts.rst @@ -215,12 +215,16 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet .. code-block:: console (dts-py3.10) $ ./main.py --help - usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--tarball TARBALL] [--compile-timeout COMPILE_TIMEOUT] [--test-cases TEST_CASES] [--re-run RE_RUN] + usage: main.py [-h] (--tarball FILEPATH | --revision ID) [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--compile-timeout COMPILE_TIMEOUT] [--test-cases TEST_CASES] [--re-run RE_RUN] Run DPDK test suites. All options may be specified with the environment variables provided in brackets. Command line arguments have higher priority. options: -h, --help show this help message and exit + --tarball FILEPATH, --snapshot FILEPATH + Path to DPDK source code tarball to test. (default: None) + --revision ID, --rev ID, --git-ref ID + Git revision ID to test. Could be commit, tag, tree ID and vice versa. To test local changes, first commit them, then use their commit ID (default: None) --config-file CONFIG_FILE [DTS_CFG_FILE] configuration file that describes the test cases, SUTs and targets. (default: ./conf.yaml) --output-dir OUTPUT_DIR, --output OUTPUT_DIR @@ -229,8 +233,6 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet [DTS_TIMEOUT] The default timeout for all DTS operations except for compiling DPDK. (default: 15) -v, --verbose [DTS_VERBOSE] Specify to enable verbose output, logging all messages to the console. (default: False) -s, --skip-setup [DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes. (default: None) - --tarball TARBALL, --snapshot TARBALL, --git-ref TARBALL - [DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID, tag ID or tree ID to test. To test local changes, first commit them, then use the commit ID with this option. (default: dpdk.tar.xz) --compile-timeout COMPILE_TIMEOUT [DTS_COMPILE_TIMEOUT] The timeout for compiling DPDK. (default: 1200) --test-cases TEST_CASES diff --git a/dts/framework/settings.py b/dts/framework/settings.py index 609c8d0e62..2d0365e763 100644 --- a/dts/framework/settings.py +++ b/dts/framework/settings.py @@ -76,7 +76,8 @@ from pathlib import Path from typing import Any, TypeVar -from .utils import DPDKGitTarball +from .exception import ConfigurationError +from .utils import DPDKGitTarball, get_commit_id _T = TypeVar("_T") @@ -149,6 +150,26 @@ def __call__( return _EnvironmentArgument +def _parse_tarball(filepath: str) -> Path: + """Validate if the filepath is valid and return a Path object.""" + path = Path(filepath) + if not path.exists() or not path.is_file(): + raise argparse.ArgumentTypeError( + "the file path provided is not a valid file") + return path + + +def _parse_revision_id(rev_id: str) -> str: + """Retrieve effective commit ID from a revision ID. While validating it.""" + + try: + return get_commit_id(rev_id) + except ConfigurationError: + raise argparse.ArgumentTypeError( + "the Git revision ID supplied is invalid or ambiguous" + ) + + @dataclass(slots=True) class Settings: """Default framework-wide user settings. @@ -167,7 +188,7 @@ class Settings: #: skip_setup: bool = False #: - dpdk_tarball_path: Path | str = "dpdk.tar.xz" + dpdk_tarball_path: Path | str = "" #: compile_timeout: float = 1200 #: @@ -186,6 +207,28 @@ def _get_parser() -> argparse.ArgumentParser: formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) + dpdk_source = parser.add_mutually_exclusive_group(required=True) + + dpdk_source.add_argument( + "--tarball", + "--snapshot", + action='store', + type=_parse_tarball, + help="Path to DPDK source code tarball to test.", + metavar="FILEPATH", + ) + + dpdk_source.add_argument( + "--revision", + "--rev", + "--git-ref", + type=_parse_revision_id, + metavar="ID", + help="Git revision ID to test. Could be commit, tag, tree ID and " + "vice versa. To test local changes, first commit them, then use their " + "commit ID", + ) + parser.add_argument( "--config-file", action=_env_arg("DTS_CFG_FILE"), @@ -229,18 +272,6 @@ def _get_parser() -> argparse.ArgumentParser: help="[DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes.", ) - parser.add_argument( - "--tarball", - "--snapshot", - "--git-ref", - action=_env_arg("DTS_DPDK_TARBALL"), - default=SETTINGS.dpdk_tarball_path, - type=Path, - help="[DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID, " - "tag ID or tree ID to test. To test local changes, first commit them, " - "then use the commit ID with this option.", - ) - parser.add_argument( "--compile-timeout", action=_env_arg("DTS_COMPILE_TIMEOUT"), @@ -283,9 +314,8 @@ def get_settings() -> Settings: verbose=parsed_args.verbose, skip_setup=parsed_args.skip_setup, dpdk_tarball_path=Path( - Path(DPDKGitTarball(parsed_args.tarball, parsed_args.output_dir)) - if not os.path.exists(parsed_args.tarball) - else Path(parsed_args.tarball) + parsed_args.tarball or + DPDKGitTarball(parsed_args.revision, parsed_args.output_dir) ), compile_timeout=parsed_args.compile_timeout, test_cases=(parsed_args.test_cases.split(",") if parsed_args.test_cases else []), diff --git a/dts/framework/utils.py b/dts/framework/utils.py index cc5e458cc8..dbbec8b00a 100644 --- a/dts/framework/utils.py +++ b/dts/framework/utils.py @@ -70,6 +70,32 @@ def get_packet_summaries(packets: list[Packet]) -> str: return f"Packet contents: \n{packet_summaries}" +def get_commit_id(rev_id: str) -> str: + """Given a Git revision ID, return the corresponding commit ID. + + Args: + rev_id: The Git revision ID. + + Raises: + ConfigurationError: The ``git rev-parse`` command failed. Suggesting + an invalid or ambiguous revision ID was supplied. + """ + result = subprocess.run( + ["git", "rev-parse", "--verify", rev_id], + text=True, + capture_output=True, + ) + if result.returncode != 0: + raise ConfigurationError( + f"{rev_id} is neither a path to an existing DPDK " + "archive nor a valid git reference.\n" + f"Command: {result.args}\n" + f"Stdout: {result.stdout}\n" + f"Stderr: {result.stderr}" + ) + return result.stdout.strip() + + class StrEnum(Enum): """Enum with members stored as strings.""" @@ -170,7 +196,6 @@ def __init__( self._tarball_dir = Path(output_dir, "tarball") - self._get_commit_id() self._create_tarball_dir() self._tarball_name = ( @@ -180,21 +205,7 @@ def __init__( if not self._tarball_path: self._create_tarball() - def _get_commit_id(self) -> None: - result = subprocess.run( - ["git", "rev-parse", "--verify", self._git_ref], - text=True, - capture_output=True, - ) - if result.returncode != 0: - raise ConfigurationError( - f"{self._git_ref} is neither a path to an existing DPDK " - "archive nor a valid git reference.\n" - f"Command: {result.args}\n" - f"Stdout: {result.stdout}\n" - f"Stderr: {result.stderr}" - ) - self._git_ref = result.stdout.strip() + def _create_tarball_dir(self) -> None: os.makedirs(self._tarball_dir, exist_ok=True) From patchwork Mon Jan 22 18:26:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Vizzarro X-Patchwork-Id: 136042 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3C9784399C; Mon, 22 Jan 2024 19:26:47 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 48C6A40ED2; Mon, 22 Jan 2024 19:26:37 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 9109140A6C for ; Mon, 22 Jan 2024 19:26:35 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3B7FA106F; Mon, 22 Jan 2024 10:27:21 -0800 (PST) Received: from localhost.localdomain (unknown [10.57.90.210]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 742E43F762; Mon, 22 Jan 2024 10:26:34 -0800 (PST) From: Luca Vizzarro To: dev@dpdk.org Cc: Luca Vizzarro , =?utf-8?q?Juraj_Linke=C5=A1?= , Paul Szczepanek Subject: [PATCH 2/4] dts: customise argparse error message Date: Mon, 22 Jan 2024 18:26:09 +0000 Message-Id: <20240122182611.1904974-3-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240122182611.1904974-1-luca.vizzarro@arm.com> References: <20240122182611.1904974-1-luca.vizzarro@arm.com> MIME-Version: 1.0 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 This commit customises the arguments parsing class' error message, making it so the confusing usage is not displayed in these occurrences, but the user is redirected to use the help argument instead. Reviewed-by: Paul Szczepanek Signed-off-by: Luca Vizzarro --- dts/framework/settings.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dts/framework/settings.py b/dts/framework/settings.py index 2d0365e763..acfe5cad44 100644 --- a/dts/framework/settings.py +++ b/dts/framework/settings.py @@ -170,6 +170,15 @@ def _parse_revision_id(rev_id: str) -> str: ) +class ArgumentParser(argparse.ArgumentParser): + """ArgumentParser with a custom error message.""" + def error(self, message): + print(f"{self.prog}: error: {message}\n", file=sys.stderr) + self.exit(2, + "For help and usage, " + "run the command with the --help flag.\n") + + @dataclass(slots=True) class Settings: """Default framework-wide user settings. @@ -200,8 +209,8 @@ class Settings: SETTINGS: Settings = Settings() -def _get_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser( +def _get_parser() -> ArgumentParser: + parser = ArgumentParser( description="Run DPDK test suites. All options may be specified with the environment " "variables provided in brackets. Command line arguments have higher priority.", formatter_class=argparse.ArgumentDefaultsHelpFormatter, From patchwork Mon Jan 22 18:26:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Vizzarro X-Patchwork-Id: 136043 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A89DD4399C; Mon, 22 Jan 2024 19:26:52 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6505E42D2B; Mon, 22 Jan 2024 19:26:38 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 6908840E13 for ; Mon, 22 Jan 2024 19:26:36 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1C0CE1FB; Mon, 22 Jan 2024 10:27:22 -0800 (PST) Received: from localhost.localdomain (unknown [10.57.90.210]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 5346A3F762; Mon, 22 Jan 2024 10:26:35 -0800 (PST) From: Luca Vizzarro To: dev@dpdk.org Cc: Luca Vizzarro , =?utf-8?q?Juraj_Linke=C5=A1?= , Paul Szczepanek Subject: [PATCH 3/4] dts: show help when DTS is ran without args Date: Mon, 22 Jan 2024 18:26:10 +0000 Message-Id: <20240122182611.1904974-4-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240122182611.1904974-1-luca.vizzarro@arm.com> References: <20240122182611.1904974-1-luca.vizzarro@arm.com> MIME-Version: 1.0 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 This commit changes the default behaviour of DTS, making it so that the user automatically sees the help and usage page when running it without any arguments set. Instead of being welcomed by an error message. Reviewed-by: Paul Szczepanek Signed-off-by: Luca Vizzarro --- dts/framework/settings.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dts/framework/settings.py b/dts/framework/settings.py index acfe5cad44..5809fd4e91 100644 --- a/dts/framework/settings.py +++ b/dts/framework/settings.py @@ -71,6 +71,7 @@ import argparse import os +import sys from collections.abc import Callable, Iterable, Sequence from dataclasses import dataclass, field from pathlib import Path @@ -315,6 +316,11 @@ def get_settings() -> Settings: The inputs are taken from the command line and from environment variables. """ + + if len(sys.argv) == 1: + _get_parser().print_help() + sys.exit(1) + parsed_args = _get_parser().parse_args() return Settings( config_file_path=parsed_args.config_file, From patchwork Mon Jan 22 18:26:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Vizzarro X-Patchwork-Id: 136044 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 65E7A4399C; Mon, 22 Jan 2024 19:26:58 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7EC5942DB2; Mon, 22 Jan 2024 19:26:39 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 4326D40E72 for ; Mon, 22 Jan 2024 19:26:37 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F24DDFEC; Mon, 22 Jan 2024 10:27:22 -0800 (PST) Received: from localhost.localdomain (unknown [10.57.90.210]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 351603F762; Mon, 22 Jan 2024 10:26:36 -0800 (PST) From: Luca Vizzarro To: dev@dpdk.org Cc: Luca Vizzarro , =?utf-8?q?Juraj_Linke=C5=A1?= , Paul Szczepanek Subject: [PATCH 4/4] dts: log stderr with failed remote commands Date: Mon, 22 Jan 2024 18:26:11 +0000 Message-Id: <20240122182611.1904974-5-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240122182611.1904974-1-luca.vizzarro@arm.com> References: <20240122182611.1904974-1-luca.vizzarro@arm.com> MIME-Version: 1.0 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 Add the executed command stderr to RemoteCommandExecutionError. So that it is logged as an error, instead of just as debug. Reviewed-by: Paul Szczepanek Signed-off-by: Luca Vizzarro --- dts/framework/exception.py | 10 +++++++--- dts/framework/remote_session/remote_session.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/dts/framework/exception.py b/dts/framework/exception.py index 658eee2c38..9fc3fa096a 100644 --- a/dts/framework/exception.py +++ b/dts/framework/exception.py @@ -130,20 +130,24 @@ class RemoteCommandExecutionError(DTSError): #: The executed command. command: str _command_return_code: int + _command_stderr: str - def __init__(self, command: str, command_return_code: int): + def __init__(self, command: str, command_return_code: int, command_stderr: str): """Define the meaning of the first two arguments. Args: command: The executed command. command_return_code: The return code of the executed command. + command_stderr: The stderr of the executed command. """ self.command = command self._command_return_code = command_return_code + self._command_stderr = command_stderr def __str__(self) -> str: - """Include both the command and return code in the string representation.""" - return f"Command {self.command} returned a non-zero exit code: {self._command_return_code}" + """Include the command, its return code and stderr in the string representation.""" + return (f"Command '{self.command}' returned a non-zero exit code: " + f"{self._command_return_code}\n{self._command_stderr}") class RemoteDirectoryExistsError(DTSError): diff --git a/dts/framework/remote_session/remote_session.py b/dts/framework/remote_session/remote_session.py index 2059f9a981..345439f2de 100644 --- a/dts/framework/remote_session/remote_session.py +++ b/dts/framework/remote_session/remote_session.py @@ -157,7 +157,7 @@ def send_command( ) self._logger.debug(f"stdout: '{result.stdout}'") self._logger.debug(f"stderr: '{result.stderr}'") - raise RemoteCommandExecutionError(command, result.return_code) + raise RemoteCommandExecutionError(command, result.return_code, result.stderr) self._logger.debug(f"Received from '{command}':\n{result}") self.history.append(result) return result