| Message ID | 914a4374-e517-b730-c45f-f37ec8147649@grenoble.cnrs.fr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] support/testing/infra/emulator.py: prevent the commands from wrapping | expand |
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
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
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.
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.
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
>>>>> "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.
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)
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?