All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection
@ 2024-07-12 19:31 Brandon Maier via buildroot
  2024-07-14 11:19 ` Arnout Vandecappelle via buildroot
  2024-07-22 13:11 ` Thomas Petazzoni via buildroot
  0 siblings, 2 replies; 5+ messages in thread
From: Brandon Maier via buildroot @ 2024-07-12 19:31 UTC (permalink / raw)
  To: buildroot
  Cc: Julien Olivain, Brandon Maier, Thomas Petazzoni,
	Ricardo Martincoski

From: Brandon Maier via buildroot <buildroot@buildroot.org>

The qemu.run() method can break when a command happens to output the
string "# " to stdout. This is because qemu.run() detects when a command
has completed by searching for the shell prompt, which by default is
"# ". It then captures everything before the "# " as the commands
output, causing the rest of output to be lost.

Instead use the pexpect libraries REPLWrapper to handle running
commands. It has hooks to set a custom prompt and avoid some other
pitfalls of wrapping a shell.

Signed-off-by: Brandon Maier <brandon.maier@collins.com>
---
I tested this against a couple tests and test_bash.py which Yann noticed
is special in Julian patch series. I am running a full test run but
that will take awhile, so sending this in the meantime for review.

Changes in v2:
- Rewrite to use pexpect REPLWrapper (Arnout)
- Link to v1: https://patchwork.ozlabs.org/project/buildroot/patch/20240522154616.1265769-1-brandon.maier@collins.com/
- Link to Julian Olivain version: https://patchwork.ozlabs.org/project/buildroot/patch/20230611104518.337528-1-ju.o@free.fr/
---
 support/testing/infra/emulator.py | 56 +++++++++++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 11 deletions(-)

diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
index 624740fcb1..c586603787 100644
--- a/support/testing/infra/emulator.py
+++ b/support/testing/infra/emulator.py
@@ -1,12 +1,38 @@
 import pexpect
+import pexpect.replwrap
 
 import infra
 
+BR_PROMPT = '[BRTEST# '
+BR_CONTINUATION_PROMPT = '[BRTEST+ '
+
+
+def _repl_sh_spawn(spawn, orig_prompt, non_printable_insert='\\[\\]',
+                   extra_init_cmd="export PAGER=cat"):
+    """Wrap the shell prompt to handle command output
+    Based on pexpect.replwrap.bash()
+    https://github.com/pexpect/pexpect/blob/aa989594e1e413f45c18b26ded1783f7d5990fe5/pexpect/replwrap.py#L115
+    """
+
+    ps1 = BR_PROMPT[:5] + non_printable_insert + BR_PROMPT[5:]
+    ps2 = (BR_CONTINUATION_PROMPT[:5] + non_printable_insert +
+           BR_CONTINUATION_PROMPT[5:])
+    prompt_change = "PS1='{0}' PS2='{1}' PROMPT_COMMAND=''".format(ps1, ps2)
+    return pexpect.replwrap.REPLWrapper(
+            spawn,
+            orig_prompt,
+            prompt_change,
+            new_prompt=BR_PROMPT,
+            continuation_prompt=BR_CONTINUATION_PROMPT,
+            extra_init_cmd=extra_init_cmd
+        )
+
 
 class Emulator(object):
 
     def __init__(self, builddir, downloaddir, logtofile, timeout_multiplier):
         self.qemu = None
+        self.repl = None
         self.downloaddir = downloaddir
         self.logfile = infra.open_log_file(builddir, "run", logtofile)
         # We use elastic runners on the cloud to runs our tests. Those runners
@@ -97,26 +123,34 @@ class Emulator(object):
         if password:
             self.qemu.expect("Password:")
             self.qemu.sendline(password)
-        index = self.qemu.expect(["# ", pexpect.TIMEOUT])
-        if index != 0:
-            raise SystemError("Cannot login")
-        self.run("dmesg -n 1")
-        # Prevent the shell from wrapping the commands at 80 columns.
-        self.run("stty columns 29999")
+
+        extra_init_cmd = " && ".join([
+            'export PAGER=cat',
+            'dmesg -n 1',
+            # Prevent the shell from wrapping the commands at 80 columns.
+            'stty columns 29999',
+            # Fix the prompt of any subshells that get run
+            '''printf "%s\n"  "PS1='$PS1'" "PS2='$PS2'" "PROMPT_COMMAND=''" >>/etc/profile'''
+        ])
+        self.repl = _repl_sh_spawn(
+                self.qemu,
+                '# ',
+                non_printable_insert='\\[\\]',
+                extra_init_cmd=extra_init_cmd)
+        if not self.repl:
+            raise SystemError("Cannot initialize REPL prompt")
 
     # Run the given 'cmd' with a 'timeout' on the target
     # return a tuple (output, exit_code)
     def run(self, cmd, timeout=-1):
