From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Wed, 15 Nov 2017 11:31:30 -0700
Subject: [U-Boot] [PATCH] test/py: Add dependency on HUSH parser
In-Reply-To: <836cf295-8166-5bf3-d074-d0e80a24e229@monstr.eu>
References: <b63ceda928ac6402390be3d3334fd2020f702f45.1510311687.git.michal.simek@xilinx.com>
 <81bb13dd-f8e4-bd96-93ff-78e371a07b30@wwwdotorg.org>
 <836cf295-8166-5bf3-d074-d0e80a24e229@monstr.eu>
Message-ID: <9c0b7102-77a8-77eb-e6bc-a7b79899d90a@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 11/15/2017 02:34 AM, Michal Simek wrote:
> Hi,
> 
> On 10.11.2017 22:34, Stephen Warren wrote:
>> On 11/10/2017 04:01 AM, Michal Simek wrote:
>>> After adding our small zynq uboot which has hush parser off these 3 tests
>>> start to failed. It is probably just coincidence that others are
>>> passing without hush parser.
>>
>> What was the exact problem here? The set of tests you've disabled all
>> seem to rely on setenv/printenv and don't seem to do anything
>> complicated shell syntax wise. Are you sure they shouldn't depend on
>> setenv being available rather than hush_parser?
> 
> These 3 tests. (html attached too.)
> 
> Zynq> printenv baudrate
> baudrate=115200
> Zynq>
> test/py/tests/test_env.py .sZynq> printenv test_env_0
> ## Error: "test_env_0" not defined
> Zynq> .sZynq> setenv test_env_0 "foo"
> Zynq> printenv test_env_0
> test_env_0="foo"
> Zynq> F+u-boot-test-reset zynq_cse_qspi zc706
> 
> 
> 
> Zynq> Zynq> setenv test_env_0 "bar"
> Zynq> printenv test_env_0
> test_env_0="bar"
> Zynq> F

For those two failures, the issue is that the test is expecting setenv 
to print:

test_env_0=foo
test_env_0=bar

... but it prints:

test_env_0="foo"
test_env_0="bar"

I guess this is because the set_var() function wraps the values in 
quotes to ensure that values that contain spaces work as expected, yet 
when not using Hush, quotes aren't processed.

I think the best solution is to enhance set_var() to do the following if 
Hush isn't available:

a) Skip the test if the value contains any spaces (or perhaps if the 
value contains any characters that aren't in a whitelist.

b) Not use quotes when not running on Hush.

I think re-writing it as follows would work:

>     bc = state_test_env.u_boot_console.config.buildconfig
>     if bc.get('config_hush_parser', None):
>         quote = '"'
>     else:
>         quote = ''
>         if ' ' in value:
>             pytest.skip('Space in variable value on non-Hush shell')
> 
>     state_test_env.u_boot_console.run_command(
>         'setenv %s %s%s%s' % (var, quote, value, quote))
>     state_test_env.env[var] = value

s+u-boot-test-reset zynq_cse_qspi zc706
> 
> Zynq> Zynq> setenv test_env_1 " "
> Zynq> setenv test_env_2 " 1${test_env_1}${test_env_1} 2 "
> Zynq> printenv test_env_2
> test_env_2=" 1" "" " 2 "
> Zynq> setenv test_env_1
> Zynq> setenv test_env_2
> Zynq> F

This is essentially the same issue, but in a more complex setting. It's 
reasonable to make this test (test_env_expansion_spaces) depend on Hush 
since it tests a feature that non-Hush shells presumably deliberately 
don't implement.