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

[Simon Glass]

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()


> sandbox is good for unit tests and providing host-based smoke tests, but
> nothing beats actually being able to test real platforms.

Agreed, but I think that is a separate issue.

Regards,
Simon