Buildroot Archive on 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox