[U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets

[Tom Rini]

On Wed, Dec 06, 2017 at 10:53:08AM +0100, Michal Simek wrote:
> On 5.12.2017 16:13, Tom Rini wrote:
> > On Tue, Dec 05, 2017 at 01:10:47PM +0100, Michal Simek wrote:
> >> On 4.12.2017 18:14, Stephen Warren wrote:
> >>> On 12/04/2017 08:30 AM, Tom Rini wrote:
> >>>> On Mon, Dec 04, 2017 at 03:21:04PM +0100, Michal Simek wrote:
> >>>>> On 4.12.2017 15:03, Tom Rini wrote:
> >>>>>> On Mon, Dec 04, 2017 at 02:55:45PM +0100, Michal Simek wrote:
> >>>>>>> On 1.12.2017 23:44, Tom Rini wrote:
> >>>>>>>> On Fri, Dec 01, 2017 at 10:07:54AM -0700, Stephen Warren wrote:
> >>>>>>>>> On 12/01/2017 08:19 AM, Michal Simek wrote:
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> On 1.12.2017 16:06, Heinrich Schuchardt wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On 12/01/2017 03:46 PM, Michal Simek wrote:
> >>>>>>>>>>>> Qemu for arm32/arm64 has a problem with time setup.
> >>>>>>>>>>>
> >>>>>>>>>>> Wouldn't it be preferable to fix the root cause?
> >>>>>>>>>>
> >>>>>>>>>> Definitely that would be the best and IIRC I have tried to
> >>>>>>>>>> convince our
> >>>>>>>>>> qemu guy to do that but they have never done that.
> >>>>>>>>>
> >>>>>>>>> What is the exact failure condition? Is it simply that the test
> >>>>>>>>> is still
> >>>>>>>>> slightly too strict about which delays it accepts, or is sleep
> >>>>>>>>> outright
> >>>>>>>>> broken?
> >>>>>>>>>
> >>>>>>>>> You can use command-line option -k to avoid some tests. For
> >>>>>>>>> example "-k not
> >>>>>>>>> sleep". That way, we don't have to hard-code the dependency into
> >>>>>>>>> the test
> >>>>>>>>> source. Depending on the root cause (issue in U-Boot, or issue in
> >>>>>>>>> just your
> >>>>>>>>> local version of qemu, or something that will never work) this
> >>>>>>>>> might be
> >>>>>>>>> better?
> >>>>>>>>
> >>>>>>>> Even with the most recent relaxing of the sleep test requirements,
> >>>>>>>> I can
> >>>>>>>> still (depending on overall system load) have 'sleep' take too
> >>>>>>>> long, on
> >>>>>>>> QEMU.  I think it might have been half a second slow, but I don't
> >>>>>>>> have
> >>>>>>>> the log handy anymore.  Both locally and in travis we -k not sleep
> >>>>>>>> all
> >>>>>>>> of the qemu instances.
> >>>>>>>
> >>>>>>> ok. By locally do you mean just using -k not sleep?
> >>>>>>
> >>>>>> Yes, I have that in my CI scripts and similar.
> >>>>>
> >>>>> Wouldn't be easier to keep this in uboot-test-hooks repo with other
> >>>>> target setting?
> >>>>
> >>>> Or do as you did did and mark the tests as not allowed for qemu, yes.
> >>>>
> >>>>> What we are trying to do is that our testing group will run these tests
> >>>>> for me that's why it is just easier for me to change local
> >>>>> uboot-test-hooks repo instead of communicate with them what -k not XXX
> >>>>> parameters to add to certain scripts.
> >>>>>
> >>>>> It means in loop they will just run all tests on qemu, local targets and
> >>>>> in boardfarm. It is probably not big deal to tell them to add -k not
> >>>>> sleep for all qemu runs but I know that for some i2c testing qemu
> >>>>> doesn't emulate these devices that's why these tests fails. And the
> >>>>> amount of tests which we shouldn't run on qemu will probably grow.
> >>>>
> >>>> Well, I'm still open to possibly tweaking the allowed variance in the
> >>>> sleep test.  OTOH, if we just say "no QEMU" here, we can then go back to
> >>>> "sleep should be pretty darn accurate on HW" for the test too, and
> >>>> perhaps that's best.
> >>>
> >>> The fundamental problem of "over-sleeping" due to host system load/..
> >>> exists with all boards. There's nothing specific to qemu here except
> >>> that running U-Boot on qemu on the host rather than on separate HW might
> >>> more easily trigger the "high load on the host" condition; I see the
> >>> issue now and then and manually retry that test, although that is a bit
> >>> annoying.
> >>>
> >>> The original test was mostly intended to make sure that e U-Boot clock
> >>> didn't run at a significantly different rate to the host, since I had
> >>> seen that issue during development of some board support or as a
> >>> regression sometime. Perhaps the definition of "significantly different"
> >>> should be more like "1/2 rate or twice rate or more" rather than "off by
> >>> a small fraction of a second". That might avoid so many false positives.
> >>
> >> We had this issue with silicon v1 and having accurate sleep measuring is
> >> definitely good thing to have (Probably make sense to enable margin
> >> setup via variable anyway).
> >>
> >> But still I would extend this to more wider discussion how to disable
> >> just one particular test case which is verified that it is broken on
> >> certain target/target configuration.
> >> Using -k not XXX option is possible but as I said before it is not ideal
> >> to keep track of these problematic tests in two locations and share this
> >> between two teams.
> >>
> >> Better would be to add to u_boot_boardenv...py file line like this
> >> skip_test_sleep = True
> >>
> >> Which would be parsed and test won't run for specific board/configuration.
> >> The same logic can be generic that user can add for example
> >> skip_test_net_dhcp = True
> >> to skip dhcp test for whatever reason.
> >>
> >> Then for travis-ci we can just put these lines to py/travis-ci/.
> >>
> >> What do you think?
> > 
> > Ah, good idea!  We have a few cases like this already, so how about
> > env__sleep_accurate, default it to True and let the board files set it
> > to false, and have test_sleep check for and use that?
> 
> ok. this is about that variable for sleep not about generic skipping
> testcases which are failing.

Right.  I'm not sure we need to get too complicated on "skip these
tests" logic.  But I think we have enough technical information now to
say it's reasonable to skip sleep in some cases.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171206/9e843c3f/attachment.sig>