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 1D77142B91; Wed, 24 May 2023 22:45:14 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A1B7E4067E; Wed, 24 May 2023 22:45:13 +0200 (CEST) Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) by mails.dpdk.org (Postfix) with ESMTP id AD27440156 for ; Wed, 24 May 2023 22:45:11 +0200 (CEST) Received: by mail-lj1-f172.google.com with SMTP id 38308e7fff4ca-2af189d323fso2933511fa.1 for ; Wed, 24 May 2023 13:45:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1684961111; x=1687553111; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=oM9OcEmqgcjK0ZK+C1YUpcBsPcmQQWg9DTYtUZIPgR4=; b=TpTsjuA3EI9bpj17hSaFVMW4+yERmx1i6T6ttjEtbNnPUYzUwZ9JUwgr34fPPnKAZm 3Om5y1CDQfAbG6D9rV5ORpiqSQkV+ejL5ANLxl0xtHs5HybZiS2ZRKZG2kFs1wmoQpag 0DgsHDeH5qFGePgjdtm/ZwKso4Ovnh6o3YlZA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684961111; x=1687553111; h=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=oM9OcEmqgcjK0ZK+C1YUpcBsPcmQQWg9DTYtUZIPgR4=; b=lC7EpfHHzNW1q17hAq907mev+6Qd8WEllD/gL4YDzx+8uyC0ed4tjGbwkyCalRdkLl /w8MkD4HL1eMWj7UUVOI1PPdcYNJGbLOXF2p3LjBJjcvsGxuWPZCaXgiAASnFqEePgHX ESQrIJ9WGD9B2tqysVIKC9u5c9RtuaD9SlE0yy/JZnY0xSl/nT2J67pvqBxo2wYLSZQ6 KZBtAbmwmU1c1PVW5DGfEMvhOir9Fn5Zzk9Mi0m0rpCLRvLUPfd5QGtva0eaDO35GqBI rIKQOWuDG5VXtBp8jaYULLCfXTi9m7GwI6wExueH99z898v/cx3ihZ4aToHEpYdGrtcR 0EgA== X-Gm-Message-State: AC+VfDxw613HwM5mFKPAaS21ayqRzRujmwfUcpfrDfXqJBvB6dU5SdqQ Ki1vaZ3oiMjn5mPIW27M5YbeYGkF4csN20zT9BonKlG+A9kS/DCi X-Google-Smtp-Source: ACHHUZ4ZT/9VC4zGJaBtEG2f3L49PtYIr697z4RwwvJ5zwiwhi/OiHpk7F8vKIMY+8OQ41F7fmrKmj1ZIV8qA03C84M= X-Received: by 2002:a05:651c:2046:b0:2af:222d:9695 with SMTP id t6-20020a05651c204600b002af222d9695mr163452ljo.9.1684961110752; Wed, 24 May 2023 13:45:10 -0700 (PDT) MIME-Version: 1.0 References: <20230512192540.401-2-jspewock@iol.unh.edu> <20230512192540.401-3-jspewock@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Wed, 24 May 2023 16:44:57 -0400 Message-ID: Subject: Re: [RFC v2 1/2] dts: add smoke tests To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: dev@dpdk.org Content-Type: multipart/alternative; boundary="00000000000077988905fc7694ad" 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 --00000000000077988905fc7694ad Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, May 23, 2023 at 4:05=E2=80=AFAM Juraj Linke=C5=A1 wrote: > Hi Jeremy, first, a few general points: > > 1. Send patches to maintainers (Thomas, me, Honnappa, Lijuan and > anyone else involved with DTS or who might be interested) and add the > devlist to cc. > Thank you for the tip! I'm still new to sending patches and didn't think to do something like this but I will in the future. > 2. Run the linter script before submitting. > I did forget to run this, I will in the future. > 3. The use of the various nested objects breaks the current > abstractions. The basic idea is that the test suite developers should > ideally only use the sut/tg node objects and those objects should > delegate logic further to their nested objects. More below. > > I have many comments about the implementation, but I haven't run it > yet. I'm going to do that after this round of comments and I may have > more ideas. > > On Fri, May 12, 2023 at 9:28=E2=80=AFPM wrote: > > > > From: Jeremy Spewock > > > > Adds a new test suite for running smoke tests that verify general > > configuration aspects of the system under test. If any of these tests > > fail, the DTS execution terminates as part of a "fail-fast" model. > > > > Signed-off-by: Jeremy Spewock > > --- > > dts/conf.yaml | 9 ++ > > dts/framework/config/__init__.py | 21 +++++ > > dts/framework/config/conf_yaml_schema.json | 32 ++++++- > > dts/framework/dts.py | 19 +++- > > dts/framework/exception.py | 11 +++ > > dts/framework/remote_session/os_session.py | 6 +- > > .../remote_session/remote/__init__.py | 28 ++++++ > > dts/framework/test_result.py | 13 ++- > > dts/framework/test_suite.py | 24 ++++- > > dts/framework/testbed_model/__init__.py | 5 + > > .../interactive_apps/__init__.py | 6 ++ > > .../interactive_apps/interactive_command.py | 57 +++++++++++ > > .../interactive_apps/testpmd_driver.py | 24 +++++ > > dts/framework/testbed_model/node.py | 2 + > > dts/framework/testbed_model/sut_node.py | 6 ++ > > dts/tests/TestSuite_smoke_tests.py | 94 +++++++++++++++++++ > > 16 files changed, 348 insertions(+), 9 deletions(-) > > create mode 100644 > dts/framework/testbed_model/interactive_apps/__init__.py > > create mode 100644 > dts/framework/testbed_model/interactive_apps/interactive_command.py > > create mode 100644 > dts/framework/testbed_model/interactive_apps/testpmd_driver.py > > Let's not add any more levels. I don't like even the current hw > subdirectory (which I want to remove in the docs patch) and we don't > need it. I'd also like to move this functionality into remote_session, > as it's handling a type of remote session. > I think it makes sense to add a proper wrapper for it, I just didn't create a subclass for it off of remote_session because the idea behind it was only allowing users to interact with the session through the InteractiveScriptHandler/DPDK app handlers. If it were part of the remote_session class it would have to include operations for sending commands the way it is written now. I could do this but it seems like a bigger discussion about whether interactive sessions should create a new SSH session every time or instead just use one existing session and create channels off of it. > > > create mode 100644 dts/tests/TestSuite_smoke_tests.py > > > > diff --git a/dts/conf.yaml b/dts/conf.yaml > > index a9bd8a3e..042ef954 100644 > > --- a/dts/conf.yaml > > +++ b/dts/conf.yaml > > @@ -10,13 +10,22 @@ executions: > > compiler_wrapper: ccache > > perf: false > > func: true > > + nics: #physical devices to be used for testing > > + - addresses: > > + - "0000:11:00.0" > > + - "0000:11:00.1" > > + driver: "i40e" > > + vdevs: #names of virtual devices to be used for testing > > + - "crypto_openssl" > > I believe we specified the NICs under SUTs in the original DTS, just > as Owen did in his internal GitLab patch. If you can access it, have a > look at how he did it. > This brings an interesting question of where we want to specify which > NICs/vdevs to test. It could be on the SUT level, but also on the > execution or even the build target level. This should be informed by > testing needs. What makes the most sense? We could specify NIC details > per SUT/TG and then just reference the NICs on the execution/build > target level. > I put it on the execution level with the thought that you might have multiple NICs in your SUT but want to separate them into different executions. I guess in the case of smoke tests, you'd only need to know them per target because it was talked about previously that then is when we should run the smoke tests. I think however it would make sense to specify the NIC you are using for that execution rather than having to assume or specify elsewhere. > > > test_suites: > > + - smoke_tests > > - hello_world > > system_under_test: "SUT 1" > > nodes: > > - name: "SUT 1" > > hostname: sut1.change.me.localhost > > user: root > > + password: "" > > This was deliberately left out to discourage the use of passwords. > > > arch: x86_64 > > os: linux > > lcores: "" > > > > > diff --git a/dts/framework/dts.py b/dts/framework/dts.py > > index 05022845..0d03e158 100644 > > --- a/dts/framework/dts.py > > +++ b/dts/framework/dts.py > > @@ -5,6 +5,8 @@ > > > > import sys > > > > +from .exception import BlockingTestSuiteError > > + > > from .config import CONFIGURATION, BuildTargetConfiguration, > ExecutionConfiguration > > from .logger import DTSLOG, getLogger > > from .test_result import BuildTargetResult, DTSResult, ExecutionResult= , > Result > > @@ -49,6 +51,7 @@ def run_all() -> None: > > nodes[sut_node.name] =3D sut_node > > > > if sut_node: > > + #SMOKE TEST EXECUTION GOES HERE! > > _run_execution(sut_node, execution, result) > > > > except Exception as e: > > @@ -118,7 +121,7 @@ def _run_build_target( > > > > try: > > sut_node.set_up_build_target(build_target) > > - result.dpdk_version =3D sut_node.dpdk_version > > + # result.dpdk_version =3D sut_node.dpdk_version > > build_target_result.update_setup(Result.PASS) > > except Exception as e: > > dts_logger.exception("Build target setup failed.") > > @@ -146,6 +149,7 @@ def _run_suites( > > with possibly only a subset of test cases. > > If no subset is specified, run all test cases. > > """ > > + end_execution =3D False > > This only ends the build target stage, not the execution stage. We > should either find a better name for the variable or make sure that > the whole execution is blocked. I think we're aiming for the latter - > maybe we could just check whether the last exception in result is a > BlockingTestSuiteError (or better, just call a new method of result > that will check that), which we could do at multiple stages. > Good catch. When writing this and figuring out how to get it to work, I was thinking about smoke tests on a per build target basis because if it failed on one build target it could theoretically pass on another, but you'd likely want your entire execution to fail than have partial results. > > > for test_suite_config in execution.test_suites: > > try: > > full_suite_path =3D > f"tests.TestSuite_{test_suite_config.test_suite}" > > @@ -160,13 +164,24 @@ def _run_suites( > > > > else: > > for test_suite_class in test_suite_classes: > > + #HERE NEEDS CHANGING > > test_suite =3D test_suite_class( > > sut_node, > > test_suite_config.test_cases, > > execution.func, > > build_target_result, > > + sut_node._build_target_config, > > + result > > ) > > - test_suite.run() > > + try: > > + test_suite.run() > > + except BlockingTestSuiteError as e: > > + dts_logger.exception("An error occurred within a > blocking TestSuite, execution will now end.") > > + result.add_error(e) > > + end_execution =3D True > > + #if a blocking test failed and we need to bail out of suite > executions > > + if end_execution: > > + break > > > > > > def _exit_dts() -> None: > > diff --git a/dts/framework/exception.py b/dts/framework/exception.py > > index ca353d98..4e3f63d1 100644 > > --- a/dts/framework/exception.py > > +++ b/dts/framework/exception.py > > @@ -25,6 +25,7 @@ class ErrorSeverity(IntEnum): > > SSH_ERR =3D 4 > > DPDK_BUILD_ERR =3D 10 > > TESTCASE_VERIFY_ERR =3D 20 > > + BLOCKING_TESTSUITE_ERR =3D 25 > > > > > > class DTSError(Exception): > > @@ -144,3 +145,13 @@ def __init__(self, value: str): > > > > def __str__(self) -> str: > > return repr(self.value) > > + > > +class BlockingTestSuiteError(DTSError): > > + suite_name: str > > + severity: ClassVar[ErrorSeverity] =3D > ErrorSeverity.BLOCKING_TESTSUITE_ERR > > + > > + def __init__(self, suite_name:str) -> None: > > + self.suite_name =3D suite_name > > + > > + def __str__(self) -> str: > > + return f"Blocking suite {self.suite_name} failed." > > diff --git a/dts/framework/remote_session/os_session.py > b/dts/framework/remote_session/os_session.py > > index 4c48ae25..22776bc1 100644 > > --- a/dts/framework/remote_session/os_session.py > > +++ b/dts/framework/remote_session/os_session.py > > @@ -12,7 +12,9 @@ > > from framework.testbed_model import LogicalCore > > from framework.utils import EnvVarsDict, MesonArgs > > > > -from .remote import CommandResult, RemoteSession, create_remote_sessio= n > > +from .remote import CommandResult, RemoteSession, > create_remote_session, create_interactive_session > > + > > +from paramiko import SSHClient > > > > > > class OSSession(ABC): > > @@ -26,6 +28,7 @@ class OSSession(ABC): > > name: str > > _logger: DTSLOG > > remote_session: RemoteSession > > + _interactive_session: SSHClient > > > > def __init__( > > self, > > @@ -37,6 +40,7 @@ def __init__( > > self.name =3D name > > self._logger =3D logger > > self.remote_session =3D create_remote_session(node_config, nam= e, > logger) > > + self._interactive_session =3D > create_interactive_session(node_config, name, logger) > > This session should be stored in SutNode (it should be os-agnostic > (the apps should behave the same on all os's, right?) and SutNode > could use it without accessing another object), but created in > OSSession (this way we make sure any os-specific inputs (such as > paths) are passed properly). > I can move it into SutNode, the main reason I left it there is because I was essentially thinking of it the same way as the remote session that gets created once per node. However, I could just as easily have SutNode store it and call upon OSSession to create one inside its constructor. This would make creating classes for DPDK apps easier as well once those are subclasses of the InteractiveScriptHandler. > > > > def close(self, force: bool =3D False) -> None: > > """ > > diff --git a/dts/framework/remote_session/remote/__init__.py > b/dts/framework/remote_session/remote/__init__.py > > index 8a151221..abca8edc 100644 > > --- a/dts/framework/remote_session/remote/__init__.py > > +++ b/dts/framework/remote_session/remote/__init__.py > > @@ -9,8 +9,36 @@ > > from .remote_session import CommandResult, RemoteSession > > from .ssh_session import SSHSession > > > > +from paramiko import SSHClient, AutoAddPolicy > > +from framework.utils import GREEN > > > > def create_remote_session( > > node_config: NodeConfiguration, name: str, logger: DTSLOG > > ) -> RemoteSession: > > return SSHSession(node_config, name, logger) > > + > > +def create_interactive_session( > > + node_config: NodeConfiguration, name: str, logger: DTSLOG > > +) -> SSHClient: > > + """ > > + Creates a paramiko SSH session that is designed to be used for > interactive shells > > + > > + This session is meant to be used on an "as needed" basis and may > never be utilized > > + """ > > + client: SSHClient =3D SSHClient() > > + client.set_missing_host_key_policy(AutoAddPolicy) > > + ip: str =3D node_config.hostname > > + logger.info(GREEN(f"Connecting to host {ip}")) > > Using colors is a remnant from the original DTS. If we want to > (re-)introduce colors I'd do that in a separate patch in a uniform > manner. > I agree this is likely outside the scope of this patch and the color here really isn't necessary either so I'll remove it. > > > + #Preset to 22 because paramiko doesn't accept None > > + port: int =3D 22 > > This configuration handling should be moved to NodeConfiguration. > The logic should also be moved to InteractiveScriptHandler. We also > probably don't need a factory for this. > I agree that the configuration could be moved into NodeConfiguration, however the reason I use a factory and don't have this connect logic inside InteractiveScriptHandler is because that would mean there would have to be an SSH session per application. This is one potential solution, but what I am doing here instead is creating one SSH session for interactive scripts at the start alongside the usual remote session. This session essentially exists in the background for the duration of the execution and then each application for DPDK creates a channel off that session when they are created. Then, when you are done using the application, it closes the channel but the session itself stays open. > > > + if ":" in node_config.hostname: > > + ip, port =3D node_config.hostname.split(":") > > + port =3D int(port) > > + client.connect( > > + ip, > > + username=3Dnode_config.user, > > + port=3Dport, > > + password=3Dnode_config.password or "", > > + timeout=3D20 if port else 10 > > + ) > > + return client > > diff --git a/dts/framework/test_result.py b/dts/framework/test_result.p= y > > index 74391982..77202ae2 100644 > > --- a/dts/framework/test_result.py > > +++ b/dts/framework/test_result.py > > @@ -8,6 +8,7 @@ > > import os.path > > from collections.abc import MutableSequence > > from enum import Enum, auto > > +from typing import Dict > > > > from .config import ( > > OS, > > @@ -67,12 +68,13 @@ class Statistics(dict): > > Using a dict provides a convenient way to format the data. > > """ > > > > - def __init__(self, dpdk_version): > > + def __init__(self, output_info: Dict[str, str] | None): > > super(Statistics, self).__init__() > > for result in Result: > > self[result.name] =3D 0 > > self["PASS RATE"] =3D 0.0 > > - self["DPDK VERSION"] =3D dpdk_version > > + if output_info: > > + for info_key, info_val in output_info.items(): > self[info_key] =3D info_val > > > > def __iadd__(self, other: Result) -> "Statistics": > > """ > > @@ -258,6 +260,7 @@ class DTSResult(BaseResult): > > """ > > > > dpdk_version: str | None > > + output: dict | None > > _logger: DTSLOG > > _errors: list[Exception] > > _return_code: ErrorSeverity > > @@ -267,6 +270,7 @@ class DTSResult(BaseResult): > > def __init__(self, logger: DTSLOG): > > super(DTSResult, self).__init__() > > self.dpdk_version =3D None > > + self.output =3D None > > self._logger =3D logger > > self._errors =3D [] > > self._return_code =3D ErrorSeverity.NO_ERR > > @@ -296,7 +300,10 @@ def process(self) -> None: > > for error in self._errors: > > self._logger.debug(repr(error)) > > > > - self._stats_result =3D Statistics(self.dpdk_version) > > + self._stats_result =3D Statistics(self.output) > > + #add information gathered from the smoke tests to the statisti= cs > > + # for info_key, info_val in smoke_test_info.items(): > self._stats_result[info_key] =3D info_val > > + # print(self._stats_result) > > self.add_stats(self._stats_result) > > with open(self._stats_filename, "w+") as stats_file: > > stats_file.write(str(self._stats_result)) > > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py > > index 0705f38f..1518fb8a 100644 > > --- a/dts/framework/test_suite.py > > +++ b/dts/framework/test_suite.py > > @@ -10,11 +10,14 @@ > > import inspect > > import re > > from types import MethodType > > +from typing import Dict > > > > -from .exception import ConfigurationError, SSHTimeoutError, > TestCaseVerifyError > > +from .config import BuildTargetConfiguration > > + > > +from .exception import BlockingTestSuiteError, ConfigurationError, > SSHTimeoutError, TestCaseVerifyError > > from .logger import DTSLOG, getLogger > > from .settings import SETTINGS > > -from .test_result import BuildTargetResult, Result, TestCaseResult, > TestSuiteResult > > +from .test_result import BuildTargetResult, DTSResult, Result, > TestCaseResult, TestSuiteResult > > from .testbed_model import SutNode > > > > > > @@ -37,10 +40,12 @@ class TestSuite(object): > > """ > > > > sut_node: SutNode > > + is_blocking =3D False > > _logger: DTSLOG > > _test_cases_to_run: list[str] > > _func: bool > > _result: TestSuiteResult > > + _dts_result: DTSResult > > > > def __init__( > > self, > > @@ -48,6 +53,8 @@ def __init__( > > test_cases: list[str], > > func: bool, > > build_target_result: BuildTargetResult, > > + build_target_conf: BuildTargetConfiguration, > > + dts_result: DTSResult > > ): > > self.sut_node =3D sut_node > > self._logger =3D getLogger(self.__class__.__name__) > > @@ -55,6 +62,8 @@ def __init__( > > self._test_cases_to_run.extend(SETTINGS.test_cases) > > self._func =3D func > > self._result =3D > build_target_result.add_test_suite(self.__class__.__name__) > > + self.build_target_info =3D build_target_conf > > + self._dts_result =3D dts_result > > > > def set_up_suite(self) -> None: > > """ > > @@ -118,6 +127,9 @@ def run(self) -> None: > > f"the next test suite may be affected." > > ) > > self._result.update_setup(Result.ERROR, e) > > + if len(self._result.get_errors()) > 0 and self.is_blocking= : > > + raise BlockingTestSuiteError(test_suite_name) > > + > > > > def _execute_test_suite(self) -> None: > > """ > > @@ -137,6 +149,7 @@ def _execute_test_suite(self) -> None: > > f"Attempt number {attempt_nr} out of > {all_attempts}." > > ) > > self._run_test_case(test_case_method, > test_case_result) > > + > > > > def _get_functional_test_cases(self) -> list[MethodType]: > > """ > > @@ -232,6 +245,11 @@ def _execute_test_case( > > test_case_result.update(Result.SKIP) > > raise KeyboardInterrupt("Stop DTS") > > > > + def write_to_statistics_file(self, output: Dict[str, str]): > > + if self._dts_result.output !=3D None: > > + self._dts_result.output.update(output) > > + else: > > + self._dts_result.output =3D output > > > > def get_test_suites(testsuite_module_path: str) -> > list[type[TestSuite]]: > > def is_test_suite(object) -> bool: > > @@ -252,3 +270,5 @@ def is_test_suite(object) -> bool: > > test_suite_class > > for _, test_suite_class in inspect.getmembers(testcase_module, > is_test_suite) > > ] > > + > > + > > diff --git a/dts/framework/testbed_model/__init__.py > b/dts/framework/testbed_model/__init__.py > > index f54a9470..63f17cc3 100644 > > --- a/dts/framework/testbed_model/__init__.py > > +++ b/dts/framework/testbed_model/__init__.py > > @@ -20,3 +20,8 @@ > > ) > > from .node import Node > > from .sut_node import SutNode > > + > > +from .interactive_apps import ( > > + InteractiveScriptHandler, > > + TestpmdDriver > > +) > > diff --git a/dts/framework/testbed_model/interactive_apps/__init__.py > b/dts/framework/testbed_model/interactive_apps/__init__.py > > new file mode 100644 > > index 00000000..0382d7e0 > > --- /dev/null > > +++ b/dts/framework/testbed_model/interactive_apps/__init__.py > > @@ -0,0 +1,6 @@ > > +from .interactive_command import ( > > + InteractiveScriptHandler > > +) > > +from .testpmd_driver import ( > > + TestpmdDriver > > +) > > \ No newline at end of file > > diff --git > a/dts/framework/testbed_model/interactive_apps/interactive_command.py > b/dts/framework/testbed_model/interactive_apps/interactive_command.py > > new file mode 100644 > > index 00000000..7467911b > > --- /dev/null > > +++ b/dts/framework/testbed_model/interactive_apps/interactive_command.= py > > In general, the file name should match the class name. > > > @@ -0,0 +1,57 @@ > > +# import paramiko > > +from paramiko import SSHClient, Channel, channel > > +from framework.settings import SETTINGS > > + > > +class InteractiveScriptHandler: > > Once merged with the init functionality and moved to remote_session, > I'd rename it to InteractiveAppSession or something similar. > Good point, I'll rename the class and the file. > > > + > > + _ssh_client: SSHClient > > + _stdin: channel.ChannelStdinFile > > + _ssh_channel: Channel > > + > > + def __init__(self, ssh_client: SSHClient, timeout:float =3D > SETTINGS.timeout) -> None: > > + self._ssh_client =3D ssh_client > > + self._ssh_channel =3D self._ssh_client.invoke_shell() > > + self._stdin =3D self._ssh_channel.makefile_stdin("wb") > > + self._ssh_channel.settimeout(timeout) > > + > > + def send_command(self, command:str) -> None: > > + """ > > + Send command to channel without recording output. > > + > > + This method will not verify any input or output, it will > > + simply assume the command succeeded > > + """ > > + self._stdin.write(command + '\n') > > + self._stdin.flush() > > + > > + def send_command_get_output(self, command:str, expect:str) -> str: > > Let's rename expect to prompt. At least for me it was just confusing > in the original DTS. > It definitely can be confusing, I'll change it accordingly as you're right, prompt is more clear. > > > + """ > > + Send a command and get all output before the expected ending > string. > > + > > + **NOTE** > > + Lines that expect input are not included in the stdout buffer > so they cannot be > > + used for expect. For example, if you were prompted to log into > something > > + with a username and password, you cannot expect "username:" > because it wont > > + yet be in the stdout buffer. A work around for this could be > consuming an > > + extra newline character to force the current prompt into the > stdout buffer. > > + > > + *Return* > > + All output before expected string > > + """ > > + stdout =3D self._ssh_channel.makefile("r") > > + self._stdin.write(command + '\n') > > + self._stdin.flush() > > + out:str =3D "" > > + for line in stdout: > > + out +=3D str(line) > > + if expect in str(line): > > + break > > + stdout.close() #close the buffer to flush the output > > + return out > > + > > + def close(self): > > + self._stdin.close() > > + self._ssh_channel.close() > > + > > + def __del__(self): > > + self.close() > > diff --git > a/dts/framework/testbed_model/interactive_apps/testpmd_driver.py > b/dts/framework/testbed_model/interactive_apps/testpmd_driver.py > > new file mode 100644 > > index 00000000..1993eae6 > > --- /dev/null > > +++ b/dts/framework/testbed_model/interactive_apps/testpmd_driver.py > > @@ -0,0 +1,24 @@ > > +from framework.testbed_model.interactive_apps import > InteractiveScriptHandler > > + > > +from pathlib import PurePath > > + > > +class TestpmdDriver: > > This could also be called TestPmdSession. Not sure which is better. > This should extend InteractiveScriptHandler, which should contain > basic functionality common to all apps and then the drivers would > contain app-specific functionality. > Originally the reason I avoided doing this was because I viewed them as separate layers of the process essentially. I wrote it in a way where the InteractiveScriptHandler was almost like an interface for any CLI and then DPDK apps could interact with this interface, but after thinking about it more, this isn't necessary. There is no reason really that the applications themselves can't just use these methods directly and it also avoids the need to have a reference to the handler within the object. > > > + prompt:str =3D "testpmd>" > > + interactive_handler: InteractiveScriptHandler > > + > > + def __init__(self, handler: InteractiveScriptHandler, > dpdk_build_dir:PurePath, eal_flags:str =3D "", cmd_line_options:str =3D "= ") -> > None: > > + """ > > + Sets the handler to drive the SSH session and starts testpmd > > + """ > > + self.interactive_handler =3D handler > > + # self.interactive_handler.send_command("sudo su") > > + # self.interactive_handler.send_command("cd > /root/testpmd-testing/dpdk/build") > > + > self.interactive_handler.send_command_get_output(f"{dpdk_build_dir}/app/d= pdk-testpmd > {eal_flags} -- -i {cmd_line_options}\n", self.prompt) > > The paths need to be handled in os-agnostic manner in os_session and > then passed here. > > > + > > + def send_command(self, command:str, expect:str =3D prompt) -> str: > > + """ > > + Specific way of handling the command for testpmd > > + > > + An extra newline character is consumed in order to force the > current line into the stdout buffer > > + """ > > + return self.interactive_handler.send_command_get_output(comman= d > + "\n", expect) > > \ No newline at end of file > > diff --git a/dts/framework/testbed_model/node.py > b/dts/framework/testbed_model/node.py > > index d48fafe6..c5147e0e 100644 > > --- a/dts/framework/testbed_model/node.py > > +++ b/dts/framework/testbed_model/node.py > > @@ -40,6 +40,7 @@ class Node(object): > > lcores: list[LogicalCore] > > _logger: DTSLOG > > _other_sessions: list[OSSession] > > + _execution_config: ExecutionConfiguration > > > > def __init__(self, node_config: NodeConfiguration): > > self.config =3D node_config > > @@ -64,6 +65,7 @@ def set_up_execution(self, execution_config: > ExecutionConfiguration) -> None: > > """ > > self._setup_hugepages() > > self._set_up_execution(execution_config) > > + self._execution_config =3D execution_config > > > > def _set_up_execution(self, execution_config: > ExecutionConfiguration) -> None: > > """ > > diff --git a/dts/framework/testbed_model/sut_node.py > b/dts/framework/testbed_model/sut_node.py > > index 2b2b50d9..8c39a66d 100644 > > --- a/dts/framework/testbed_model/sut_node.py > > +++ b/dts/framework/testbed_model/sut_node.py > > @@ -14,6 +14,7 @@ > > > > from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice > > from .node import Node > > +from .interactive_apps import InteractiveScriptHandler > > > > > > class SutNode(Node): > > @@ -261,6 +262,11 @@ def run_dpdk_app( > > return self.main_session.send_command( > > f"{app_path} {eal_args}", timeout, verify=3DTrue > > ) > > + def create_interactive_session_handler(self) -> > InteractiveScriptHandler: > > + """ > > + Create a handler for interactive sessions > > + """ > > + return > InteractiveScriptHandler(self.main_session._interactive_session) > > > > > > class EalParameters(object): > > diff --git a/dts/tests/TestSuite_smoke_tests.py > b/dts/tests/TestSuite_smoke_tests.py > > new file mode 100644 > > index 00000000..bacf289d > > --- /dev/null > > +++ b/dts/tests/TestSuite_smoke_tests.py > > @@ -0,0 +1,94 @@ > > +from framework.test_suite import TestSuite > > +from framework.testbed_model.sut_node import SutNode > > + > > +from framework.testbed_model.interactive_apps import TestpmdDriver > > + > > +def get_compiler_version(compiler_name: str, sut_node: SutNode) -> str= : > > I don't see a reason why this is outside SmokeTest. > > + match compiler_name: > > + case "gcc": > > + return > sut_node.main_session.send_command(f"{compiler_name} --version", > 60).stdout.split("\n")[0] > > As I alluded to earlier, the call here should be > sut_node.get_compiler_version(). This is to hide implementation > details from test suite developers. > Then, SutNode.get_compiler_version should then call > self.main_session.get_compiler_version(). The reason here is this is > an os-agnostic call. > get_compiler_version() should then be defined in os_session and > implemented in os specific classes. > I originally placed the method outside of smoke tests just so it wouldn't be run with the rest of the suite but it does make sense to make this and the other comments os-agnostic. > > > + case "clang": > > + return > sut_node.main_session.send_command(f"{compiler_name} --version", > 60).stdout.split("\n")[0] > > + case "msvc": > > + return sut_node.main_session.send_command(f"cl", > 60).stdout > > + case "icc": > > + return > sut_node.main_session.send_command(f"{compiler_name} -V", 60).stdout > > + > > +class SmokeTests(TestSuite): > > + is_blocking =3D True > > + > > + def set_up_suite(self) -> None: > > + """ > > + Setup: > > + build all DPDK > > + """ > > + self.dpdk_build_dir_path =3D self.sut_node.remote_dpdk_build_d= ir > > + > > + > > + def test_unit_tests(self) -> None: > > + """ > > + Test: > > + run the fast-test unit-test suite through meson > > + """ > > + self.sut_node.main_session.send_command(f"meson test -C > {self.dpdk_build_dir_path} --suite fast-tests", 300) > > Same here, there already are methods that build dpdk. If anything > needs to be added, we should expand those methods. > I don't think this is building DPDK, this is just running fast tests and ensuring that you are in the correct directory. > > > + > > + def test_driver_tests(self) -> None: > > + """ > > + Test: > > + run the driver-test unit-test suite through meson > > + """ > > + list_of_vdevs =3D "" > > + for dev in self.sut_node._execution_config.vdevs: > > + list_of_vdevs +=3D f"{dev}," > > + print(list_of_vdevs) > > + if len(list_of_vdevs) > 0: > > + self.sut_node.main_session.send_command(f"meson test -C > {self.dpdk_build_dir_path} --suite driver-tests --test-args \"--vdev > {list_of_vdevs}\"", 300) > > + else: > > + self.sut_node.main_session.send_command(f"meson test -C > {self.dpdk_build_dir_path} --suite driver-tests", 300) > > + > > + def test_gather_info(self) -> None: > > + """ > > + Test: > > + gather information about the system and send output to > statistics.txt > > + """ > > + out =3D {} > > + > > + out['OS'] =3D self.sut_node.main_session.send_command("awk -F= =3D > '$1=3D=3D\"NAME\" {print $2}' /etc/os-release", 60).stdout > > + out["OS VERSION"] =3D > self.sut_node.main_session.send_command("awk -F=3D '$1=3D=3D\"VERSION\" {= print > $2}' /etc/os-release", 60, True).stdout > > + out["COMPILER VERSION"] =3D get_compiler_version( > self.build_target_info.compiler.name, self.sut_node) > > + out["DPDK VERSION"] =3D self.sut_node.dpdk_version > > + if self.build_target_info.os.name =3D=3D "linux": > > + out['KERNEL VERSION'] =3D > self.sut_node.main_session.send_command("uname -r", 60).stdout > > + elif self.build_target_info.os.name =3D=3D "windows": > > + out['KERNEL VERSION'] =3D > self.sut_node.main_session.send_command("uname -a", 60).stdout > > + self.write_to_statistics_file(out) > > + > > This is not really a test. If we want to add this, it should be stored > elsewhere (in their respective objects, probably in result objects). > Three of these (os, os and kernel versions) are node data, the > remaining two are build target data. > I'm not sure it's a good idea to put these into statistics, as the > statistics are aggregated over all executions and build targets. In > case of multiple SUTs (in different executions) and multiple build > targets we'd record misleading data. We could include data from all > build targets and SUTs though. > The reason we (and original DTS) are storing DPDK version in the > current manner is that that doesn't change across anything, we're > always testing the same tarball. > You're right that this isn't really testing anything, I had originally included it here because it was just part of the smoke test outline. It's definitely out of place though and I can move it out to their respective classes. I think it might make sense to organize/label data based on the SUT it comes from, I just thought statistics made the most sense because you could see the test statistics as well as what it was testing in one place. > > > + > > + > > + def test_start_testpmd(self) -> None: > > + """ > > + Creates and instance of the testpmd driver to run the testpmd > app > > + """ > > + driver: TestpmdDriver =3D > TestpmdDriver(self.sut_node.create_interactive_session_handler(), > self.dpdk_build_dir_path) > > The driver should be returned by a method of self.sut_node. > Does it make more sense to have methods for creating handlers for various DPDK apps in SutNode? I had assumed it would be cleaner to just make the classes as needed so that you don't have multiple methods doing something very similar in SutNode (basically just making a class with a different name that takes in similar information). I suppose that if you were trying to have developers only interact with the SutNode class this makes sense. This is also something that could make good use of having the SSHClient in the SutNode and I could see how it would be useful. > > > + > > + print(driver.send_command("show port summary all")) > > + > > + def test_device_bound_to_driver(self) -> None: > > + """ > > + Test: > > + ensure that all drivers listed in the config are bound to > the correct drivers > > + """ > > + for nic in self.sut_node._execution_config.nics: > > + for address in nic.addresses: > > + out =3D > self.sut_node.main_session.send_command(f"{self.dpdk_build_dir_path}/../u= sertools/dpdk-devbind.py > --status | grep {address}", 60) > > This should follow the same abstractions as I outlined above. > However, we don't need to use dpdk-devbind here. It's probably safe to > do so (the script is likely not going to be changed), but we could > That's a good point about dev-bind, it might be better here to just avoid the reliance on another script. > > > + self.verify( > > + len(out.stdout) !=3D 0, > > + f"Failed to find configured device ({address}) > using dpdk-devbind.py", > > + ) > > + for string in out.stdout.split(" "): > > + if 'drv=3D' in string: > > + self.verify( > > + string.split("=3D")[1] =3D=3D nic.driver.s= trip(), > > + f'Driver for device {address} does not > match driver listed in configuration (bound to {string.split("=3D")[1]})'= , > > + ) > > + > > \ No newline at end of file > > -- > > 2.40.1 > > > I'll work on these changes, but I'd like to hear what you think about what I had mentioned about moving the connect logic to the InteractiveScriptHandler in the comments above. I had originally written it to use one session throughout rather than opening and closing SSH connections for every application but I'd like to hear which you think would be easier/better if there is a difference. --00000000000077988905fc7694ad Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, May 23, 2023 at 4:05=E2=80=AF= AM Juraj Linke=C5=A1 <juraj.linkes@pantheon.tech> wrote:
Hi Jeremy, first, a few gene= ral points:

