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

[Michal Simek]

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.

Do you see the value to implement this?
If yes. Stephen is there an easy way to do it?
The reason why I ask is that values from that boardenv files are taken
via u_boot_console.config.env.get

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171206/af73ee1f/attachment.sig>