<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:arial,sans-serif"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Format according to the Google format and PEP257, with slight<br>
deviations.<br>
<br>
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech><br>
---<br>
.../interactive_remote_session.py | 36 +++----<br>
.../remote_session/interactive_shell.py | 99 +++++++++++--------<br>
dts/framework/remote_session/python_shell.py | 26 ++++-<br>
dts/framework/remote_session/testpmd_shell.py | 58 +++++++++--<br>
4 files changed, 149 insertions(+), 70 deletions(-)<br>
<br>
diff --git a/dts/framework/remote_session/interactive_remote_session.py b/dts/framework/remote_session/interactive_remote_session.py<br>
index 098ded1bb0..1cc82e3377 100644<br>
--- a/dts/framework/remote_session/interactive_remote_session.py<br>
+++ b/dts/framework/remote_session/interactive_remote_session.py<br>
@@ -22,27 +22,23 @@<br>
class InteractiveRemoteSession:<br>
"""SSH connection dedicated to interactive applications.<br>
<br>
- This connection is created using paramiko and is a persistent connection to the<br>
- host. This class defines methods for connecting to the node and configures this<br>
- connection to send "keep alive" packets every 30 seconds. Because paramiko attempts<br>
- to use SSH keys to establish a connection first, providing a password is optional.<br>
- This session is utilized by InteractiveShells and cannot be interacted with<br>
- directly.<br>
-<br>
- Arguments:<br>
- node_config: Configuration class for the node you are connecting to.<br>
- _logger: Desired logger for this session to use.<br>
+ The connection is created using `paramiko <<a href="https://docs.paramiko.org/en/latest/" rel="noreferrer" target="_blank">https://docs.paramiko.org/en/latest/</a>>`_<br>
+ and is a persistent connection to the host. This class defines the methods for connecting<br>
+ to the node and configures the connection to send "keep alive" packets every 30 seconds.<br>
+ Because paramiko attempts to use SSH keys to establish a connection first, providing<br>
+ a password is optional. This session is utilized by InteractiveShells<br>
+ and cannot be interacted with directly.<br>
<br>
Attributes:<br>
- hostname: Hostname that will be used to initialize a connection to the node.<br>
- ip: A subsection of hostname that removes the port for the connection if there<br>
+ hostname: The hostname that will be used to initialize a connection to the node.<br>
+ ip: A subsection of `hostname` that removes the port for the connection if there<br>
is one. If there is no port, this will be the same as hostname.<br>
- port: Port to use for the ssh connection. This will be extracted from the<br>
- hostname if there is a port included, otherwise it will default to 22.<br>
+ port: Port to use for the ssh connection. This will be extracted from `hostname`<br>
+ if there is a port included, otherwise it will default to ``22``.<br>
username: User to connect to the node with.<br>
password: Password of the user connecting to the host. This will default to an<br>
empty string if a password is not provided.<br>
- session: Underlying paramiko connection.<br>
+ session: The underlying paramiko connection.<br>
<br>
Raises:<br>
SSHConnectionError: There is an error creating the SSH connection.<br>
@@ -58,9 +54,15 @@ class InteractiveRemoteSession:<br>
_node_config: NodeConfiguration<br>
_transport: Transport | None<br>
<br>
- def __init__(self, node_config: NodeConfiguration, _logger: DTSLOG) -> None:<br>
+ def __init__(self, node_config: NodeConfiguration, logger: DTSLOG) -> None:<br>
+ """Connect to the node during initialization.<br>
+<br>
+ Args:<br>
+ node_config: The test run configuration of the node to connect to.<br>
+ logger: The logger instance this session will use.<br>
+ """<br>
self._node_config = node_config<br>
- self._logger = _logger<br>
+ self._logger = logger<br>
self.hostname = node_config.hostname<br>
self.username = node_config.user<br>
self.password = node_config.password if node_config.password else ""<br>
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py<br>
index 4db19fb9b3..b158f963b6 100644<br>
--- a/dts/framework/remote_session/interactive_shell.py<br>
+++ b/dts/framework/remote_session/interactive_shell.py<br>
@@ -3,18 +3,20 @@<br>
<br>
"""Common functionality for interactive shell handling.<br>
<br>
-This base class, InteractiveShell, is meant to be extended by other classes that<br>
-contain functionality specific to that shell type. These derived classes will often<br>
-modify things like the prompt to expect or the arguments to pass into the application,<br>
-but still utilize the same method for sending a command and collecting output. How<br>
-this output is handled however is often application specific. If an application needs<br>
-elevated privileges to start it is expected that the method for gaining those<br>
-privileges is provided when initializing the class.<br>
+The base class, :class:`InteractiveShell`, is meant to be extended by subclasses that contain<br>
+functionality specific to that shell type. These subclasses will often modify things like<br>
+the prompt to expect or the arguments to pass into the application, but still utilize<br>
+the same method for sending a command and collecting output. How this output is handled however<br>
+is often application specific. If an application needs elevated privileges to start it is expected<br>
+that the method for gaining those privileges is provided when initializing the class.<br>
+<br>
+The :option:`--timeout` command line argument and the :envvar:`DTS_TIMEOUT`<br>
+environment variable configure the timeout of getting the output from command execution.<br>
"""<br>
<br>
from abc import ABC<br>
from pathlib import PurePath<br>
-from typing import Callable<br>
+from typing import Callable, ClassVar<br>
<br>
from paramiko import Channel, SSHClient, channel # type: ignore[import]<br>
<br>
@@ -30,28 +32,6 @@ class InteractiveShell(ABC):<br>
and collecting input until reaching a certain prompt. All interactive applications<br>
will use the same SSH connection, but each will create their own channel on that<br>
session.<br>
-<br>
- Arguments:<br>
- interactive_session: The SSH session dedicated to interactive shells.<br>
- logger: Logger used for displaying information in the console.<br>
- get_privileged_command: Method for modifying a command to allow it to use<br>
- elevated privileges. If this is None, the application will not be started<br>
- with elevated privileges.<br>
- app_args: Command line arguments to be passed to the application on startup.<br>
- timeout: Timeout used for the SSH channel that is dedicated to this interactive<br>
- shell. This timeout is for collecting output, so if reading from the buffer<br>
- and no output is gathered within the timeout, an exception is thrown.<br>
-<br>
- Attributes<br>
- _default_prompt: Prompt to expect at the end of output when sending a command.<br>
- This is often overridden by derived classes.<br>
- _command_extra_chars: Extra characters to add to the end of every command<br>
- before sending them. This is often overridden by derived classes and is<br>
- most commonly an additional newline character.<br>
- path: Path to the executable to start the interactive application.<br>
- dpdk_app: Whether this application is a DPDK app. If it is, the build<br>
- directory for DPDK on the node will be prepended to the path to the<br>
- executable.<br>
"""<br>
<br>
_interactive_session: SSHClient<br>
@@ -61,10 +41,22 @@ class InteractiveShell(ABC):<br>
_logger: DTSLOG<br>
_timeout: float<br>
_app_args: str<br>
- _default_prompt: str = ""<br>
- _command_extra_chars: str = ""<br>
- path: PurePath<br>
- dpdk_app: bool = False<br>
+<br>
+ #: Prompt to expect at the end of output when sending a command.<br>
+ #: This is often overridden by subclasses.<br>
+ _default_prompt: ClassVar[str] = ""<br>
+<br>
+ #: Extra characters to add to the end of every command<br>
+ #: before sending them. This is often overridden by subclasses and is<br>
+ #: most commonly an additional newline character.<br>
+ _command_extra_chars: ClassVar[str] = ""<br>
+<br>
+ #: Path to the executable to start the interactive application.<br>
+ path: ClassVar[PurePath]<br>
+<br>
+ #: Whether this application is a DPDK app. If it is, the build directory<br>
+ #: for DPDK on the node will be prepended to the path to the executable.<br>
+ dpdk_app: ClassVar[bool] = False<br>
<br>
def __init__(<br>
self,<br>
@@ -74,6 +66,19 @@ def __init__(<br>
app_args: str = "",<br>
timeout: float = SETTINGS.timeout,<br>
) -> None:<br>
+ """Create an SSH channel during initialization.<br>
+<br>
+ Args:<br>
+ interactive_session: The SSH session dedicated to interactive shells.<br>
+ logger: The logger instance this session will use.<br>
+ get_privileged_command: A method for modifying a command to allow it to use<br>
+ elevated privileges. If :data:`None`, the application will not be started<br>
+ with elevated privileges.<br>
+ app_args: The command line arguments to be passed to the application on startup.<br>
+ timeout: The timeout used for the SSH channel that is dedicated to this interactive<br>
+ shell. This timeout is for collecting output, so if reading from the buffer<br>
+ and no output is gathered within the timeout, an exception is thrown.<br>
+ """<br>
self._interactive_session = interactive_session<br>
self._ssh_channel = self._interactive_session.invoke_shell()<br>
self._stdin = self._ssh_channel.makefile_stdin("w")<br>
@@ -90,6 +95,10 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None<br>
<br>
This method is often overridden by subclasses as their process for<br>
starting may look different.<br>
+<br>
+ Args:<br>
+ get_privileged_command: A function (but could be any callable) that produces<br>
+ the version of the command with elevated privileges.<br>
"""<br>
start_command = f"{self.path} {self._app_args}"<br>
if get_privileged_command is not None:<br>
@@ -97,16 +106,24 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None<br>
self.send_command(start_command)<br>
<br>
def send_command(self, command: str, prompt: str | None = None) -> str:<br>
- """Send a command and get all output before the expected ending string.<br>
+ """Send `command` and get all output before the expected ending string.<br>
<br>
Lines that expect input are not included in the stdout buffer, so they cannot<br>
- be used for expect. For example, if you were prompted to log into something<br>
- with a username and password, you cannot expect "username:" because it won't<br>
- yet be in the stdout buffer. A workaround for this could be consuming an<br>
- extra newline character to force the current prompt into the stdout buffer.<br>
+ be used for expect.<br>
+<br>
+ Example:<br>
+ If you were prompted to log into something with a username and password,<br>
+ you cannot expect ``username:`` because it won't yet be in the stdout buffer.<br>
+ A workaround for this could be consuming an extra newline character to force<br>
+ the current `prompt` into the stdout buffer.<br>
+<br>
+ Args:<br>
+ command: The command to send.<br>
+ prompt: After sending the command, `send_command` will be expecting this string.<br>
+ If :data:`None`, will use the class's default prompt.<br>
<br>
Returns:<br>
- All output in the buffer before expected string<br>
+ All output in the buffer before expected string.<br>
"""<br>
self._<a href="http://logger.info" rel="noreferrer" target="_blank">logger.info</a>(f"Sending: '{command}'")<br>
if prompt is None:<br>
@@ -124,8 +141,10 @@ def send_command(self, command: str, prompt: str | None = None) -> str:<br>
return out<br>
<br>
def close(self) -> None:<br>
+ """Properly free all resources."""<br>
self._stdin.close()<br>
self._ssh_channel.close()<br>
<br>
def __del__(self) -> None:<br>
+ """Make sure the session is properly closed before deleting the object."""<br>
self.close()<br>
diff --git a/dts/framework/remote_session/python_shell.py b/dts/framework/remote_session/python_shell.py<br>
index cc3ad48a68..ccfd3783e8 100644<br>
--- a/dts/framework/remote_session/python_shell.py<br>
+++ b/dts/framework/remote_session/python_shell.py<br>
@@ -1,12 +1,32 @@<br>
# SPDX-License-Identifier: BSD-3-Clause<br>
# Copyright(c) 2023 PANTHEON.tech s.r.o.<br>
<br>
+"""Python interactive shell.<br>
+<br>
+Typical usage example in a TestSuite::<br>
+<br>
+ from framework.remote_session import PythonShell<br>
+ python_shell = self.tg_node.create_interactive_shell(<br>
+ PythonShell, timeout=5, privileged=True<br>
+ )<br>
+ python_shell.send_command("print('Hello World')")<br>
+ python_shell.close()<br>
+"""<br>
+<br>
from pathlib import PurePath<br>
+from typing import ClassVar<br>
<br>
from .interactive_shell import InteractiveShell<br>
<br>
<br>
class PythonShell(InteractiveShell):<br>
- _default_prompt: str = ">>>"<br>
- _command_extra_chars: str = "\n"<br>
- path: PurePath = PurePath("python3")<br>
+ """Python interactive shell."""<br>
+<br>
+ #: Python's prompt.<br>
+ _default_prompt: ClassVar[str] = ">>>"<br>
+<br>
+ #: This forces the prompt to appear after sending a command.<br>
+ _command_extra_chars: ClassVar[str] = "\n"<br>
+<br>
+ #: The Python executable.<br>
+ path: ClassVar[PurePath] = PurePath("python3")<br>
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py<br>
index 08ac311016..79481e845c 100644<br>
--- a/dts/framework/remote_session/testpmd_shell.py<br>
+++ b/dts/framework/remote_session/testpmd_shell.py<br>
@@ -1,41 +1,79 @@<br>
# SPDX-License-Identifier: BSD-3-Clause<br>
# Copyright(c) 2023 University of New Hampshire<br>
<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,sans-serif">Should you add to the copyright here for adding comments?</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+"""Testpmd interactive shell.<br>
+<br>
+Typical usage example in a TestSuite::<br>
+<br>
+ testpmd_shell = self.sut_node.create_interactive_shell(<br>
+ TestPmdShell, privileged=True<br>
+ )<br>
+ devices = testpmd_shell.get_devices()<br>
+ for device in devices:<br>
+ print(device)<br>
+ testpmd_shell.close()<br>
+"""<br>
+<br>
from pathlib import PurePath<br>
-from typing import Callable<br>
+from typing import Callable, ClassVar<br>
<br>
from .interactive_shell import InteractiveShell<br>
<br>
<br>
class TestPmdDevice(object):<br>
+ """The data of a device that testpmd can recognize.<br>
+<br>
+ Attributes:<br>
+ pci_address: The PCI address of the device.<br>
+ """<br>
+<br>
pci_address: str<br>
<br>
def __init__(self, pci_address_line: str):<br>
+ """Initialize the device from the testpmd output line string.<br>
+<br>
+ Args:<br>
+ pci_address_line: A line of testpmd output that contains a device.<br>
+ """<br>
self.pci_address = pci_address_line.strip().split(": ")[1].strip()<br>
<br>
def __str__(self) -> str:<br>
+ """The PCI address captures what the device is."""<br>
return self.pci_address<br>
<br>
<br>
class TestPmdShell(InteractiveShell):<br>
- path: PurePath = PurePath("app", "dpdk-testpmd")<br>
- dpdk_app: bool = True<br>
- _default_prompt: str = "testpmd>"<br>
- _command_extra_chars: str = "\n" # We want to append an extra newline to every command<br>
+ """Testpmd interactive shell.<br>
+<br>
+ The testpmd shell users should never use<br>
+ the :meth:`~.interactive_shell.InteractiveShell.send_command` method directly, but rather<br>
+ call specialized methods. If there isn't one that satisfies a need, it should be added.<br>
+ """<br>
+<br>
+ #: The path to the testpmd executable.<br>
+ path: ClassVar[PurePath] = PurePath("app", "dpdk-testpmd")<br>
+<br>
+ #: Flag this as a DPDK app so that it's clear this is not a system app and<br>
+ #: needs to be looked in a specific path.<br>
+ dpdk_app: ClassVar[bool] = True<br>
+<br>
+ #: The testpmd's prompt.<br>
+ _default_prompt: ClassVar[str] = "testpmd>"<br>
+<br>
+ #: This forces the prompt to appear after sending a command.<br>
+ _command_extra_chars: ClassVar[str] = "\n"<br>
<br>
def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:<br>
- """See "_start_application" in InteractiveShell."""<br>
self._app_args += " -- -i"<br>
super()._start_application(get_privileged_command)<br>
<br>
def get_devices(self) -> list[TestPmdDevice]:<br>
- """Get a list of device names that are known to testpmd<br>
+ """Get a list of device names that are known to testpmd.<br>
<br>
- Uses the device info listed in testpmd and then parses the output to<br>
- return only the names of the devices.<br>
+ Uses the device info listed in testpmd and then parses the output.<br>
<br>
Returns:<br>
- A list of strings representing device names (e.g. 0000:14:00.1)<br>
+ A list of devices.<br>
"""<br>
dev_info: str = self.send_command("show device info all")<br>
dev_list: list[TestPmdDevice] = []<br>
-- <br>
2.34.1<br>
<br>
</blockquote></div></div>