1. Send patches to maintainers (Thomas, me, Honnappa, Lijuan and
anyone else involved with DTS or who might be interested) and add the
devlist to cc.

Thank you for the tip! I= 'm still new to sending patches and didn't think to do something li= ke this but I will in the future.
=C2=A0
2. Run the linter script before submitting.

=
I did forget to run this, I will in the future.
=C2=A0
=
3. The use of the various nested objects breaks the current
abstractions. The basic idea is that the test suite developers should
ideally only use the sut/tg node objects and those objects should
delegate logic further to their nested objects. More below.

I have many comments about the implementation, but I haven't run it
yet. I'm going to do that after this round of comments and I may have more ideas.

On Fri, May 12, 2023 at 9:28=E2=80=AFPM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Adds a new test suite for running smoke tests that verify general
> configuration aspects of the system under test. If any of these tests<= br> > fail, the DTS execution terminates as part of a "fail-fast" = model.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>=C2=A0 dts/conf.yaml=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2= =A0 9 ++
>=C2=A0 dts/framework/config/__init__.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 | 21 +++++
>=C2=A0 dts/framework/config/conf_yaml_schema.json=C2=A0 =C2=A0 | 32 +++= +++-
>=C2=A0 dts/framework/dts.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 19 +++-
>=C2=A0 dts/framework/exception.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 11 +++
>=C2=A0 dts/framework/remote_session/os_session.py=C2=A0 =C2=A0 |=C2=A0 = 6 +-
>=C2=A0 .../remote_session/remote/__init__.py=C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0| 28 ++++++
>=C2=A0 dts/framework/test_result.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 13 ++-
>=C2=A0 dts/framework/test_suite.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 24 ++++-
>=C2=A0 dts/framework/testbed_model/__init__.py=C2=A0 =C2=A0 =C2=A0 =C2= =A0|=C2=A0 5 +
>=C2=A0 .../interactive_apps/__init__.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 |=C2=A0 6 ++
>=C2=A0 .../interactive_apps/interactive_command.py=C2=A0 =C2=A0| 57 +++= ++++++++
>=C2=A0 .../interactive_apps/testpmd_driver.py=C2=A0 =C2=A0 =C2=A0 =C2= =A0 | 24 +++++
>=C2=A0 dts/framework/testbed_model/node.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0|=C2=A0 2 +
>=C2=A0 dts/framework/testbed_model/sut_node.py=C2=A0 =C2=A0 =C2=A0 =C2= =A0|=C2=A0 6 ++
>=C2=A0 dts/tests/TestSuite_smoke_tests.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 | 94 +++++++++++++++++++
>=C2=A0 16 files changed, 348 insertions(+), 9 deletions(-)
>=C2=A0 create mode 100644 dts/framework/testbed_model/interactive_apps/= __init__.py
>=C2=A0 create mode 100644 dts/framework/testbed_model/interactive_apps/= interactive_command.py
>=C2=A0 create mode 100644 dts/framework/testbed_model/interactive_apps/= testpmd_driver.py

