All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
@ 2017-12-01 14:46 Michal Simek
  2017-12-01 15:06 ` Heinrich Schuchardt
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2017-12-01 14:46 UTC (permalink / raw)
  To: u-boot

Qemu for arm32/arm64 has a problem with time setup.
That's why sleep test is failing. Add boardidentity marker to remove
specific boards from running that test.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

If you want to add this checking in one patch and then put it to one
test then it is fine for me.

---
 test/py/conftest.py         | 28 ++++++++++++++++++++++++++++
 test/py/pytest.ini          |  1 +
 test/py/tests/test_sleep.py |  1 +
 3 files changed, 30 insertions(+)

diff --git a/test/py/conftest.py b/test/py/conftest.py
index 6e66a48c15fd..1812ff340e6a 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -436,6 +436,33 @@ def setup_boardspec(item):
     if required_boards and ubconfig.board_type not in required_boards:
         pytest.skip('board "%s" not supported' % ubconfig.board_type)
 
+def setup_boardidentity(item):
+    """Process any 'boardidentity' marker for a test.
+
+    Such a marker lists the set of board identity that a test does/doesn't
+    support. If tests are being executed on an unsupported board, the test is
+    marked to be skipped.
+
+    Args:
+        item: The pytest test item.
+
+    Returns:
+        Nothing.
+    """
+    mark = item.get_marker('boardidentity')
+    if not mark:
+        return
+    required_boards = []
+    for board in mark.args:
+        if board.startswith('!'):
+            if ubconfig.board_identity == board[1:]:
+                pytest.skip('board identity not supported')
+                return
+        else:
+            required_boards.append(board)
+    if required_boards and ubconfig.board_identity not in required_boards:
+        pytest.skip('board identity not supported')
+
 def setup_buildconfigspec(item):
     """Process any 'buildconfigspec' marker for a test.
 
@@ -503,6 +530,7 @@ def pytest_runtest_setup(item):
 
     start_test_section(item)
     setup_boardspec(item)
+    setup_boardidentity(item)
     setup_buildconfigspec(item)
     setup_requiredtool(item)
 
diff --git a/test/py/pytest.ini b/test/py/pytest.ini
index 67e514f42058..9d64671814de 100644
--- a/test/py/pytest.ini
+++ b/test/py/pytest.ini
@@ -8,4 +8,5 @@
 [pytest]
 markers =
     boardspec: U-Boot: Describes the set of boards a test can/can't run on.
+    boardidentity: U-Boot: Describes the board identity a test can/can't run on.
     buildconfigspec: U-Boot: Describes Kconfig/config-header constraints.
diff --git a/test/py/tests/test_sleep.py b/test/py/tests/test_sleep.py
index 64e057132622..02a8a85b0e22 100644
--- a/test/py/tests/test_sleep.py
+++ b/test/py/tests/test_sleep.py
@@ -5,6 +5,7 @@
 import pytest
 import time
 
+ at pytest.mark.boardidentity("!qemu")
 def test_sleep(u_boot_console):
     """Test the sleep command, and validate that it sleeps for approximately
     the correct amount of time."""
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-01 14:46 [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets Michal Simek
@ 2017-12-01 15:06 ` Heinrich Schuchardt
  2017-12-01 15:19   ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-12-01 15:06 UTC (permalink / raw)
  To: u-boot



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?

> That's why sleep test is failing. Add boardidentity marker to remove
> specific boards from running that test.

Isn't this what 'boardspec' is used for?

test/py/pytest.ini:
boardspec: U-Boot: Describes the set of boards a test can/can't run on.

> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> If you want to add this checking in one patch and then put it to one
> test then it is fine for me.
> 
> ---
>   test/py/conftest.py         | 28 ++++++++++++++++++++++++++++
>   test/py/pytest.ini          |  1 +
>   test/py/tests/test_sleep.py |  1 +
>   3 files changed, 30 insertions(+)
> 
> diff --git a/test/py/conftest.py b/test/py/conftest.py
> index 6e66a48c15fd..1812ff340e6a 100644
> --- a/test/py/conftest.py
> +++ b/test/py/conftest.py
> @@ -436,6 +436,33 @@ def setup_boardspec(item):
>       if required_boards and ubconfig.board_type not in required_boards:
>           pytest.skip('board "%s" not supported' % ubconfig.board_type)
>   
> +def setup_boardidentity(item):
> +    """Process any 'boardidentity' marker for a test.
> +
> +    Such a marker lists the set of board identity that a test does/doesn't
> +    support. If tests are being executed on an unsupported board, the test is
> +    marked to be skipped.
> +
> +    Args:
> +        item: The pytest test item.
> +
> +    Returns:
> +        Nothing.
> +    """
> +    mark = item.get_marker('boardidentity')
> +    if not mark:
> +        return
> +    required_boards = []
> +    for board in mark.args:
> +        if board.startswith('!'):
> +            if ubconfig.board_identity == board[1:]:
> +                pytest.skip('board identity not supported')
> +                return
> +        else:
> +            required_boards.append(board)
> +    if required_boards and ubconfig.board_identity not in required_boards:
> +        pytest.skip('board identity not supported')
> +
>   def setup_buildconfigspec(item):
>       """Process any 'buildconfigspec' marker for a test.
>   
> @@ -503,6 +530,7 @@ def pytest_runtest_setup(item):
>   
>       start_test_section(item)
>       setup_boardspec(item)
> +    setup_boardidentity(item)
>       setup_buildconfigspec(item)
>       setup_requiredtool(item)
>   
> diff --git a/test/py/pytest.ini b/test/py/pytest.ini
> index 67e514f42058..9d64671814de 100644
> --- a/test/py/pytest.ini
> +++ b/test/py/pytest.ini
> @@ -8,4 +8,5 @@
>   [pytest]
>   markers =
>       boardspec: U-Boot: Describes the set of boards a test can/can't run on.
> +    boardidentity: U-Boot: Describes the board identity a test can/can't run on.
>       buildconfigspec: U-Boot: Describes Kconfig/config-header constraints.
> diff --git a/test/py/tests/test_sleep.py b/test/py/tests/test_sleep.py
> index 64e057132622..02a8a85b0e22 100644
> --- a/test/py/tests/test_sleep.py
> +++ b/test/py/tests/test_sleep.py
> @@ -5,6 +5,7 @@
>   import pytest
>   import time
>   
> + at pytest.mark.boardidentity("!qemu")

According to your commit message you don't want to exclude qemu-x86 here.

Best regards

Heinrich Schuchardt

>   def test_sleep(u_boot_console):
>       """Test the sleep command, and validate that it sleeps for approximately
>       the correct amount of time."""
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-01 15:06 ` Heinrich Schuchardt
@ 2017-12-01 15:19   ` Michal Simek
  2017-12-01 17:07     ` Stephen Warren
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2017-12-01 15:19 UTC (permalink / raw)
  To: u-boot

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.

> 
>> That's why sleep test is failing. Add boardidentity marker to remove
>> specific boards from running that test.
> 
> Isn't this what 'boardspec' is used for?

There are two things here.

./test/py/test.py --bd zynq_zc706 --build  --board-identity zc706  -s

boardspec is zynq_z706 -> u-boot config
and I use board identity (zc706) as actual HW I have here

And I use this
./test/py/test.py --bd zynq_zc706 --build  --board-identity qemu -s
which means run the same config for zc706 but on qemu

etc.

> 
> test/py/pytest.ini:
> boardspec: U-Boot: Describes the set of boards a test can/can't run on.

as above. This file should be also update if this "feature" is fine.


>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> If you want to add this checking in one patch and then put it to one
>> test then it is fine for me.
>>
>> ---
>>   test/py/conftest.py         | 28 ++++++++++++++++++++++++++++
>>   test/py/pytest.ini          |  1 +
>>   test/py/tests/test_sleep.py |  1 +
>>   3 files changed, 30 insertions(+)
>>
>> diff --git a/test/py/conftest.py b/test/py/conftest.py
>> index 6e66a48c15fd..1812ff340e6a 100644
>> --- a/test/py/conftest.py
>> +++ b/test/py/conftest.py
>> @@ -436,6 +436,33 @@ def setup_boardspec(item):
>>       if required_boards and ubconfig.board_type not in required_boards:
>>           pytest.skip('board "%s" not supported' % ubconfig.board_type)
>>   +def setup_boardidentity(item):
>> +    """Process any 'boardidentity' marker for a test.
>> +
>> +    Such a marker lists the set of board identity that a test
>> does/doesn't
>> +    support. If tests are being executed on an unsupported board, the
>> test is
>> +    marked to be skipped.
>> +
>> +    Args:
>> +        item: The pytest test item.
>> +
>> +    Returns:
>> +        Nothing.
>> +    """
>> +    mark = item.get_marker('boardidentity')
>> +    if not mark:
>> +        return
>> +    required_boards = []
>> +    for board in mark.args:
>> +        if board.startswith('!'):
>> +            if ubconfig.board_identity == board[1:]:
>> +                pytest.skip('board identity not supported')
>> +                return
>> +        else:
>> +            required_boards.append(board)
>> +    if required_boards and ubconfig.board_identity not in
>> required_boards:
>> +        pytest.skip('board identity not supported')
>> +
>>   def setup_buildconfigspec(item):
>>       """Process any 'buildconfigspec' marker for a test.
>>   @@ -503,6 +530,7 @@ def pytest_runtest_setup(item):
>>         start_test_section(item)
>>       setup_boardspec(item)
>> +    setup_boardidentity(item)
>>       setup_buildconfigspec(item)
>>       setup_requiredtool(item)
>>   diff --git a/test/py/pytest.ini b/test/py/pytest.ini
>> index 67e514f42058..9d64671814de 100644
>> --- a/test/py/pytest.ini
>> +++ b/test/py/pytest.ini
>> @@ -8,4 +8,5 @@
>>   [pytest]
>>   markers =
>>       boardspec: U-Boot: Describes the set of boards a test can/can't
>> run on.
>> +    boardidentity: U-Boot: Describes the board identity a test
>> can/can't run on.
>>       buildconfigspec: U-Boot: Describes Kconfig/config-header
>> constraints.
>> diff --git a/test/py/tests/test_sleep.py b/test/py/tests/test_sleep.py
>> index 64e057132622..02a8a85b0e22 100644
>> --- a/test/py/tests/test_sleep.py
>> +++ b/test/py/tests/test_sleep.py
>> @@ -5,6 +5,7 @@
>>   import pytest
>>   import time
>>   + at pytest.mark.boardidentity("!qemu")
> 
> According to your commit message you don't want to exclude qemu-x86 here.

