From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Tue, 22 Oct 2019 09:54:23 -0600 Subject: [U-Boot] [PATCH] test/py: hush_if_test: Add tests to cover octal/hex values In-Reply-To: References: <2861e9ee042ad93a9d36f551cd90ce7cbc6030aa.1570707876.git.michal.simek@xilinx.com> <54db2685-3437-eed8-73fa-63d95a59b3ea@xilinx.com> <2bc39daf-b1e2-0996-2d41-ca53de9fbb70@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 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:-)