Let's not add any more levels. I don't like even the current hw
subdirectory (which I want to remove in the docs patch) and we don't need it. I'd also like to move this functionality into remote_session,<= br> as it's handling a type of remote session.

I think it makes sense to add a proper wrapper for it, I just didn&= #39;t create a subclass for it off of remote_session because the idea behin= d it was only allowing users to interact with the session through the Inter= activeScriptHandler/DPDK app handlers. If it were part of the remote_sessio= n class it would have to include operations for sending commands the way it= is written now. I could do this but it seems like a bigger discussion abou= t whether interactive sessions should create a new SSH session every time o= r instead just use one existing session and create channels off of it.
=C2=A0

>=C2=A0 create mode 100644 dts/tests/TestSuite_smoke_tests.py
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index a9bd8a3e..042ef954 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -10,13 +10,22 @@ executions:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 compiler_wrapper: ccache
>=C2=A0 =C2=A0 =C2=A0 perf: false
>=C2=A0 =C2=A0 =C2=A0 func: true
> +=C2=A0 =C2=A0 nics: #physical devices to be used for testing
> +=C2=A0 =C2=A0 =C2=A0 - addresses:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 - "0000:11:00.0"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 - "0000:11:00.1"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 driver: "i40e"
> +=C2=A0 =C2=A0 vdevs: #names of virtual devices to be used for testing=
> +=C2=A0 =C2=A0 =C2=A0 - "crypto_openssl"