It is really just qemu board identity not qemu target.

The issue with this patch is that everybody can select whatever they
want to for board identification.

Maybe it would be much simpler to create variable and check it in that
test and skip that test.

Anyway this is what I have in the tree and it is time to find proper
solution that's why RFC.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-01 15:19   ` Michal Simek
@ 2017-12-01 17:07     ` Stephen Warren
  2017-12-01 22:44       ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Warren @ 2017-12-01 17:07 UTC (permalink / raw)
  To: u-boot

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?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-01 17:07     ` Stephen Warren
@ 2017-12-01 22:44       ` Tom Rini
  2017-12-04 13:55         ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2017-12-01 22:44 UTC (permalink / raw)
  To: u-boot

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.

-- 
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/20171201/fe0c2bed/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-01 22:44       ` Tom Rini
@ 2017-12-04 13:55         ` Michal Simek
  2017-12-04 14:03           ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2017-12-04 13:55 UTC (permalink / raw)
  To: u-boot

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?
Or is there any other way how to put this to
uboot-test-hooks/py/<hostname>/*.py

We are talking about one test here but there could be different issues
with different tests in connection to qemu.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-04 13:55         ` Michal Simek
@ 2017-12-04 14:03           ` Tom Rini
  2017-12-04 14:21             ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2017-12-04 14:03 UTC (permalink / raw)
  To: u-boot

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.

-- 
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/20171204/2cab3049/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-04 14:03           ` Tom Rini
@ 2017-12-04 14:21             ` Michal Simek
  2017-12-04 15:30               ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2017-12-04 14:21 UTC (permalink / raw)
  To: u-boot

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?
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.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-04 14:21             ` Michal Simek
@ 2017-12-04 15:30               ` Tom Rini
  2017-12-04 17:14                 ` Stephen Warren
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2017-12-04 15:30 UTC (permalink / raw)
  To: u-boot

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.

-- 
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/20171204/fd43c499/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-04 15:30               ` Tom Rini
@ 2017-12-04 17:14                 ` Stephen Warren
  2017-12-04 23:21                   ` Tom Rini
  2017-12-05 12:10                   ` Michal Simek
  0 siblings, 2 replies; 20+ messages in thread
From: Stephen Warren @ 2017-12-04 17:14 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-04 17:14                 ` Stephen Warren
@ 2017-12-04 23:21                   ` Tom Rini
  2017-12-05 18:20                     ` Stephen Warren
  2017-12-05 12:10                   ` Michal Simek
  1 sibling, 1 reply; 20+ messages in thread
From: Tom Rini @ 2017-12-04 23:21 UTC (permalink / raw)
  To: u-boot

On Mon, Dec 04, 2017 at 10:14:06AM -0700, 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.

I've pushed this up to 10 seconds and 0.5s worth of overrun and on
qemu-mips here I see a 13.2s sleep.  That's pretty close to 1/3rd fast
and to me a wrong-clocking value, yes?

-- 
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/20171204/1c00f32f/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-04 17:14                 ` Stephen Warren
  2017-12-04 23:21                   ` Tom Rini
@ 2017-12-05 12:10                   ` Michal Simek
  2017-12-05 15:13                     ` Tom Rini
  1 sibling, 1 reply; 20+ messages in thread
From: Michal Simek @ 2017-12-05 12:10 UTC (permalink / raw)
  To: u-boot

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?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-05 12:10                   ` Michal Simek
@ 2017-12-05 15:13                     ` Tom Rini
  2017-12-06  9:53                       ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2017-12-05 15:13 UTC (permalink / raw)
  To: u-boot

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?

-- 
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/20171205/9775d514/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-04 23:21                   ` Tom Rini
@ 2017-12-05 18:20                     ` Stephen Warren
  2017-12-05 18:38                       ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Warren @ 2017-12-05 18:20 UTC (permalink / raw)
  To: u-boot

On 12/04/2017 04:21 PM, Tom Rini wrote:
> On Mon, Dec 04, 2017 at 10:14:06AM -0700, 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.
> 
> I've pushed this up to 10 seconds and 0.5s worth of overrun and on
> qemu-mips here I see a 13.2s sleep.  That's pretty close to 1/3rd fast
> and to me a wrong-clocking value, yes?

For me the qemu-x86 build of mid-Nov commit of U-Boot running under the 
same qemu version that U-Boot's Travis CI builds use, "sleep 10" takes 
about 10.5 seconds (including my reaction time), so ~13.2 does sound 
like it's probably a bug. Or maybe qemu just isn't fast enough in its 
emulation to keep up with real-time? I'd hope not for something simple 
like this, assuming you're using a recent CPU, but maybe.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-05 18:20                     ` Stephen Warren
@ 2017-12-05 18:38                       ` Tom Rini
  2017-12-05 20:46                         ` Daniel Schwierzeck
  2017-12-06  9:49                         ` Michal Simek
  0 siblings, 2 replies; 20+ messages in thread
From: Tom Rini @ 2017-12-05 18:38 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 05, 2017 at 11:20:57AM -0700, Stephen Warren wrote:
> On 12/04/2017 04:21 PM, Tom Rini wrote:
> >On Mon, Dec 04, 2017 at 10:14:06AM -0700, 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.
> >
> >I've pushed this up to 10 seconds and 0.5s worth of overrun and on
> >qemu-mips here I see a 13.2s sleep.  That's pretty close to 1/3rd fast
> >and to me a wrong-clocking value, yes?
> 
> For me the qemu-x86 build of mid-Nov commit of U-Boot running under the same
> qemu version that U-Boot's Travis CI builds use, "sleep 10" takes about 10.5
> seconds (including my reaction time), so ~13.2 does sound like it's probably
> a bug. Or maybe qemu just isn't fast enough in its emulation to keep up with
> real-time? I'd hope not for something simple like this, assuming you're
> using a recent CPU, but maybe.

Yeah, I can do x86, ARM and PowerPC but it fails on MIPS.  And my build
box isn't super new but an 8core/16thread E5-2670 should be good enough
:)

Hey Daniel, do you have test.py running on real MIPS hardware?  Thanks!

-- 
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/20171205/65228006/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-05 18:38                       ` Tom Rini
@ 2017-12-05 20:46                         ` Daniel Schwierzeck
  2017-12-06  9:49                         ` Michal Simek
  1 sibling, 0 replies; 20+ messages in thread
From: Daniel Schwierzeck @ 2017-12-05 20:46 UTC (permalink / raw)
  To: u-boot



On 05.12.2017 19:38, Tom Rini wrote:
> On Tue, Dec 05, 2017 at 11:20:57AM -0700, Stephen Warren wrote:
>> On 12/04/2017 04:21 PM, Tom Rini wrote:
>>> On Mon, Dec 04, 2017 at 10:14:06AM -0700, 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.
>>>
>>> I've pushed this up to 10 seconds and 0.5s worth of overrun and on
>>> qemu-mips here I see a 13.2s sleep.  That's pretty close to 1/3rd fast
>>> and to me a wrong-clocking value, yes?
>>
>> For me the qemu-x86 build of mid-Nov commit of U-Boot running under the same
>> qemu version that U-Boot's Travis CI builds use, "sleep 10" takes about 10.5
>> seconds (including my reaction time), so ~13.2 does sound like it's probably
>> a bug. Or maybe qemu just isn't fast enough in its emulation to keep up with
>> real-time? I'd hope not for something simple like this, assuming you're
>> using a recent CPU, but maybe.
> 
> Yeah, I can do x86, ARM and PowerPC but it fails on MIPS.  And my build
> box isn't super new but an 8core/16thread E5-2670 should be good enough
> :)

the problem with MIPS is that the timer frequency is hard-coded with
config option CONFIG_SYS_MIPS_TIMER_FREQ in all older boards and the
boards supported by Qemu (qemu_mips, malta, boston). The reason is that
the timer frequency can't be derived from some hardware register. This
is only possible with modern MIPS SoCs, where one can derive the timer
frequency directly (or indirectly via the CPU frequency) from a PLL.

Also I can't see if or how the relevant Qemu timers for MIPS
(mips_gictimer.c, i8254.c) are throttled to a deterministic value.
Therefore I assume the timers are running as fast as possible. Thus
there always will be a discrepancy between the hard-coded U-Boot timer
frequency and the Qemu timer frequency dependent on the host system
where Qemu runs. Due to this it doesn't make sense to enable the sleep
test on MIPS.

I can think of following options to fix this:

1) U-Boot gets the possibilty to read the actual timer frequency somehow
from Qemu

2) estimate the timer frequency by reading the CPU counter register at
different times. But than we need some reference time or emulated RTC.
At least for the Malta board the kernel does this.

