DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Luca Vizzarro <luca.vizzarro@arm.com>
Cc: dev@dpdk.org, Jeremy Spewock <jspewock@iol.unh.edu>,
	 Paul Szczepanek <paul.szczepanek@arm.com>,
	Jack Bond-Preston <jack.bond-preston@arm.com>
Subject: Re: [PATCH 1/5] dts: fix InteractiveShell command prompt filtering
Date: Tue, 16 Apr 2024 10:40:32 +0200	[thread overview]
Message-ID: <CAOb5WZZYbO2jEQJ-GMJQ3NbKvzMy0Ob=knMML2qkzOcDegvu4g@mail.gmail.com> (raw)
In-Reply-To: <20240412111136.3470304-2-luca.vizzarro@arm.com>

On Fri, Apr 12, 2024 at 1:11 PM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> When sending a command using an instance of InteractiveShell the output
> is meant to filter out the leading shell prompt. The filtering logic is
> present but the line is appended anyways.
>

I don't think that's what's happening here. The output collecting
logic is "stop when we encounter a prompt, but not the prompt with the
command we sent". We could change the comment though.

> Bugzilla ID: 1411
> Fixes: 88489c0501af ("dts: add smoke tests")
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Reviewed-by: Jack Bond-Preston <jack.bond-preston@arm.com>
> ---
> Cc: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/framework/remote_session/interactive_shell.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
> index 5cfe202e15..8a9bf96ea9 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -132,11 +132,11 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
>          self._stdin.flush()
>          out: str = ""
>          for line in self._stdout:
> -            out += line
>              if prompt in line and not line.rstrip().endswith(
>                  command.rstrip()
>              ):  # ignore line that sent command
>                  break
> +            out += line

If we do this, we'll only filter out the last prompt, which may not be
desirable, since the last prompt is there only because all of our
interactive shells force an extra prompt with _command_extra_chars.

One thing we could improve though is removing the distribution welcome
message from logs, or at least separate it from the first command sent
with the interactive shell. The second option will allow us to see
clearly that an interactive session has been established, although we
could just emit a shorter log (something like "Started a testpmd
session" and then flush the welcome screen output).

>          self._logger.debug(f"Got output: {out}")
>          return out
>
> --
> 2.34.1
>

  reply	other threads:[~2024-04-16  8:40 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12 11:11 [PATCH 0/5] dts: testpmd show port info/stats Luca Vizzarro
2024-04-12 11:11 ` [PATCH 1/5] dts: fix InteractiveShell command prompt filtering Luca Vizzarro
2024-04-16  8:40   ` Juraj Linkeš [this message]
2024-04-16 12:12     ` Luca Vizzarro
2024-04-17 13:06       ` Juraj Linkeš
2024-04-17 14:17         ` Luca Vizzarro
2024-04-18  6:31           ` Juraj Linkeš
2024-04-29 16:16             ` Jeremy Spewock
2024-04-12 11:11 ` [PATCH 2/5] dts: skip first line of send_command output Luca Vizzarro
2024-04-16  8:48   ` Juraj Linkeš
2024-04-16 12:15     ` Luca Vizzarro
2024-04-17 13:18       ` Juraj Linkeš
2024-04-29 15:18         ` Jeremy Spewock
2024-04-12 11:11 ` [PATCH 3/5] dts: add parsing utility module Luca Vizzarro
2024-04-16  8:59   ` Juraj Linkeš
2024-04-16 12:16     ` Luca Vizzarro
2024-04-29 16:15   ` Jeremy Spewock
2024-04-30 10:49     ` Luca Vizzarro
2024-04-30 20:03       ` Jeremy Spewock
2024-04-12 11:11 ` [PATCH 4/5] dts: add `show port info` command to TestPmdShell Luca Vizzarro
2024-04-16  9:03   ` Juraj Linkeš
2024-04-16 12:24     ` Luca Vizzarro
2024-04-17 13:22       ` Juraj Linkeš
2024-04-17 14:25         ` Luca Vizzarro
2024-04-17 15:29           ` Luca Vizzarro
2024-04-18  6:41             ` Juraj Linkeš
2024-04-18 10:52               ` Luca Vizzarro
2024-04-12 11:11 ` [PATCH 5/5] dts: add `show port stats` " Luca Vizzarro
2024-04-16  9:04   ` Juraj Linkeš
2024-04-29 15:54   ` Jeremy Spewock
2024-04-30 10:51     ` Luca Vizzarro
2024-05-09 11:26 ` [PATCH v2 0/5] dts: testpmd show port info/stats Luca Vizzarro
2024-05-09 11:26   ` [PATCH v2 1/5] dts: fix InteractiveShell command prompt filtering Luca Vizzarro
2024-05-09 11:26   ` [PATCH v2 2/5] dts: skip first line of send command output Luca Vizzarro
2024-05-09 11:26   ` [PATCH v2 3/5] dts: add parsing utility module Luca Vizzarro
2024-05-09 11:26   ` [PATCH v2 4/5] dts: add `show port info` command to TestPmdShell Luca Vizzarro
2024-05-09 11:26   ` [PATCH v2 5/5] dts: add `show port stats` " Luca Vizzarro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOb5WZZYbO2jEQJ-GMJQ3NbKvzMy0Ob=knMML2qkzOcDegvu4g@mail.gmail.com' \
    --to=juraj.linkes@pantheon.tech \
    --cc=dev@dpdk.org \
    --cc=jack.bond-preston@arm.com \
    --cc=jspewock@iol.unh.edu \
    --cc=luca.vizzarro@arm.com \
    --cc=paul.szczepanek@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).