I believe we specified the NICs under SUTs in the original DTS, just
as Owen did in his internal GitLab patch. If you can access it, have a
look at how he did it.
This brings an interesting question of where we want to specify which
NICs/vdevs to test. It could be on the SUT level, but also on the
execution or even the build target level. This should be informed by
testing needs. What makes the most sense? We could specify NIC details
per SUT/TG and then just reference the NICs on the execution/build
target level.

I put it on the execution= level with the thought that you might have multiple NICs in your SUT but w= ant to separate them into different executions. I guess in the case of smok= e tests, you'd only need to know them per target because it was talked = about previously that then is when we should run the smoke tests. I think h= owever it would make sense to specify the NIC you are using for that execut= ion rather than having to assume or specify elsewhere.
=C2=A0

>=C2=A0 =C2=A0 =C2=A0 test_suites:
> +=C2=A0 =C2=A0 =C2=A0 - smoke_tests
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 - hello_world
>=C2=A0 =C2=A0 =C2=A0 system_under_test: "SUT 1"
>=C2=A0 nodes:
>=C2=A0 =C2=A0 - name: "SUT 1"
>=C2=A0 =C2=A0 =C2=A0 hostname: sut1.change.me.localhost
>=C2=A0 =C2=A0 =C2=A0 user: root
> +=C2=A0 =C2=A0 password: ""

This was deliberately left out to discourage the use of passwords.

>=C2=A0 =C2=A0 =C2=A0 arch: x86_64
>=C2=A0 =C2=A0 =C2=A0 os: linux
>=C2=A0 =C2=A0 =C2=A0 lcores: ""

<snip>

> diff --git a/dts/framework/dts.py b/dts/framework/dts.py
> index 05022845..0d03e158 100644
> --- a/dts/framework/dts.py
> +++ b/dts/framework/dts.py
> @@ -5,6 +5,8 @@
>
>=C2=A0 import sys
>
> +from .exception import BlockingTestSuiteError
> +
>=C2=A0 from .config import CONFIGURATION, BuildTargetConfiguration, Exe= cutionConfiguration
>=C2=A0 from .logger import DTSLOG, getLogger
>=C2=A0 from .test_result import BuildTargetResult, DTSResult, Execution= Result, Result
> @@ -49,6 +51,7 @@ def run_all() -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 nodes[sut_node.name] =3D sut_node
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if sut_node:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 #SMOKE TEST E= XECUTION GOES HERE!
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 _run_exe= cution(sut_node, execution, result)
>
>=C2=A0 =C2=A0 =C2=A0 except Exception as e:
> @@ -118,7 +121,7 @@ def _run_build_target(
>
>=C2=A0 =C2=A0 =C2=A0 try:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sut_node.set_up_build_target(build_t= arget)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 result.dpdk_version =3D sut_node.dpdk_ver= sion
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # result.dpdk_version =3D sut_node.dpdk_v= ersion
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 build_target_result.update_setup(Res= ult.PASS)
>=C2=A0 =C2=A0 =C2=A0 except Exception as e:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dts_logger.exception("Build tar= get setup failed.")
> @@ -146,6 +149,7 @@ def _run_suites(
>=C2=A0 =C2=A0 =C2=A0 with possibly only a subset of test cases.
>=C2=A0 =C2=A0 =C2=A0 If no subset is specified, run all test cases.
>=C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 end_execution =3D False

This only ends the build target stage, not the execution stage. We
should either find a better name for the variable or make sure that
the whole execution is blocked. I think we're aiming for the latter - maybe we could just check whether the last exception in result is a
BlockingTestSuiteError (or better, just call a new method of result
that will check that), which we could do at multiple stages.

Good catch. When writing this and figuring out how to= get it to work, I was thinking about smoke tests on a per build target bas= is because if it failed on one build target it could theoretically pass on = another, but you'd likely want your entire execution to fail than have = partial results.
=C2=A0

>=C2=A0 =C2=A0 =C2=A0 for test_suite_config in execution.test_suites: >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 try:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 full_suite_path =3D f&= quot;tests.TestSuite_{test_suite_config.test_suite}"
> @@ -160,13 +164,24 @@ def _run_suites(
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 else:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for test_suite_class i= n test_suite_classes:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 #HERE NEEDS C= HANGING
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 test_sui= te =3D test_suite_class(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 sut_node,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 test_suite_config.test_cases,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 execution.func,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 build_target_result,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= sut_node._build_target_config,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= result
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 test_suite.ru= n()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 try:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= test_suite.run()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 except Blocki= ngTestSuiteError as e:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= dts_logger.exception("An error occurred within a blocking TestSuite, = execution will now end.")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= result.add_error(e)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= end_execution =3D True
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 #if a blocking test failed and we need to= bail out of suite executions
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if end_execution:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break
>
>
>=C2=A0 def _exit_dts() -> None:
> diff --git a/dts/framework/exception.py b/dts/framework/exception.py > index ca353d98..4e3f63d1 100644
> --- a/dts/framework/exception.py
> +++ b/dts/framework/exception.py
> @@ -25,6 +25,7 @@ class ErrorSeverity(IntEnum):
>=C2=A0 =C2=A0 =C2=A0 SSH_ERR =3D 4
>=C2=A0 =C2=A0 =C2=A0 DPDK_BUILD_ERR =3D 10
>=C2=A0 =C2=A0 =C2=A0 TESTCASE_VERIFY_ERR =3D 20
> +=C2=A0 =C2=A0 BLOCKING_TESTSUITE_ERR =3D 25
>
>
>=C2=A0 class DTSError(Exception):
> @@ -144,3 +145,13 @@ def __init__(self, value: str):
>
>=C2=A0 =C2=A0 =C2=A0 def __str__(self) -> str:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return repr(self.value)
> +
> +class BlockingTestSuiteError(DTSError):
> +=C2=A0 =C2=A0 suite_name: str
> +=C2=A0 =C2=A0 severity: ClassVar[ErrorSeverity]=C2=A0 =3D ErrorSeveri= ty.BLOCKING_TESTSUITE_ERR
> +
> +=C2=A0 =C2=A0 def __init__(self, suite_name:str) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.suite_name =3D suite_name
> +
> +=C2=A0 =C2=A0 def __str__(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return f"Blocking suite {self.suite_= name} failed."
> diff --git a/dts/framework/remote_session/os_session.py b/dts/framewor= k/remote_session/os_session.py
> index 4c48ae25..22776bc1 100644
> --- a/dts/framework/remote_session/os_session.py
> +++ b/dts/framework/remote_session/os_session.py
> @@ -12,7 +12,9 @@
>=C2=A0 from framework.testbed_model import LogicalCore
>=C2=A0 from framework.utils import EnvVarsDict, MesonArgs
>
> -from .remote import CommandResult, RemoteSession, create_remote_sessi= on
> +from .remote import CommandResult, RemoteSession, create_remote_sessi= on, create_interactive_session
> +
> +from paramiko import SSHClient
>
>
>=C2=A0 class OSSession(ABC):
> @@ -26,6 +28,7 @@ class OSSession(ABC):
>=C2=A0 =C2=A0 =C2=A0 name: str
>=C2=A0 =C2=A0 =C2=A0 _logger: DTSLOG
>=C2=A0 =C2=A0 =C2=A0 remote_session: RemoteSession
> +=C2=A0 =C2=A0 _interactive_session: SSHClient
>
>=C2=A0 =C2=A0 =C2=A0 def __init__(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self,
> @@ -37,6 +40,7 @@ def __init__(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.name =3D name
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger =3D logger
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.remote_session =3D create_remot= e_session(node_config, name, logger)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._interactive_session =3D create_inte= ractive_session(node_config, name, logger)

This session should be stored in SutNode (it should be os-agnostic
(the apps should behave the same on all os's, right?) and SutNode
could use it without accessing another object), but created in
OSSession (this way we make sure any os-specific inputs (such as
paths) are passed properly).

I can move= it into SutNode, the main reason I left it there is because I was essentia= lly thinking of it the same way as the remote session that gets created onc= e per node. However, I could just as easily have SutNode store it and call = upon OSSession to create one inside its constructor. This would make creati= ng classes for DPDK apps easier as well once those are subclasses of the In= teractiveScriptHandler.


>
>=C2=A0 =C2=A0 =C2=A0 def close(self, force: bool =3D False) -> None:=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> diff --git a/dts/framework/remote_session/remote/__init__.py b/dts/fra= mework/remote_session/remote/__init__.py
> index 8a151221..abca8edc 100644
> --- a/dts/framework/remote_session/remote/__init__.py
> +++ b/dts/framework/remote_session/remote/__init__.py
> @@ -9,8 +9,36 @@
>=C2=A0 from .remote_session import CommandResult, RemoteSession
>=C2=A0 from .ssh_session import SSHSession
>
> +from paramiko import SSHClient, AutoAddPolicy
> +from framework.utils import GREEN
>
>=C2=A0 def create_remote_session(
>=C2=A0 =C2=A0 =C2=A0 node_config: NodeConfiguration, name: str, logger:= DTSLOG
>=C2=A0 ) -> RemoteSession:
>=C2=A0 =C2=A0 =C2=A0 return SSHSession(node_config, name, logger)
> +
> +def create_interactive_session(
> +=C2=A0 =C2=A0 node_config: NodeConfiguration, name: str, logger: DTSL= OG
> +) -> SSHClient:
> +=C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 Creates a paramiko SSH session that is designed to be u= sed for interactive shells
> +
> +=C2=A0 =C2=A0 This session is meant to be used on an "as needed&= quot; basis and may never be utilized
> +=C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 client: SSHClient =3D SSHClient()
> +=C2=A0 =C2=A0 client.set_missing_host_key_policy(AutoAddPolicy)
> +=C2=A0 =C2=A0 ip: str =3D node_config.hostname
> +=C2=A0 =C2=A0 logger.info(GREEN(f"Connecting to host {ip}"))
Using colors is a remnant from the original DTS. If we want to
(re-)introduce colors I'd do that in a separate patch in a uniform
manner.

I agree this is likely outside = the scope of this patch and the color here really isn't necessary eithe= r so I'll remove it.
=C2=A0

> +=C2=A0 =C2=A0 #Preset to 22 because paramiko doesn't accept None<= br> > +=C2=A0 =C2=A0 port: int =3D 22

This configuration handling should be moved to NodeConfiguration.
The logic should also be moved to InteractiveScriptHandler. We also
probably don't need a factory for this.

=
I agree that the configuration could be moved into NodeConfiguration,= however the reason I use a factory and don't have this connect logic i= nside InteractiveScriptHandler is because that would mean there would have = to be an SSH session per application. This is one potential solution, but w= hat I am doing here instead is creating one SSH session for interactive scr= ipts at the start alongside the usual remote session. This session essentia= lly exists in the background for the duration of the execution and then eac= h application for DPDK creates a channel off that session when they are cre= ated. Then, when you are done using the application, it closes the channel = but the session itself stays open.=C2=A0
=C2=A0

> +=C2=A0 =C2=A0 if ":" in node_config.hostname:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ip, port =3D node_config.ho= stname.split(":")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 port =3D int(port)
> +=C2=A0 =C2=A0 client.connect(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ip,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 username=3Dnode_config.user,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 port=3Dport,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 password=3Dnode_config.password or "= ",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 timeout=3D20 if port else 10
> +=C2=A0 =C2=A0 )
> +=C2=A0 =C2=A0 return client
> diff --git a/dts/framework/test_result.py b/dts/framework/test_result.= py
> index 74391982..77202ae2 100644
> --- a/dts/framework/test_result.py
> +++ b/dts/framework/test_result.py
> @@ -8,6 +8,7 @@
>=C2=A0 import os.path
>=C2=A0 from collections.abc import MutableSequence
>=C2=A0 from enum import Enum, auto
> +from typing import Dict
>
>=C2=A0 from .config import (
>=C2=A0 =C2=A0 =C2=A0 OS,
> @@ -67,12 +68,13 @@ class Statistics(dict):
>=C2=A0 =C2=A0 =C2=A0 Using a dict provides a convenient way to format t= he data.
>=C2=A0 =C2=A0 =C2=A0 """
>
> -=C2=A0 =C2=A0 def __init__(self, dpdk_version):
> +=C2=A0 =C2=A0 def __init__(self, output_info: Dict[str, str] | None):=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 super(Statistics, self).__init__() >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for result in Result:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self[result.name] =3D 0 >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self["PASS RATE"] =3D 0.0<= br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 self["DPDK VERSION"] =3D dpdk_v= ersion
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if output_info:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for info_key, info_val in o= utput_info.items(): self[info_key] =3D info_val
>
>=C2=A0 =C2=A0 =C2=A0 def __iadd__(self, other: Result) -> "Stat= istics":
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> @@ -258,6 +260,7 @@ class DTSResult(BaseResult):
>=C2=A0 =C2=A0 =C2=A0 """
>
>=C2=A0 =C2=A0 =C2=A0 dpdk_version: str | None
> +=C2=A0 =C2=A0 output: dict | None
>=C2=A0 =C2=A0 =C2=A0 _logger: DTSLOG
>=C2=A0 =C2=A0 =C2=A0 _errors: list[Exception]
>=C2=A0 =C2=A0 =C2=A0 _return_code: ErrorSeverity
> @@ -267,6 +270,7 @@ class DTSResult(BaseResult):
>=C2=A0 =C2=A0 =C2=A0 def __init__(self, logger: DTSLOG):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 super(DTSResult, self).__init__() >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.dpdk_version =3D None
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.output =3D None
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger =3D logger
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._errors =3D []
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._return_code =3D ErrorSeverity.= NO_ERR
> @@ -296,7 +300,10 @@ def process(self) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for error in self._err= ors:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._lo= gger.debug(repr(error))
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stats_result =3D Statistics(self.dp= dk_version)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stats_result =3D Statistics(self.ou= tput)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 #add information gathered from the smoke = tests to the statistics
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # for info_key, info_val in smoke_test_in= fo.items(): self._stats_result[info_key] =3D info_val
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # print(self._stats_result)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.add_stats(self._stats_result) >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with open(self._stats_filename, &quo= t;w+") as stats_file:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 stats_file.write(str(s= elf._stats_result))
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py=
> index 0705f38f..1518fb8a 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -10,11 +10,14 @@
>=C2=A0 import inspect
>=C2=A0 import re
>=C2=A0 from types import MethodType
> +from typing import Dict
>
> -from .exception import ConfigurationError, SSHTimeoutError, TestCaseV= erifyError
> +from .config import BuildTargetConfiguration
> +
> +from .exception import BlockingTestSuiteError, ConfigurationError, SS= HTimeoutError, TestCaseVerifyError
>=C2=A0 from .logger import DTSLOG, getLogger
>=C2=A0 from .settings import SETTINGS
> -from .test_result import BuildTargetResult, Result, TestCaseResult, T= estSuiteResult
> +from .test_result import BuildTargetResult, DTSResult, Result, TestCa= seResult, TestSuiteResult
>=C2=A0 from .testbed_model import SutNode
>
>
> @@ -37,10 +40,12 @@ class TestSuite(object):
>=C2=A0 =C2=A0 =C2=A0 """
>
>=C2=A0 =C2=A0 =C2=A0 sut_node: SutNode
> +=C2=A0 =C2=A0 is_blocking =3D False
>=C2=A0 =C2=A0 =C2=A0 _logger: DTSLOG
>=C2=A0 =C2=A0 =C2=A0 _test_cases_to_run: list[str]
>=C2=A0 =C2=A0 =C2=A0 _func: bool
>=C2=A0 =C2=A0 =C2=A0 _result: TestSuiteResult
> +=C2=A0 =C2=A0 _dts_result: DTSResult
>
>=C2=A0 =C2=A0 =C2=A0 def __init__(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self,
> @@ -48,6 +53,8 @@ def __init__(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 test_cases: list[str],
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 func: bool,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 build_target_result: BuildTargetResu= lt,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 build_target_conf: BuildTargetConfigurati= on,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 dts_result: DTSResult
>=C2=A0 =C2=A0 =C2=A0 ):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.sut_node =3D sut_node
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger =3D getLogger(self.__cl= ass__.__name__)
> @@ -55,6 +62,8 @@ def __init__(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._test_cases_to_run.extend(SETTI= NGS.test_cases)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._func =3D func
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._result =3D build_target_result= .add_test_suite(self.__class__.__name__)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.build_target_info =3D build_target_c= onf
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._dts_result =3D dts_result
>
>=C2=A0 =C2=A0 =C2=A0 def set_up_suite(self) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> @@ -118,6 +127,9 @@ def run(self) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 f"the next test suite may be affected."
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._re= sult.update_setup(Result.ERROR, e)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if len(self._result.get_err= ors()) > 0 and self.is_blocking:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise Blockin= gTestSuiteError(test_suite_name)
> +
>
>=C2=A0 =C2=A0 =C2=A0 def _execute_test_suite(self) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> @@ -137,6 +149,7 @@ def _execute_test_suite(self) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 f"Attempt number {attempt_nr} out of {all_attempt= s}."
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 )
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 self._run_test_case(test_case_method, test_case_result)
> +
>
>=C2=A0 =C2=A0 =C2=A0 def _get_functional_test_cases(self) -> list[Me= thodType]:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> @@ -232,6 +245,11 @@ def _execute_test_case(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 test_case_result.updat= e(Result.SKIP)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise KeyboardInterrup= t("Stop DTS")
>
> +=C2=A0 =C2=A0 def write_to_statistics_file(self, output: Dict[str, st= r]):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if self._dts_result.output !=3D None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._dts_result.output.upd= ate(output)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 else:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._dts_result.output =3D= output
>
>=C2=A0 def get_test_suites(testsuite_module_path: str) -> list[type[= TestSuite]]:
>=C2=A0 =C2=A0 =C2=A0 def is_test_suite(object) -> bool:
> @@ -252,3 +270,5 @@ def is_test_suite(object) -> bool:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 test_suite_class
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for _, test_suite_class in inspect.g= etmembers(testcase_module, is_test_suite)
>=C2=A0 =C2=A0 =C2=A0 ]
> +
> +
> diff --git a/dts/framework/testbed_model/__init__.py b/dts/framework/t= estbed_model/__init__.py
> index f54a9470..63f17cc3 100644
> --- a/dts/framework/testbed_model/__init__.py
> +++ b/dts/framework/testbed_model/__init__.py
> @@ -20,3 +20,8 @@
>=C2=A0 )
>=C2=A0 from .node import Node
>=C2=A0 from .sut_node import SutNode
> +
> +from .interactive_apps import (
> +=C2=A0 =C2=A0 InteractiveScriptHandler,
> +=C2=A0 =C2=A0 TestpmdDriver
> +)
> diff --git a/dts/framework/testbed_model/interactive_apps/__init__.py = b/dts/framework/testbed_model/interactive_apps/__init__.py
> new file mode 100644
> index 00000000..0382d7e0
> --- /dev/null
> +++ b/dts/framework/testbed_model/interactive_apps/__init__.py
> @@ -0,0 +1,6 @@
> +from .interactive_command import (
> +=C2=A0 =C2=A0 InteractiveScriptHandler
> +)
> +from .testpmd_driver import (
> +=C2=A0 =C2=A0 TestpmdDriver
> +)
> \ No newline at end of file
> diff --git a/dts/framework/testbed_model/interactive_apps/interactive_= command.py b/dts/framework/testbed_model/interactive_apps/interactive_comma= nd.py
> new file mode 100644
> index 00000000..7467911b
> --- /dev/null
> +++ b/dts/framework/testbed_model/interactive_apps/interactive_command= .py

In general, the file name should match the class name.

> @@ -0,0 +1,57 @@
> +# import paramiko
> +from paramiko import SSHClient, Channel, channel
> +from framework.settings import SETTINGS
> +
> +class InteractiveScriptHandler:

Once merged with the init functionality and moved to remote_session,
I'd rename it to InteractiveAppSession or something similar.

Good point, I'll rename the class and the fil= e.
=C2=A0

> +
> +=C2=A0 =C2=A0 _ssh_client: SSHClient
> +=C2=A0 =C2=A0 _stdin: channel.ChannelStdinFile
> +=C2=A0 =C2=A0 _ssh_channel: Channel
> +
> +=C2=A0 =C2=A0 def __init__(self, ssh_client: SSHClient, timeout:float= =3D SETTINGS.timeout) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._ssh_client =3D ssh_client
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._ssh_channel =3D self._ssh_client.in= voke_shell()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stdin =3D self._ssh_channel.makefil= e_stdin("wb")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._ssh_channel.settimeout(timeout)
> +
> +=C2=A0 =C2=A0 def send_command(self, command:str) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Send command to channel without recording= output.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 This method will not verify any input or = output, it will
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 simply assume the command succeeded
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stdin.write(command + '\n')=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stdin.flush()
> +
> +=C2=A0 =C2=A0 def send_command_get_output(self, command:str, expect:s= tr) -> str:

Let's rename expect to prompt. At least for me it was just confusing in the original DTS.

It definitely can = be confusing, I'll change it accordingly as you're right, prompt is= more clear.
=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Send a command and get all output before = the expected ending string.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 **NOTE**
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Lines that expect input are not included = in the stdout buffer so they cannot be
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 used for expect. For example, if you were= prompted to log into something
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with a username and password, you cannot = expect "username:" because it wont
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 yet be in the stdout buffer. A work aroun= d for this could be consuming an
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 extra newline character to force the curr= ent prompt into the stdout buffer.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 *Return*
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 All output before expected = string
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 stdout =3D self._ssh_channel.makefile(&qu= ot;r")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stdin.write(command + '\n')=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stdin.flush()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 out:str =3D ""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for line in stdout:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out +=3D str(line)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if expect in str(line):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 stdout.close() #close the buffer to flush= the output
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return out
> +
> +=C2=A0 =C2=A0 def close(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._stdin.close()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._ssh_channel.close()
> +
> +=C2=A0 =C2=A0 def __del__(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.close()
> diff --git a/dts/framework/testbed_model/interactive_apps/testpmd_driv= er.py b/dts/framework/testbed_model/interactive_apps/testpmd_driver.py
> new file mode 100644
> index 00000000..1993eae6
> --- /dev/null
> +++ b/dts/framework/testbed_model/interactive_apps/testpmd_driver.py > @@ -0,0 +1,24 @@
> +from framework.testbed_model.interactive_apps import InteractiveScrip= tHandler
> +
> +from pathlib import PurePath
> +
> +class TestpmdDriver:

This could also be called TestPmdSession. Not sure which is better.
This should extend InteractiveScriptHandler, which should contain
basic functionality common to all apps and then the drivers would
contain app-specific functionality.

Ori= ginally the reason I avoided doing this was because I viewed them as separa= te layers of the process essentially. I wrote it in a way where the Interac= tiveScriptHandler was almost like an interface for any CLI and then DPDK ap= ps could interact with this interface, but after thinking about it more, th= is isn't necessary. There is no reason really that the applications the= mselves can't just use these methods directly and it also avoids the ne= ed to have a reference to the handler within the object.
=C2=A0

> +=C2=A0 =C2=A0 prompt:str =3D "testpmd>"
> +=C2=A0 =C2=A0 interactive_handler: InteractiveScriptHandler
> +
> +=C2=A0 =C2=A0 def __init__(self, handler: InteractiveScriptHandler, d= pdk_build_dir:PurePath, eal_flags:str =3D "", cmd_line_options:st= r =3D "") -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Sets the handler to drive the SSH session= and starts testpmd
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.interactive_handler =3D handler
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # self.interactive_handler.send_command(&= quot;sudo su")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # self.interactive_handler.send_command(&= quot;cd /root/testpmd-testing/dpdk/build")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.interactive_handler.send_command_get= _output(f"{dpdk_build_dir}/app/dpdk-testpmd {eal_flags} -- -i {cmd_lin= e_options}\n", self.prompt)

The paths need to be handled in os-agnostic manner in os_session and
then passed here.

> +
> +=C2=A0 =C2=A0 def send_command(self, command:str, expect:str =3D prom= pt) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Specific way of handling the command for = testpmd
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 An extra newline character is consumed in= order to force the current line into the stdout buffer
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.interactive_handler.send_comm= and_get_output(command + "\n", expect)
> \ No newline at end of file
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testb= ed_model/node.py
> index d48fafe6..c5147e0e 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -40,6 +40,7 @@ class Node(object):
>=C2=A0 =C2=A0 =C2=A0 lcores: list[LogicalCore]
>=C2=A0 =C2=A0 =C2=A0 _logger: DTSLOG
>=C2=A0 =C2=A0 =C2=A0 _other_sessions: list[OSSession]
> +=C2=A0 =C2=A0 _execution_config: ExecutionConfiguration
>
>=C2=A0 =C2=A0 =C2=A0 def __init__(self, node_config: NodeConfiguration)= :
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.config =3D node_config
> @@ -64,6 +65,7 @@ def set_up_execution(self, execution_config: Executi= onConfiguration) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._setup_hugepages()
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._set_up_execution(execution_con= fig)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._execution_config =3D execution_conf= ig
>
>=C2=A0 =C2=A0 =C2=A0 def _set_up_execution(self, execution_config: Exec= utionConfiguration) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/t= estbed_model/sut_node.py
> index 2b2b50d9..8c39a66d 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -14,6 +14,7 @@
>
>=C2=A0 from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice=
>=C2=A0 from .node import Node
> +from .interactive_apps import InteractiveScriptHandler
>
>
>=C2=A0 class SutNode(Node):
> @@ -261,6 +262,11 @@ def run_dpdk_app(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.main_session.send_comman= d(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 f"{app_path} {eal= _args}", timeout, verify=3DTrue
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +=C2=A0 =C2=A0 def create_interactive_session_handler(self) -> Inte= ractiveScriptHandler:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Create a handler for interactive sessions=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return InteractiveScriptHandler(self.main= _session._interactive_session)
>
>
>=C2=A0 class EalParameters(object):
> diff --git a/dts/tests/TestSuite_smoke_tests.py b/dts/tests/TestSuite_= smoke_tests.py
> new file mode 100644
> index 00000000..bacf289d
> --- /dev/null
> +++ b/dts/tests/TestSuite_smoke_tests.py
> @@ -0,0 +1,94 @@
> +from framework.test_suite import TestSuite
> +from framework.testbed_model.sut_node import SutNode
> +
> +from framework.testbed_model.interactive_apps import TestpmdDriver > +
> +def get_compiler_version(compiler_name: str, sut_node: SutNode) ->= str:

I don't see a reason why this is outside SmokeTest.=C2=A0
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">
> +=C2=A0 =C2=A0 match compiler_name:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 case "gcc":
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return sut_no= de.main_session.send_command(f"{compiler_name} --version", 60).st= dout.split("\n")[0]

As I alluded to earlier, the call here should be
sut_node.get_compiler_version(). This is to hide implementation
details from test suite developers.
Then, SutNode.get_compiler_version should then call
self.main_session.get_compiler_version(). The reason here is this is
an os-agnostic call.
get_compiler_version() should then be defined in os_session and
implemented in os specific classes.

I o= riginally placed the method outside of smoke tests just so it wouldn't = be run with the rest of the suite but it does make sense to make this and t= he other comments os-agnostic.
=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 case "clang":
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return sut_no= de.main_session.send_command(f"{compiler_name} --version", 60).st= dout.split("\n")[0]
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 case "msvc":
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return sut_no= de.main_session.send_command(f"cl", 60).stdout
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 case "icc":
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return sut_no= de.main_session.send_command(f"{compiler_name} -V", 60).stdout > +
> +class SmokeTests(TestSuite):
> +=C2=A0 =C2=A0 is_blocking =3D True
> +
> +=C2=A0 =C2=A0 def set_up_suite(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Setup:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 build all DPDK
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.dpdk_build_dir_path =3D self.sut_nod= e.remote_dpdk_build_dir
> +
> +
> +=C2=A0 =C2=A0 def test_unit_tests(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Test:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 run the fast-test unit-test= suite through meson
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.sut_node.main_session.send_command(f= "meson test -C {self.dpdk_build_dir_path} --suite fast-tests", 30= 0)

Same here, there already are methods that build dpdk. If anything
needs to be added, we should expand those methods.
I don't think this is building DPDK, this is just running f= ast tests and ensuring that you are in the correct directory.
=C2= =A0

> +
> +=C2=A0 =C2=A0 def test_driver_tests(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Test:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 run the driver-test unit-te= st suite through meson
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 list_of_vdevs =3D ""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for dev in self.sut_node._execution_confi= g.vdevs:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 list_of_vdevs +=3D f"{= dev},"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 print(list_of_vdevs)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if len(list_of_vdevs) > 0:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.sut_node.main_session.= send_command(f"meson test -C {self.dpdk_build_dir_path} --suite driver= -tests --test-args \"--vdev {list_of_vdevs}\"", 300)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 else:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.sut_node.main_session.= send_command(f"meson test -C {self.dpdk_build_dir_path} --suite driver= -tests", 300)
> +
> +=C2=A0 =C2=A0 def test_gather_info(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Test:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gather information about th= e system and send output to statistics.txt
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 out =3D {}
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 out['OS'] =3D self.sut_node.main_= session.send_command("awk -F=3D '$1=3D=3D\"NAME\" {print= $2}' /etc/os-release", 60).stdout
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 out["OS VERSION"] =3D self.sut_= node.main_session.send_command("awk -F=3D '$1=3D=3D\"VERSION\= " {print $2}' /etc/os-release", 60, True).stdout
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 out["COMPILER VERSION"] =3D get= _compiler_version(self.build_target_info.compiler.name, self.sut_node)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 out["DPDK VERSION"] =3D self.su= t_node.dpdk_version
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if self.build_target_info.os.= name =3D=3D "linux":
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out['KERNEL VERSION'= ;] =3D self.sut_node.main_session.send_command("uname -r", 60).st= dout
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 elif self.build_target_info.o= s.name =3D=3D "windows":
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out['KERNEL VERSION'= ;] =3D self.sut_node.main_session.send_command("uname -a", 60).st= dout
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.write_to_statistics_file(out)
> +

This is not really a test. If we want to add this, it should be stored
elsewhere (in their respective objects, probably in result objects).
Three of these (os, os and kernel versions) are node data, the
remaining two are build target data.
I'm not sure it's a good idea to put these into statistics, as the<= br> statistics are aggregated over all executions and build targets. In
case of multiple SUTs (in different executions) and multiple build
targets we'd record misleading data. We could include data from all
build targets and SUTs though.
The reason we (and original DTS) are storing DPDK version in the
current manner is that that doesn't change across anything, we're always testing the same tarball.

You= 9;re right that this isn't really testing anything, I had originally in= cluded it here because it was just part of the smoke test outline. It's= definitely out of place though and I can move it out to their respective c= lasses. I think it might make sense to organize/label data based on the SUT= it comes from, I just thought statistics made the most sense because you c= ould see the test statistics as well as what it was testing in one place.
=C2=A0

> +
> +
> +=C2=A0 =C2=A0 def test_start_testpmd(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Creates and instance of the testpmd drive= r to run the testpmd app
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 driver: TestpmdDriver =3D TestpmdDriver(s= elf.sut_node.create_interactive_session_handler(), self.dpdk_build_dir_path= )

The driver should be returned by a method of self.sut_node.

Does it make more sense to have methods for creating h= andlers for various DPDK apps in SutNode? I had assumed it would be cleaner= to just make the classes as needed so that you don't have multiple met= hods doing something very similar in SutNode (basically just making a class= with a different name that takes in similar information). I suppose that i= f you were trying to have developers only interact with the SutNode class t= his makes sense.

This is also something that could= make good use of having the SSHClient in the SutNode and I could see how i= t would be useful.
=C2=A0

> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 print(driver.send_command("show port= summary all"))
> +
> +=C2=A0 =C2=A0 def test_device_bound_to_driver(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Test:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ensure that all drivers lis= ted in the config are bound to the correct drivers
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for nic in self.sut_node._execution_confi= g.nics:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for address in nic.addresse= s:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out =3D self.= sut_node.main_session.send_command(f"{self.dpdk_build_dir_path}/../use= rtools/dpdk-devbind.py --status | grep {address}", 60)

This should follow the same abstractions as I outlined above.
However, we don't need to use dpdk-devbind here. It's probably safe= to
do so (the script is likely not going to be changed), but we could

That's a good point about dev-bind, it migh= t be better here to just avoid the reliance on another script.
= =C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.verify(<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= len(out.stdout) !=3D 0,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= f"Failed to find configured device ({address}) using dpdk-devbind.py&= quot;,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for string in= out.stdout.split(" "):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= if 'drv=3D' in string:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 self.verify(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 string.split("=3D")[1] =3D=3D nic.dr= iver.strip(),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 f'Driver for device {address} does not mat= ch driver listed in configuration (bound to {string.split("=3D")[= 1]})',
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 )
> +
> \ No newline at end of file
> --
> 2.40.1
>

I'll work on these changes, bu= t I'd like to hear what you think about what I had mentioned about movi= ng the connect logic to the InteractiveScriptHandler in the comments above.= I had originally written it to use one session throughout rather than open= ing and closing SSH connections for every application but I'd like to h= ear which you think would be easier/better if there is a difference.
<= /div>
--00000000000077988905fc7694ad--