3) the Qemu timer frequency can be throttled to a deterministic value
(maybe configurable via Qemu command line argument).

4) keep the sleep test disabled

Or does someone know if and how the QEMU_CLOCK_VIRTUAL frequency can be
configured?

> 
> Hey Daniel, do you have test.py running on real MIPS hardware?  Thanks!
> 

unfortunately not. I don't have any of the boards supported in mainline.
And the boards I have aren't in mainline yet ;)

-- 
- Daniel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-05 18:38                       ` Tom Rini
  2017-12-05 20:46                         ` Daniel Schwierzeck
@ 2017-12-06  9:49                         ` Michal Simek
  1 sibling, 0 replies; 20+ messages in thread
From: Michal Simek @ 2017-12-06  9:49 UTC (permalink / raw)
  To: u-boot

On 5.12.2017 19:38, Tom Rini wrote:
> On Tue, Dec 05, 2017 at 11:20:57AM -0700, Stephen Warren wrote:
>> On 12/04/2017 04:21 PM, Tom Rini wrote:
>>> On Mon, Dec 04, 2017 at 10:14:06AM -0700, 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.
>>>
>>> I've pushed this up to 10 seconds and 0.5s worth of overrun and on
>>> qemu-mips here I see a 13.2s sleep.  That's pretty close to 1/3rd fast
>>> and to me a wrong-clocking value, yes?
>>
>> For me the qemu-x86 build of mid-Nov commit of U-Boot running under the same
>> qemu version that U-Boot's Travis CI builds use, "sleep 10" takes about 10.5
>> seconds (including my reaction time), so ~13.2 does sound like it's probably
>> a bug. Or maybe qemu just isn't fast enough in its emulation to keep up with
>> real-time? I'd hope not for something simple like this, assuming you're
>> using a recent CPU, but maybe.
> 
> Yeah, I can do x86, ARM and PowerPC but it fails on MIPS.  And my build
> box isn't super new but an 8core/16thread E5-2670 should be good enough
> :)

I should spend some time to add also microblaze. :-)

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/e7af911e/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-05 15:13                     ` Tom Rini
@ 2017-12-06  9:53                       ` Michal Simek
  2017-12-06 12:17                         ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2017-12-06  9:53 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-06  9:53                       ` Michal Simek
@ 2017-12-06 12:17                         ` Tom Rini
  2017-12-08 13:08                           ` Michal Simek
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2017-12-06 12:17 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets
  2017-12-06 12:17                         ` Tom Rini
@ 2017-12-08 13:08                           ` Michal Simek
  0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2017-12-08 13:08 UTC (permalink / raw)
  To: u-boot

On 6.12.2017 13:17, Tom Rini wrote:
> 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.
> 

Ok. Patch send please review.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-12-08 13:08 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01 14:46 [U-Boot] [RFC PATCH] test: py: Disable sleep test for qemu targets Michal Simek
2017-12-01 15:06 ` Heinrich Schuchardt
2017-12-01 15:19   ` Michal Simek
2017-12-01 17:07     ` Stephen Warren
2017-12-01 22:44       ` Tom Rini
2017-12-04 13:55         ` Michal Simek
2017-12-04 14:03           ` Tom Rini
2017-12-04 14:21             ` Michal Simek
2017-12-04 15:30               ` Tom Rini
2017-12-04 17:14                 ` Stephen Warren
2017-12-04 23:21                   ` Tom Rini
2017-12-05 18:20                     ` Stephen Warren
2017-12-05 18:38                       ` Tom Rini
2017-12-05 20:46                         ` Daniel Schwierzeck
2017-12-06  9:49                         ` Michal Simek
2017-12-05 12:10                   ` Michal Simek
2017-12-05 15:13                     ` Tom Rini
2017-12-06  9:53                       ` Michal Simek
2017-12-06 12:17                         ` Tom Rini
2017-12-08 13:08                           ` Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.