Re: [RFC v2 1/2] dts: add smoke tests

[Jeremy Spewock <jspewock@iol.unh.edu>]

[-- Attachment #1: Type: text/plain, Size: 50956 bytes --]

On Fri, May 26, 2023 at 3:36 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> On Thu, May 25, 2023 at 8:03 PM Jeremy Spewock <jspewock@iol.unh.edu>
> wrote:
> >
> > Hey Juraj,
> >
> > On Thu, May 25, 2023 at 4:33 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
> wrote:
> >>
> >> One more point that doesn't fit elsewhere:
> >>
> >> On Wed, May 24, 2023 at 10:45 PM Jeremy Spewock <jspewock@iol.unh.edu>
> wrote:
> >> >
> >> >
> >> >
> >> > On Tue, May 23, 2023 at 4:05 AM Juraj Linkeš
> <juraj.linkes@pantheon.tech> 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 PM <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
> >> >> > fail, the DTS execution terminates as part of a "fail-fast" model.
> >> >> >
> >> >> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> >> >> > ---
> >> >> >  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.
> >> >
> >>
> >> I wasn't clear, I meant to move the python modules into the
> >> remote_session folder. The subclassing would be there only to have a
> >> common API across these remote sessions (as the
> >> InteractiveScriptHandler is a kind of a remote session). If at all
> >> possible, this should be our aim.
> >
> >
> > I see what you mean now, moving them there shouldn't be a problem.
> >
> >>
> >>
> >> >>
> >> >>
> >> >> >  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.
> >>
> >> This is a good point. Is this something you're interested in in the
> >> lab? We should ask Lijuan what she thinks of this. In general, I like
> >> this, so we should at least think of how to add NIC config
> >> implementation so that we could add this later if we're not adding it
> >> now.
> >>
> >> > 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.
> >> >
> >>
> >> Nothing is set in stone, we don't have to run them per build target.
> >> We could have two smoke test suites, one per execution and one per
> >> build target. I actually like that a lot and we should explore that.
> >> If it's not much extra work, we could just do the split like that.
> >>
> >
> > I also like the sound of that. In theory it doesn't seem that hard, if
> we made two different suites that tested different things all that needs to
> change is when they are run. This would probably take some more planning as
> to what we want to run at which points during the run and it might be
> easier to get these tests in a better spot first. I do like the idea
> though, and agree that it's worth exploring.
> >
> >> >>
> >> >>
> >> >> >      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: ""
> >> >>
> >> >> <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 @@
> >> >> >
> >> >> >  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] = 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 = sut_node.dpdk_version
> >> >> > +        # result.dpdk_version = 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 = 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.
> >> >
> >>
> >> Well, these will be well defined partial results (and definitely
> >> valuable, imagine one build target passing and one failing, we could
> >> easily compare the two to sometimes quickly identify the failure), so
> >> if there's a possibility that the test would pass on another build
> >> target, we should try to run the build target.
> >>
> >> In this case, let's just rename the variable to end_build_target or
> >> something similar.
> >>
> >
> > Alright, will do.
> >
> >>
> >> >>
> >> >>
> >> >> >      for test_suite_config in execution.test_suites:
> >> >> >          try:
> >> >> >              full_suite_path =
> 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 = 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 = 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 = 4
> >> >> >      DPDK_BUILD_ERR = 10
> >> >> >      TESTCASE_VERIFY_ERR = 20
> >> >> > +    BLOCKING_TESTSUITE_ERR = 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]  =
> ErrorSeverity.BLOCKING_TESTSUITE_ERR
> >> >> > +
> >> >> > +    def __init__(self, suite_name:str) -> None:
> >> >> > +        self.suite_name = 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_session
> >> >> > +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 = name
> >> >> >          self._logger = logger
> >> >> >          self.remote_session = create_remote_session(node_config,
> name, logger)
> >> >> > +        self._interactive_session =
> 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.
> >> >
> >>
> >> I was thinking of this interactive session more like
> >> Node._other_sessions, created only when needed. Now that I think about
> >> it, it's always going to be needed if we use it in smoke tests, so we
> >> might as well create it in OSSession (or its subclasses if needed).
> >>
> >> Thinking further, we want to use only the app driver object in test
> >> suites. So the SutNode object should have a method that returns the
> >> object and the rest of the implementation needs to be delegated to the
> >> rest of the objects in the hierarchy to ensure proper remote OS
> >> handling.
> >
> >
> > I like the sound of this. If we use a method and something like an enum
> inside of SutNode as well to create these objects then we don't even really
> need a method to return this object because it would never be interacted
> with directly.
> >
> >>
> >>
> >> >>
> >> >> >
> >> >> >      def close(self, force: bool = 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 = SSHClient()
> >> >> > +    client.set_missing_host_key_policy(AutoAddPolicy)
> >> >> > +    ip: str = 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 = 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.
> >> >
> >>
> >> Ok, I see. What do you think about this:
> >> Let's then move the session creation into a subclass of RemoteSession.
> >> The session would be stored as a class variable and with that we could
> >> subclass further:
> >> RemoteSession -> new subclass -> InteractiveScriptHandler -> DpdkApp
> >>
> >> Everything could be handled from within the DdpdkApp objects, but we'd
> >> need to be careful about cleanup (maybe we'd only need to check that
> >> there are no open channels when closing the session).
> >>
> >> Or alternatively, the new subclass could just have a method that
> >> returns DpdkApps. We'd have two different class relations:
> >> RemoteSession -> new subclass and InteractiveScriptHandler -> DpdkApp.
> >>
> >> I'd put all of the classes into the remote_session directory.
> >>
> >
> > I like the idea of making a subclass of the remote session and then a
> method in the subclass that generates DpdkApps. This way you have a proper
> class that maintains the interactive session and InteractiveScriptHandlers
> as well as any other DpdkApp can be created and destroyed as needed. The
> only reason I wouldn't make InteractiveScriptHandler a subclass of
> RemoteSession is because then it would make the InteractiveScriptHandler
> something that stored the main SSH session rather than something that just
> uses it to make a channel.
> >
> > Originally, I avoided this because I didn't want to implement the
> send_command methods from inside RemoteSession, but a solution to this
> could just be creating an InteractiveScriptHandler, sending the command
> blindly, and not returning the output. Or maybe there would be some kind of
> default prompt I could expect to still collect the output, but I'm not sure
> there would be something that is os-agnostic for this. I'm not sure the
> best way to show that I don't want people to use the method on the new
> subclass that I'm going to create though. I guess one way to handle this
> would be just not creating a method that returns this subclass in SutNode.
> SutNode could just have a method that returns DPDK apps which should
> discourage touching this session directly.
> >
>
> Ah right, the fact that the workflow with interactive sessions is a
> bit different doesn't make subclassing RemoteSession ideal - the
> abstract methods _send_command and copy_file don't make much sense in
> the new class.
>
> With that in mind, I think we have two options:
> 1. Don't use one session with multiple channels. This would make it
> more in line with RemoteSession, but the copy_file method still
> doesn't make sense, so I prefer 2.
> 2. Don't subclass RemoteSession as it's a bit too different from what
> we need. Just create a new class (named InteractiveRemoteSession
> perpahs) that would basically be a factory for drivers (with two parts
> - ssh session init and then the creation of driver/app objects). The
> InteractiveScriptHandler (I'm thinking of renaming it to
> InteractiveShell now) -> DpdkApp would be the other part. I'd put
> InteractiveRemoteSession and InteractiveScriptHandler/InteractiveShell
> into the same file (those two basically define the API on the user and
> developer side).
>

I think I also prefer 2, I can work on putting that together. These
comments definitely gave me a lot of good insight into where to go from
here though, thank you!


>
> >>
> >> >>
> >> >>
> >> >> > +    if ":" in node_config.hostname:
> >> >> > +            ip, port = node_config.hostname.split(":")
> >> >> > +            port = int(port)
> >> >> > +    client.connect(
> >> >> > +        ip,
> >> >> > +        username=node_config.user,
> >> >> > +        port=port,
> >> >> > +        password=node_config.password or "",
> >> >> > +        timeout=20 if port else 10
> >> >> > +    )
> >> >> > +    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 @@
> >> >> >  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] = 0
> >> >> >          self["PASS RATE"] = 0.0
> >> >> > -        self["DPDK VERSION"] = dpdk_version
> >> >> > +        if output_info:
> >> >> > +            for info_key, info_val in output_info.items():
> self[info_key] = 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 = None
> >> >> > +        self.output = None
> >> >> >          self._logger = logger
> >> >> >          self._errors = []
> >> >> >          self._return_code = ErrorSeverity.NO_ERR
> >> >> > @@ -296,7 +300,10 @@ def process(self) -> None:
> >> >> >              for error in self._errors:
> >> >> >                  self._logger.debug(repr(error))
> >> >> >
> >> >> > -        self._stats_result = Statistics(self.dpdk_version)
> >> >> > +        self._stats_result = Statistics(self.output)
> >> >> > +        #add information gathered from the smoke tests to the
> statistics
> >> >> > +        # for info_key, info_val in smoke_test_info.items():
> self._stats_result[info_key] = 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 = 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 = sut_node
> >> >> >          self._logger = getLogger(self.__class__.__name__)
> >> >> > @@ -55,6 +62,8 @@ def __init__(
> >> >> >          self._test_cases_to_run.extend(SETTINGS.test_cases)
> >> >> >          self._func = func
> >> >> >          self._result =
> build_target_result.add_test_suite(self.__class__.__name__)
> >> >> > +        self.build_target_info = build_target_conf
> >> >> > +        self._dts_result = 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 != None:
> >> >> > +            self._dts_result.output.update(output)
> >> >> > +        else:
> >> >> > +            self._dts_result.output = 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 =
> SETTINGS.timeout) -> None:
> >> >> > +        self._ssh_client = ssh_client
> >> >> > +        self._ssh_channel = self._ssh_client.invoke_shell()
> >> >> > +        self._stdin = 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 = self._ssh_channel.makefile("r")
> >> >> > +        self._stdin.write(command + '\n')
> >> >> > +        self._stdin.flush()
> >> >> > +        out:str = ""
> >> >> > +        for line in stdout:
> >> >> > +            out += 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 = "testpmd>"
> >> >> > +    interactive_handler: InteractiveScriptHandler
> >> >> > +
> >> >> > +    def __init__(self, handler: InteractiveScriptHandler,
> dpdk_build_dir:PurePath, eal_flags:str = "", cmd_line_options:str = "") ->
> None:
> >> >> > +        """
> >> >> > +        Sets the handler to drive the SSH session and starts
> testpmd
> >> >> > +        """
> >> >> > +        self.interactive_handler = 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/dpdk-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 = 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(command + "\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 = 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 = 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=True
> >> >> >          )
> >> >> > +    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 = True
> >> >> > +
> >> >> > +    def set_up_suite(self) -> None:
> >> >> > +        """
> >> >> > +        Setup:
> >> >> > +            build all DPDK
> >> >> > +        """
> >> >> > +        self.dpdk_build_dir_path =
> self.sut_node.remote_dpdk_build_dir
> >> >> > +
> >> >> > +
> >> >> > +    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.
> >> >
> >>
> >> Ah, right. These would be probably used only in smoke tests, so it's
> >> probably fine to leave them here. Maybe we could add a method to the
> >> suite? I'm thinking it would make the code a bit more readable.
> >>
> >
> > I could look into this.
> >
> >>
> >> >>
> >> >>
> >> >> > +
> >> >> > +    def test_driver_tests(self) -> None:
> >> >> > +        """
> >> >> > +        Test:
> >> >> > +            run the driver-test unit-test suite through meson
> >> >> > +        """
> >> >> > +        list_of_vdevs = ""
> >> >> > +        for dev in self.sut_node._execution_config.vdevs:
> >> >> > +            list_of_vdevs += 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 = {}
> >> >> > +
> >> >> > +        out['OS'] = self.sut_node.main_session.send_command("awk
> -F= '$1==\"NAME\" {print $2}' /etc/os-release", 60).stdout
> >> >> > +        out["OS VERSION"] =
> self.sut_node.main_session.send_command("awk -F= '$1==\"VERSION\" {print
> $2}' /etc/os-release", 60, True).stdout
> >> >> > +        out["COMPILER VERSION"] = get_compiler_version(
> self.build_target_info.compiler.name, self.sut_node)
> >> >> > +        out["DPDK VERSION"] = self.sut_node.dpdk_version
> >> >> > +        if self.build_target_info.os.name == "linux":
> >> >> > +            out['KERNEL VERSION'] =
> self.sut_node.main_session.send_command("uname -r", 60).stdout
> >> >> > +        elif self.build_target_info.os.name == "windows":
> >> >> > +            out['KERNEL VERSION'] =
> 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.
> >> >
> >>
> >> We could put anything we want into statistics (it's not a good idea
> >> now, but if we change the format of statistics it'd be fine, but I
> >> think changing the format is out of the scope of this patch) and this
> >> would make sense there, but the first step is properly storing the
> >> data. Moving them to statistics would be trivial then.
> >>
> >
> > Right, that makes sense.
> >
> >>
> >> >>
> >> >>
> >> >> > +
> >> >> > +
> >> >> > +    def test_start_testpmd(self) -> None:
> >> >> > +        """
> >> >> > +        Creates and instance of the testpmd driver to run the
> testpmd app
> >> >> > +        """
> >> >> > +        driver: TestpmdDriver =
> 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.
> >> >
> >>
> >> That's what I meant. The method would return the appropriate app based
> >> on input (which could be an enum) - I think that's preferable to
> >> having a method for each app. We should think about non-interactive
> >> apps here as well (such as the helloworld app that's already in use).
> >>
> >
> > I like this idea for how to handle the creation of the DpdkApps.
> >
> >>
> >> > 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 =
> self.sut_node.main_session.send_command(f"{self.dpdk_build_dir_path}/../usertools/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) != 0,
> >> >> > +                    f"Failed to find configured device
> ({address}) using dpdk-devbind.py",
> >> >> > +                )
> >> >> > +                for string in out.stdout.split(" "):
> >> >> > +                    if 'drv=' in string:
> >> >> > +                        self.verify(
> >> >> > +                            string.split("=")[1] ==
> nic.driver.strip(),
> >> >> > +                            f'Driver for device {address} does
> not match driver listed in configuration (bound to {string.split("=")[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.
> >>
> >> Yes, let's do one session with each app having its own channel.
>

[-- Attachment #2: Type: text/html, Size: 69652 bytes --]