[Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Edgar Bonet]

Traditional VT-10x terminals (and their emulators) have a "magic
margins" feature that enables the last character position to be updated
without scrolling the screen: whenever a character is printed on the
last column, the cursor stays over the character, instead of moving to
the next line.

The Busybox shell, ash, attempts to defeat this feature by printing
CF,LF right after echoing a character to the last column.[1] This
doesn't play well with emulator.py. The run() method of the Emulator
class captures the output of the emulated system and assumes the first
line it reads is the echo of the command, and all subsequent lines are
the command's output. If the line made by the command + shell prompt is
longer than 80 characters, then it is echoed as two or more lines, and
all but the first one are mistaken for the command's output.

We fix this by telling the emulated system that we are using an
ultra-wide terminal with 29999 columns. Larger values would be ignored
and replaced by the default, namely 80 columns.[2]

[1] https://git.busybox.net/busybox/tree/libbb/lineedit.c?h=1_34_0#n412
[2] https://git.busybox.net/busybox/tree/libbb/xfuncs.c?h=1_34_0#n258

Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Co-authored-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 support/testing/infra/emulator.py | 2 ++
 1 file changed, 2 insertions(+)

Note that I tested that the command

    stty columns 29999

does fix the wrapping behavior on an actual board, while using a serial
connection. I did not test the script emulator.py, as I am unfamiliar
with the testing infrastructure. Would someone volunteer to do that
test?

diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
index 0a77eb80fc..f1f5eac3a8 100644
--- a/support/testing/infra/emulator.py
+++ b/support/testing/infra/emulator.py
@@ -100,6 +100,8 @@ class Emulator(object):
         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")
 
     # Run the given 'cmd' with a 'timeout' on the target
     # return a tuple (output, exit_code)
-- 
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Yann E. MORIN]

Edgar, All,

+Thomas

On 2021-10-05 19:17 +0200, Edgar Bonet spake thusly:
> Traditional VT-10x terminals (and their emulators) have a "magic
> margins" feature that enables the last character position to be updated
> without scrolling the screen: whenever a character is printed on the
> last column, the cursor stays over the character, instead of moving to
> the next line.
> 
> The Busybox shell, ash, attempts to defeat this feature by printing
> CF,LF right after echoing a character to the last column.[1] This
> doesn't play well with emulator.py. The run() method of the Emulator
> class captures the output of the emulated system and assumes the first
> line it reads is the echo of the command, and all subsequent lines are
> the command's output. If the line made by the command + shell prompt is
> longer than 80 characters, then it is echoed as two or more lines, and
> all but the first one are mistaken for the command's output.
> 
> We fix this by telling the emulated system that we are using an
> ultra-wide terminal with 29999 columns. Larger values would be ignored
> and replaced by the default, namely 80 columns.[2]
> 
> [1] https://git.busybox.net/busybox/tree/libbb/lineedit.c?h=1_34_0#n412
> [2] https://git.busybox.net/busybox/tree/libbb/xfuncs.c?h=1_34_0#n258

Woot! Very, very good commit log, thanks!

So I decided to investigate those "magic margins" (I'm a grey beard,
but not that grey yet, so I missed that info). And thus I stumbled on
that (and yes, I've now bookmarked that for future reference; I'm just
torn between bookmarking in the "software" or "hardware" categories...
:-] ):

    https://vt100.net/docs/vt100-ug/chapter3.html

    DECAWM – Autowrap Mode (DEC Private)

    The reset state causes any displayable characters received when the
    cursor is at the right margin to replace any previous characters
    there. The set state causes these characters to advance to the start
    of the next line, doing a scroll up if required and permitted.

But that seems to be DEC-only, and I could not get a better, more
generic hit.

> Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Co-authored-by: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
>  support/testing/infra/emulator.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> Note that I tested that the command
> 
>     stty columns 29999
> 
> does fix the wrapping behavior on an actual board, while using a serial
> connection. I did not test the script emulator.py, as I am unfamiliar
> with the testing infrastructure. Would someone volunteer to do that
> test?

The thing is, as Thomas said, he had the issue with new tests he was
adding to the infra; we currently do not have tests with commands that
are too long to be wrapped.

Thomas?

> diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
> index 0a77eb80fc..f1f5eac3a8 100644
> --- a/support/testing/infra/emulator.py
> +++ b/support/testing/infra/emulator.py
> @@ -100,6 +100,8 @@ class Emulator(object):
>          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")

I would do that even before running dmesg. No need to resend, we can
change when applying, unless there is a good reason not to.

Regards,
Yann E. MORIN.