-        self.qemu.sendline(cmd)
         if timeout != -1:
             timeout *= self.timeout_multiplier
-        self.qemu.expect("# ", timeout=timeout)
+        output = self.repl.run_command(cmd, timeout=timeout)
         # Remove double carriage return from qemu stdout so str.splitlines()
         # works as expected.
-        output = self.qemu.before.replace("\r\r", "\r").splitlines()[1:]
+        output = output.replace("\r\r", "\r").splitlines()[1:]
 
-        self.qemu.sendline("echo $?")
-        self.qemu.expect("# ")
+        exit_code = self.repl.run_command("echo $?")
         exit_code = self.qemu.before.splitlines()[2]
         exit_code = int(exit_code)
 

---
base-commit: 5543661bc0c1ebcd01e5e6bf5ac565fd0f1ac8d8
change-id: 20240712-pytest-qemu-prompt-detection-34f3ed337b09

Best regards,
-- 
Brandon Maier <brandon.maier@collins.com>

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection
  2024-07-12 19:31 [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection Brandon Maier via buildroot
@ 2024-07-14 11:19 ` Arnout Vandecappelle via buildroot
  2024-07-22 13:11 ` Thomas Petazzoni via buildroot
  1 sibling, 0 replies; 5+ messages in thread
From: Arnout Vandecappelle via buildroot @ 2024-07-14 11:19 UTC (permalink / raw)
  To: Brandon Maier, buildroot
  Cc: Julien Olivain, Thomas Petazzoni, Ricardo Martincoski

  Hi Brandon,

On 12/07/2024 21:31, Brandon Maier wrote:
> From: Brandon Maier via buildroot <buildroot@buildroot.org>
> 
> The qemu.run() method can break when a command happens to output the
> string "# " to stdout. This is because qemu.run() detects when a command
> has completed by searching for the shell prompt, which by default is
> "# ". It then captures everything before the "# " as the commands
> output, causing the rest of output to be lost.
> 
> Instead use the pexpect libraries REPLWrapper to handle running
> commands. It has hooks to set a custom prompt and avoid some other
> pitfalls of wrapping a shell.
> 
> Signed-off-by: Brandon Maier <brandon.maier@collins.com>
> ---
> I tested this against a couple tests and test_bash.py which Yann noticed
> is special in Julian patch series. I am running a full test run but
> that will take awhile, so sending this in the meantime for review.

  I'm also running the full battery but it will take +- 20 hours to finish.

  I've merged already. If there's breakage, we'll fix it :-)

  There are a couple more changes I made:

      - Make all arguments to _repl_sh_spawn non-optional.
      - Move non_printable_insert to a local variable instead of an argument,
        we don't need to override it.
      - Copy the comment from _repl_sh that explains why non_printable_insert
        is needed.
      - Add a comment about timeouts.
      - Rename spawn to child (we don't actually spawn anything so this felt
        more natural, even though the class
      - Use single quotes instead of triple quotes, and explicitly escape the
        nested quotes.

  I also pushed one additional commit that adds a copyright header. The copied 
_repl_sh part makes the file ISC licensed in addition to our default GPL-2.0, so 
it's better to make that explicit.

  Regards,
  Arnout

> 
> Changes in v2:
> - Rewrite to use pexpect REPLWrapper (Arnout)
> - Link to v1: https://patchwork.ozlabs.org/project/buildroot/patch/20240522154616.1265769-1-brandon.maier@collins.com/
> - Link to Julian Olivain version: https://patchwork.ozlabs.org/project/buildroot/patch/20230611104518.337528-1-ju.o@free.fr/
> ---
>   support/testing/infra/emulator.py | 56 +++++++++++++++++++++++++++++++--------
>   1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
> index 624740fcb1..c586603787 100644
> --- a/support/testing/infra/emulator.py
> +++ b/support/testing/infra/emulator.py
> @@ -1,12 +1,38 @@
>   import pexpect
> +import pexpect.replwrap
>   
>   import infra
>   
> +BR_PROMPT = '[BRTEST# '
> +BR_CONTINUATION_PROMPT = '[BRTEST+ '
> +
> +
> +def _repl_sh_spawn(spawn, orig_prompt, non_printable_insert='\\[\\]',
> +                   extra_init_cmd="export PAGER=cat"):
> +    """Wrap the shell prompt to handle command output
> +    Based on pexpect.replwrap.bash()
> +    https://github.com/pexpect/pexpect/blob/aa989594e1e413f45c18b26ded1783f7d5990fe5/pexpect/replwrap.py#L115
> +    """
> +
> +    ps1 = BR_PROMPT[:5] + non_printable_insert + BR_PROMPT[5:]
> +    ps2 = (BR_CONTINUATION_PROMPT[:5] + non_printable_insert +
> +           BR_CONTINUATION_PROMPT[5:])
> +    prompt_change = "PS1='{0}' PS2='{1}' PROMPT_COMMAND=''".format(ps1, ps2)
> +    return pexpect.replwrap.REPLWrapper(
> +            spawn,
> +            orig_prompt,
> +            prompt_change,
> +            new_prompt=BR_PROMPT,
> +            continuation_prompt=BR_CONTINUATION_PROMPT,
> +            extra_init_cmd=extra_init_cmd
> +        )
> +
>   
>   class Emulator(object):
>   
>       def __init__(self, builddir, downloaddir, logtofile, timeout_multiplier):
>           self.qemu = None
> +        self.repl = None
>           self.downloaddir = downloaddir
>           self.logfile = infra.open_log_file(builddir, "run", logtofile)
>           # We use elastic runners on the cloud to runs our tests. Those runners
> @@ -97,26 +123,34 @@ class Emulator(object):
>           if password:
>               self.qemu.expect("Password:")
>               self.qemu.sendline(password)
> -        index = self.qemu.expect(["# ", pexpect.TIMEOUT])
> -        if index != 0:
> -            raise SystemError("Cannot login")
> -        self.run("dmesg -n 1")
> -        # Prevent the shell from wrapping the commands at 80 columns.
> -        self.run("stty columns 29999")
> +
> +        extra_init_cmd = " && ".join([
> +            'export PAGER=cat',
> +            'dmesg -n 1',
> +            # Prevent the shell from wrapping the commands at 80 columns.
> +            'stty columns 29999',
> +            # Fix the prompt of any subshells that get run
> +            '''printf "%s\n"  "PS1='$PS1'" "PS2='$PS2'" "PROMPT_COMMAND=''" >>/etc/profile'''
> +        ])
> +        self.repl = _repl_sh_spawn(
> +                self.qemu,
> +                '# ',
> +                non_printable_insert='\\[\\]',
> +                extra_init_cmd=extra_init_cmd)
> +        if not self.repl:
> +            raise SystemError("Cannot initialize REPL prompt")
>   
>       # Run the given 'cmd' with a 'timeout' on the target
>       # return a tuple (output, exit_code)
>       def run(self, cmd, timeout=-1):
> -        self.qemu.sendline(cmd)
>           if timeout != -1:
>               timeout *= self.timeout_multiplier
> -        self.qemu.expect("# ", timeout=timeout)
> +        output = self.repl.run_command(cmd, timeout=timeout)
>           # Remove double carriage return from qemu stdout so str.splitlines()
>           # works as expected.
> -        output = self.qemu.before.replace("\r\r", "\r").splitlines()[1:]
> +        output = output.replace("\r\r", "\r").splitlines()[1:]
>   
> -        self.qemu.sendline("echo $?")
> -        self.qemu.expect("# ")
> +        exit_code = self.repl.run_command("echo $?")
>           exit_code = self.qemu.before.splitlines()[2]
>           exit_code = int(exit_code)
>   
> 
> ---
> base-commit: 5543661bc0c1ebcd01e5e6bf5ac565fd0f1ac8d8
> change-id: 20240712-pytest-qemu-prompt-detection-34f3ed337b09
> 
> Best regards,
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection
  2024-07-12 19:31 [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection Brandon Maier via buildroot
  2024-07-14 11:19 ` Arnout Vandecappelle via buildroot
@ 2024-07-22 13:11 ` Thomas Petazzoni via buildroot
  2024-07-22 17:26   ` Brandon Maier via buildroot
  1 sibling, 1 reply; 5+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-07-22 13:11 UTC (permalink / raw)
  To: Brandon Maier via buildroot
  Cc: Julien Olivain, Ricardo Martincoski, Brandon Maier

On Fri, 12 Jul 2024 19:31:04 +0000
Brandon Maier via buildroot <buildroot@buildroot.org> wrote:

> From: Brandon Maier via buildroot <buildroot@buildroot.org>
> 
> The qemu.run() method can break when a command happens to output the
> string "# " to stdout. This is because qemu.run() detects when a command
> has completed by searching for the shell prompt, which by default is
> "# ". It then captures everything before the "# " as the commands
> output, causing the rest of output to be lost.
> 
> Instead use the pexpect libraries REPLWrapper to handle running
> commands. It has hooks to set a custom prompt and avoid some other
> pitfalls of wrapping a shell.
> 
> Signed-off-by: Brandon Maier <brandon.maier@collins.com>
> ---
> I tested this against a couple tests and test_bash.py which Yann noticed
> is special in Julian patch series. I am running a full test run but
> that will take awhile, so sending this in the meantime for review.

It seems like this change has broken one test:

  https://gitlab.com/buildroot.org/buildroot/-/jobs/7391792869

Traceback (most recent call last):
  File "/builds/buildroot.org/buildroot/support/testing/tests/init/test_none.py", line 26, in test_run
    out, exit_code = self.emulator.run("sh -c 'echo $PPID'")
  File "/builds/buildroot.org/buildroot/support/testing/infra/emulator.py", line 153, in run
    output = self.repl.run_command(cmd, timeout=timeout)
AttributeError: 'NoneType' object has no attribute 'run_command'

Could you have a look perhaps?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection
  2024-07-22 13:11 ` Thomas Petazzoni via buildroot
@ 2024-07-22 17:26   ` Brandon Maier via buildroot
  2024-07-22 19:50     ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 5+ messages in thread
From: Brandon Maier via buildroot @ 2024-07-22 17:26 UTC (permalink / raw)
  To: Thomas Petazzoni, Brandon Maier via buildroot
  Cc: Julien Olivain, Ricardo Martincoski

Hi Thomas,

On Mon Jul 22, 2024 at 1:11 PM UTC, Thomas Petazzoni via buildroot wrote:
> On Fri, 12 Jul 2024 19:31:04 +0000
> Brandon Maier via buildroot <buildroot@buildroot.org> wrote:
>
> > From: Brandon Maier via buildroot <buildroot@buildroot.org>
> > 
> > The qemu.run() method can break when a command happens to output the
> > string "# " to stdout. This is because qemu.run() detects when a command
> > has completed by searching for the shell prompt, which by default is
> > "# ". It then captures everything before the "# " as the commands
> > output, causing the rest of output to be lost.
> > 
> > Instead use the pexpect libraries REPLWrapper to handle running
> > commands. It has hooks to set a custom prompt and avoid some other
> > pitfalls of wrapping a shell.
> > 
> > Signed-off-by: Brandon Maier <brandon.maier@collins.com>
> > ---
> > I tested this against a couple tests and test_bash.py which Yann noticed
> > is special in Julian patch series. I am running a full test run but
> > that will take awhile, so sending this in the meantime for review.
>
> It seems like this change has broken one test:
>
>   https://gitlab.com/buildroot.org/buildroot/-/jobs/7391792869
>
> Traceback (most recent call last):
>   File "/builds/buildroot.org/buildroot/support/testing/tests/init/test_none.py", line 26, in test_run
>     out, exit_code = self.emulator.run("sh -c 'echo $PPID'")
>   File "/builds/buildroot.org/buildroot/support/testing/infra/emulator.py", line 153, in run
>     output = self.repl.run_command(cmd, timeout=timeout)
> AttributeError: 'NoneType' object has no attribute 'run_command'
>
> Could you have a look perhaps?

I already sent a fix for this ;)

https://patchwork.ozlabs.org/project/buildroot/patch/20240715-support-testing-fix-testinitsystemnone-v1-2-299145b2d342@collins.com/

Thanks,
Brandon

>
> Thanks a lot!
>
> Thomas

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection
  2024-07-22 17:26   ` Brandon Maier via buildroot
@ 2024-07-22 19:50     ` Thomas Petazzoni via buildroot
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-07-22 19:50 UTC (permalink / raw)
  To: Brandon Maier
  Cc: Julien Olivain, Ricardo Martincoski, Brandon Maier via buildroot

On Mon, 22 Jul 2024 17:26:50 +0000
"Brandon Maier" <brandon.maier@collins.com> wrote:

> I already sent a fix for this ;)
> 
> https://patchwork.ozlabs.org/project/buildroot/patch/20240715-support-testing-fix-testinitsystemnone-v1-2-299145b2d342@collins.com/

Thanks, applied!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-07-22 19:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-12 19:31 [Buildroot] [PATCH v2] support/testing/infra/emulator.py: fix qemu prompt detection Brandon Maier via buildroot
2024-07-14 11:19 ` Arnout Vandecappelle via buildroot
2024-07-22 13:11 ` Thomas Petazzoni via buildroot
2024-07-22 17:26   ` Brandon Maier via buildroot
2024-07-22 19:50     ` Thomas Petazzoni via buildroot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.