On 22. 10. 19 17:54, Stephen Warren wrote: > On 10/21/19 5:46 PM, Simon Glass wrote: >> Hi Stephen, >> >> On Mon, 21 Oct 2019 at 17:04, Stephen Warren <swar...@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.si...@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.si...@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.si...@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.si...@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:-)
Ok. Would be good to get any outcome of this discussion regarding this one patch. Right now this is what I have and I have tested it. The patch which adds this functionality is already in the tree. I expect that the same logic can be applied to all tests in this file that's why I think that would be the best to add TODO to this file to let everybody know what should happen with these tests and how it should be converted. Thanks, Michal _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
