[U-Boot] [PATCH] test/py: hush_if_test: Add tests to cover octal/hex values

[Stephen Warren]

On 10/21/19 5:46 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, 21 Oct 2019 at 17:04, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>
>> On 10/21/19 4:53 PM, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On Tue, 15 Oct 2019 at 00:09, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On 11. 10. 19 17:53, Simon Glass wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On Fri, 11 Oct 2019 at 01:50, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>>>
>>>>>> On 10. 10. 19 19:06, Simon Glass wrote:
>>>>>>> Hi Michal,
>>>>>>>
>>>>>>> On Thu, 10 Oct 2019 at 05:44, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>>>>>
>>>>>>>> Extend test suite to cover also automatic octal/hex converstions which
>>>>>>>> haven't been implemented in past.
>>>>>>>>
>>>>>>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Depends on https://lists.denx.de/pipermail/u-boot/2019-September/383309.html
>>>>>>>>
>>>>>>>> There are of course other tests which we can run but not sure if make sense
>>>>>>>> to have there all combinations. The most interesting are mixed tests which
>>>>>>>> are failing before patch above is applied.
>>>>>>>> Definitely please let me know if you want to add any other test.
>>>>>>>> ---
>>>>>>>>    test/py/tests/test_hush_if_test.py | 27 +++++++++++++++++++++++++++
>>>>>>>>    1 file changed, 27 insertions(+)
>>>>>>>>
>>>>>>>
>>>>>>> I worry that these tests might be very slow since it requires a lot of
>>>>>>> interaction with U-Boot over a pipe. Is it possible to put them in C
>>>>>>> code instead, e.g. cmd_ut?
>>>>>>
>>>>>> I have of course running it on my HW and it is quite fast. It is just 16
>>>>>> more simple tests. And if this breaks gitlab/travis CI loops then we
>>>>>> have bigger problem.
>>>>>
>>>>> I mean running these tests on sandbox. The interactions with the
>>>>> sandbox command line are quite slow I think.
>>>>
>>>>
>>>> I am not sharing this concern.
>>>>
>>>> Before:
>>>> [u-boot]$ time ./test/py/test.py --bd sandbox -s -k hush >/dev/null
>>>>
>>>> real    0m2,403s
>>>> user    0m1,263s
>>>> sys     0m0,299s
>>>>
>>>> After
>>>> [u-boot]$ time ./test/py/test.py --bd sandbox -s -k hush >/dev/null
>>>>
>>>> real    0m2,864s
>>>> user    0m1,563s
>>>> sys     0m0,305s
>>>>
>>>> And if 0.4s on testing will cause issues somewhere else we have
>>>> different kind of problem.
>>>
>>> +Stephen Warren
>>>
>>> I originally mentioned this concern to Stephen we the test setup was
>>> created. At present even 'make qcheck' takes over a minute. Adding
>>> half a second to this every time we add a new test is not going to
>>> lead to a good place.
>>>
>>> Stephen made some improvements to speed things up, and suggested that
>>> the problem would not bear out. The alternative was presumably to
>>> build U-Boot into a Python module to avoid the comms overhead. But we
>>> didn't go that path.
>>>
>>> So I think we should only use Python when the tests cannot be written in C.
>>
>> I don't really see any concern with the addition of a couple extra
>> seconds of test. Clearly I'd rather see the test written in Python and
>> using external interfaces (i.e. the shell) where they test features
>> accessible through those interfaces, since that allows them to be
>> validated on all platforms, rather than only in sandbox. I feel that
> 
> But cmd_ut.c works fine on non-sandbox platforms. I'm asking that we
> do the same approach here.
> 
> We can use run_command() and run_command_list()

In that case, I'd agree it's fine to use that approach since presumably 
those functions run through the standard shell parsing code. But I still 
wouldn't want to prevent anyone from invoking stuff from test/py myself, 
even if you might:-)