From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Tue, 29 Oct 2019 19:48:26 -0600 Subject: [U-Boot] [PATCH] test/py: hush_if_test: Add tests to cover octal/hex values In-Reply-To: <45d26fe9-11fc-4d7e-4391-3d29b03f5a18@xilinx.com> References: <2861e9ee042ad93a9d36f551cd90ce7cbc6030aa.1570707876.git.michal.simek@xilinx.com> <54db2685-3437-eed8-73fa-63d95a59b3ea@xilinx.com> <2bc39daf-b1e2-0996-2d41-ca53de9fbb70@xilinx.com> <45d26fe9-11fc-4d7e-4391-3d29b03f5a18@xilinx.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Michal, On Thu, 24 Oct 2019 at 00:51, Michal Simek wrote: > > 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 > >> wrote: > >>> > >>> On 10/21/19 4:53 PM, Simon Glass wrote: > >>>> Hi Michal, > >>>> > >>>> On Tue, 15 Oct 2019 at 00:09, Michal Simek > >>>> wrote: > >>>>> > >>>>> Hi Simon, > >>>>> > >>>>> On 11. 10. 19 17:53, Simon Glass wrote: > >>>>>> Hi Michal, > >>>>>> > >>>>>> On Fri, 11 Oct 2019 at 01:50, Michal Simek > >>>>>> wrote: > >>>>>>> > >>>>>>> On 10. 10. 19 19:06, Simon Glass wrote: > >>>>>>>> Hi Michal, > >>>>>>>> > >>>>>>>> On Thu, 10 Oct 2019 at 05:44, Michal Simek > >>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> Extend test suite to cover also automatic octal/hex > >>>>>>>>> converstions which > >>>>>>>>> haven't been implemented in past. > >>>>>>>>> > >>>>>>>>> Signed-off-by: Michal Simek > >>>>>>>>> --- > >>>>>>>>> > >>>>>>>>> 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. Yes please add a TODO, but I don't want to add more tests here for someone to convert! I just timed it - 2.5 seconds to run the hush tests, 22 seconds to run the whole test suite - so these tests take 10% of the time! As you mention they run instantly when done in C. I run these tests a lot - the total time for 'make qcheck' is about 75s for me and I run it a lot, so am actively looking to reduce this down. We may need to rethink the pytest stuff a bit, perhaps building U-Boot as a Python library? But in any case, please add a C test. It is really easy to do, e.g. like cmd_ut.c Regards, Simon