>      # Run the given 'cmd' with a 'timeout' on the target
>      # return a tuple (output, exit_code)
> -- 
> 2.25.1
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Thomas Petazzoni]

Hello Yann,

On Tue, 5 Oct 2021 22:01:11 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> On 2021-10-05 19:17 +0200, Edgar Bonet spake thusly:
> > Traditional VT-10x terminals (and their emulators) have a "magic
> > margins" feature that enables the last character position to be updated
> > without scrolling the screen: whenever a character is printed on the
> > last column, the cursor stays over the character, instead of moving to
> > the next line.
> > 
> > The Busybox shell, ash, attempts to defeat this feature by printing
> > CF,LF right after echoing a character to the last column.[1] This
> > doesn't play well with emulator.py. The run() method of the Emulator
> > class captures the output of the emulated system and assumes the first
> > line it reads is the echo of the command, and all subsequent lines are
> > the command's output. If the line made by the command + shell prompt is
> > longer than 80 characters, then it is echoed as two or more lines, and
> > all but the first one are mistaken for the command's output.
> > 
> > We fix this by telling the emulated system that we are using an
> > ultra-wide terminal with 29999 columns. Larger values would be ignored
> > and replaced by the default, namely 80 columns.[2]
> > 
> > [1] https://git.busybox.net/busybox/tree/libbb/lineedit.c?h=1_34_0#n412
> > [2] https://git.busybox.net/busybox/tree/libbb/xfuncs.c?h=1_34_0#n258  
> 
> Woot! Very, very good commit log, thanks!

And very good investigation too!

> The thing is, as Thomas said, he had the issue with new tests he was
> adding to the infra; we currently do not have tests with commands that
> are too long to be wrapped.
> 
> Thomas?

We do:

  ./support/testing/tests/package/test_python_flask_expects_json.py

is such a test. The test is doing:

        self.assertEqual(output[-1], str(expects))

instead of:

        self.assertEqual(output[0], str(expects))

precisely to work around this issue. I.e instead of getting the first
line of the output (which would be the end of the command being
executed), I test against the last line of the output. It works in my
case because I know my "interesting" output has only a single line.

So, if that test works by using output[0] instead of output[-1], then
the fix of using stty columns 29999 would be confirmed to work!

Best regards,

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

Re: [Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Edgar Bonet]

Hello!

About testing this patch, Thomas Petazzoni wrote:
>   ./support/testing/tests/package/test_python_flask_expects_json.py
>
> is such a test. The test is doing:
>
>         self.assertEqual(output[-1], str(expects))
>
> [...] if that test works by using output[0] instead of output[-1],
> then the fix of using stty columns 29999 would be confirmed to work!

I tried to run this test but ran into an unexpected issue. The test
issues commands like

    curl -s -o /dev/null -w "%%{http_code}\\n" -X POST ...

and then curl outputs "%{http_code}" literally. The test fails with

    AssertionError: '%{http_code}' != '200'

It looks like curl simply replaced "%%" with "%", which is consistent
with `man curl'. I replaced %%" with "%" in
test_python_flask_expects_json.py and then got the expected behavior:

1. With no changes to master other than the above mentioned 's/%%/%/',
   the test succeeds.

2. If, in addition to this change, I replace output[-1] with output[0],
   the test fails with

     AssertionError: '' != '200'

   Why did it get an empty line? For some reason the command line got
   cut with the sequence CR,CR,CR,LF (yes, 3 CRs in a row). I guess the
   host tty driver may be adding its own CRs, as every CR,LF sequence in
   the output log had another CR right before it.

3. If, in addition to these two changes, I add the patch discussed here,
   the test succeeds again.

I would say the `stty columns 29999' fix is now confirmed to work.

Regards,

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

Re: [Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Yann E. MORIN]

Edgar, All,

On 2021-10-06 12:22 +0200, Edgar Bonet spake thusly:
> About testing this patch, Thomas Petazzoni wrote:
> >   ./support/testing/tests/package/test_python_flask_expects_json.py
> I tried to run this test but ran into an unexpected issue. The test
> issues commands like
>     curl -s -o /dev/null -w "%%{http_code}\\n" -X POST ...
> and then curl outputs "%{http_code}" literally. The test fails with
>     AssertionError: '%{http_code}' != '200'

I can confirm that the test is currently broken for me too...

> It looks like curl simply replaced "%%" with "%", which is consistent
> with `man curl'. I replaced %%" with "%" in
> test_python_flask_expects_json.py and then got the expected behavior:
> 
> 1. With no changes to master other than the above mentioned 's/%%/%/',
>    the test succeeds.
> 
> 2. If, in addition to this change, I replace output[-1] with output[0],
>    the test fails with
> 
>      AssertionError: '' != '200'
> 
>    Why did it get an empty line? For some reason the command line got
>    cut with the sequence CR,CR,CR,LF (yes, 3 CRs in a row). I guess the
>    host tty driver may be adding its own CRs, as every CR,LF sequence in
>    the output log had another CR right before it.
> 
> 3. If, in addition to these two changes, I add the patch discussed here,
>    the test succeeds again.
> 
> I would say the `stty columns 29999' fix is now confirmed to work.

Agreed, it works as expected.

Since the test is already broken before this patch, I'll apply you
change, and we can fix the test case in a subsequent patch.

Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Yann E. MORIN]

Edgar, All,

On 2021-10-05 19:17 +0200, Edgar Bonet spake thusly:
> Traditional VT-10x terminals (and their emulators) have a "magic

I've added a reference to https://vt100.net/docs/vt100-ug/chapter3.html,
and...

> margins" feature that enables the last character position to be updated
> without scrolling the screen: whenever a character is printed on the
> last column, the cursor stays over the character, instead of moving to
> the next line.
> 
> The Busybox shell, ash, attempts to defeat this feature by printing
> CF,LF right after echoing a character to the last column.[1] This

... I fixed s/CF/CR/ here, then applied to master, thanks.

Regards,
Yann E. MORIN.

> doesn't play well with emulator.py. The run() method of the Emulator
> class captures the output of the emulated system and assumes the first
> line it reads is the echo of the command, and all subsequent lines are
> the command's output. If the line made by the command + shell prompt is
> longer than 80 characters, then it is echoed as two or more lines, and
> all but the first one are mistaken for the command's output.
> 
> We fix this by telling the emulated system that we are using an
> ultra-wide terminal with 29999 columns. Larger values would be ignored
> and replaced by the default, namely 80 columns.[2]
> 
> [1] https://git.busybox.net/busybox/tree/libbb/lineedit.c?h=1_34_0#n412
> [2] https://git.busybox.net/busybox/tree/libbb/xfuncs.c?h=1_34_0#n258
> 
> Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Co-authored-by: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
>  support/testing/infra/emulator.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> Note that I tested that the command
> 
>     stty columns 29999
> 
> does fix the wrapping behavior on an actual board, while using a serial
> connection. I did not test the script emulator.py, as I am unfamiliar
> with the testing infrastructure. Would someone volunteer to do that
> test?
> 
> diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
> index 0a77eb80fc..f1f5eac3a8 100644
> --- a/support/testing/infra/emulator.py
> +++ b/support/testing/infra/emulator.py
> @@ -100,6 +100,8 @@ class Emulator(object):
>          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")
>  
>      # Run the given 'cmd' with a 'timeout' on the target
>      # return a tuple (output, exit_code)
> -- 
> 2.25.1
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] support/testing/infra/emulator.py: prevent the commands from wrapping

[Peter Korsgaard]

>>>>> "Edgar" == Edgar Bonet <bonet@grenoble.cnrs.fr> writes:

 > Traditional VT-10x terminals (and their emulators) have a "magic
 > margins" feature that enables the last character position to be updated
 > without scrolling the screen: whenever a character is printed on the
 > last column, the cursor stays over the character, instead of moving to
 > the next line.

 > The Busybox shell, ash, attempts to defeat this feature by printing
 > CF,LF right after echoing a character to the last column.[1] This
 > doesn't play well with emulator.py. The run() method of the Emulator
 > class captures the output of the emulated system and assumes the first
 > line it reads is the echo of the command, and all subsequent lines are
 > the command's output. If the line made by the command + shell prompt is
 > longer than 80 characters, then it is echoed as two or more lines, and
 > all but the first one are mistaken for the command's output.

 > We fix this by telling the emulated system that we are using an
 > ultra-wide terminal with 29999 columns. Larger values would be ignored
 > and replaced by the default, namely 80 columns.[2]

 > [1] https://git.busybox.net/busybox/tree/libbb/lineedit.c?h=1_34_0#n412
 > [2] https://git.busybox.net/busybox/tree/libbb/xfuncs.c?h=1_34_0#n258

 > Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
 > Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Co-authored-by: Yann E. MORIN <yann.morin.1998@free.fr>
 > ---
 >  support/testing/infra/emulator.py | 2 ++
 >  1 file changed, 2 insertions(+)

 > Note that I tested that the command

 >     stty columns 29999

 > does fix the wrapping behavior on an actual board, while using a serial
 > connection. I did not test the script emulator.py, as I am unfamiliar
 > with the testing infrastructure. Would someone volunteer to do that
 > test?

Committed to 2021.02.x, 2021.05.x and 2021.08.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot