Re: [Buildroot] [PATCH v2 1/3] support/testing/infra/emulator.py: change the shell prompt before running tests

[Julien Olivain <ju.o@free.fr>]

Hi Yann,

On 26/06/2023 23:06, Yann E. MORIN wrote:
> Julien, All,
> 
> On 2023-06-11 12:45 +0200, Julien Olivain spake thusly:
>> If a program has the string '# ' (i.e. the default shell prompt) in
>> its output, the test execution in the Buildroot runtime test infra is
>> failing.
>> 
>> This can be reproduced by adding a single line in a package test
>> script:
>>     self.assertRunOk("echo ### this is a string with hashes ###")
> 
> The most obvious issue is when testing the environment of the shell 
> with
> env, which will output PS1 itself... There is not easy way around to 
> fix
> that, of course... :-/
> 
> I was beaten by this when adding a test for bash. I eventually solved 
> it
> in another way, but took a note to find a better solution "for 
> later"...
> ;-)
> 
>> Since the string "# " is quite common, this patch changes the prompt
>> after the emulator.login(), by setting the PS1 variable to a string
>> which is less likely to appear in a normal program output. A small
>> caveat: since there is a command echo, the command setting the new
>> prompt needs to be protected to make sure it will not be detected as
>> an actual shell prompt. The prompt is encoded by single-quoting
>> each character (e.g. abc -> 'a''b''c').
> 
> That's not really what join() will do. E.g. with your code:
> 
>     >>> s = "#BRTEST# "
>     >>> "''".join(s)
>     "#''B''R''T''E''S''T''#'' "

I agree doing this, we miss the first and last quote. This is exactly 
what
the next line is doing:

     self.run("export PS1='{}'".format(encoded_prompt))

Note the extra quotes. If you look in a run-test run log, there will be:

     # export PS1='#''B''R''T''E''S''T''#'' '
     #BRTEST# echo $?
     0

This was my initial intent: setting the PS1 without the local echo
begin caught by pexpect.

If you prefer any other encoding or prompt, please suggest something
else. I initially tried by temporarily disabling the local echo with
"stty -echo" but this was creating other issues...

> But we don;t really care because it is then quite improbable that some
> random program will output exactly this...
> 
> I was on my side thinking about using ANSI escape sequences to carry 
> the
> information that the command was actually fnished, something along the
> lines of:
> 
>     export PS1="\x1B_BR_COMMAND_FINISHED\x1B\\# "
> 
>   - \x1B_ is APC (Application Program Command),
>   - \x1B\\ is ST (String Terminator), which terminates APC
> 
> Instead of APC, we could use any of:
> 
>   - \x1BP DCS, Device Control String
>   - \x1BX SOS, Start Of String
>   - \x1B^ PM, Privacy Message
>   - or even \x1B] OSC, Operating System Command
> 
> But maybe that is a bit overkill in the end...
> 
> Anyway, your patch at least breaks tests.package.test_bash...

I overlooked this bash test, while testing this patch. Thanks for
pointing that out. I tried with a dozen of test cases, but not that
one.

The /etc/profile in Buildroot skeleton is overwriting PS1, see:
https://git.buildroot.org/buildroot/tree/system/skeleton/etc/profile?h=2023.05#n5

I was able to fix this test_bash by passing the PS1 in another exported
variable:

     self.assertRunOk("export BR_PS1=\"$PS1\"")
     self.emulator.qemu.sendline("bash -il")
     self.assertRunOk("export PS1=\"${BR_PS1}\"")

The "bash -il" return code is no longer tested, but I don't think it's
an issue since the rest of the test covers that.

If you agree with this workaround, I'll send an updated v3 patch series.

> 
> Regards,
> Yann E. MORIN.
> 
>> Signed-off-by: Julien Olivain <ju.o@free.fr>
>> ---
>> Changes v1 -> v2:
>> - reworded commit log, to mention this issue was also seen while 
>> writing
>>   a test for the dmidecode package
>> - the patch series also introduce the new test for dmidecode
>> ---
>>  support/testing/infra/emulator.py | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>> 
>> diff --git a/support/testing/infra/emulator.py 
>> b/support/testing/infra/emulator.py
>> index 02cf486128..390c582e9d 100644
>> --- a/support/testing/infra/emulator.py
>> +++ b/support/testing/infra/emulator.py
>> @@ -13,6 +13,7 @@ class Emulator(object):
>>          # can take a long time to run the emulator. Use a timeout 
>> multiplier
>>          # when running the tests to avoid sporadic failures.
>>          self.timeout_multiplier = timeout_multiplier
>> +        self.shell_prompt = "#BRTEST# "
>> 
>>      # Start Qemu to boot the system
>>      #
>> @@ -100,6 +101,15 @@ class Emulator(object):
>>          index = self.qemu.expect(["# ", pexpect.TIMEOUT])
>>          if index != 0:
>>              raise SystemError("Cannot login")
>> +        # Set a special shell prompt while testing. Since the 
>> standard
>> +        # prompt '# ' is quite generic, a normal process output could
>> +        # contain that string and confuse expect. When changing the
>> +        # prompt, we also need to encode or escape it in some way to
>> +        # make sure the command echo will not be seen as a prompt
>> +        # itself. The prompt is encoded by single-quoting each
>> +        # character (e.g. abc -> 'a''b''c').
>> +        encoded_prompt = "''".join(self.shell_prompt)
>> +        self.run("export PS1='{}'".format(encoded_prompt))
>>          self.run("dmesg -n 1")
>>          # Prevent the shell from wrapping the commands at 80 columns.
>>          self.run("stty columns 29999")
>> @@ -110,13 +120,13 @@ class Emulator(object):
>>          self.qemu.sendline(cmd)
>>          if timeout != -1:
>>              timeout *= self.timeout_multiplier
>> -        self.qemu.expect("# ", timeout=timeout)
>> +        self.qemu.expect(self.shell_prompt, 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:]
>> 
>>          self.qemu.sendline("echo $?")
>> -        self.qemu.expect("# ")
>> +        self.qemu.expect(self.shell_prompt)
>>          exit_code = self.qemu.before.splitlines()[2]
>>          exit_code = int(exit_code)
>> 
>> --
>> 2.41.0
>> 
>> _______________________________________________
>> buildroot mailing list
>> buildroot@buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot

Best regards,

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