[PATCH] selftests: rtc: Increase test timeout so that all tests run

[PATCH] selftests: rtc: Increase test timeout so that all tests run

[Nícolas F. R. A. Prado]

The timeout setting for the rtc kselftest is currently 90 seconds.
However, two of the tests set alarms, which take one minute to complete
each. So the timeout should be at least 120. Set it to 180, so that all
tests are able to complete and still have some slack.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

This issue was discovered as part of adding the rtc kselftest to run on KernelCI
for the rk3399-gru-kevin device, which uses rtc-cros-ec as the RTC driver.

The output log with the current timeout is shown in [1]. As can be seen, the
whole test times out before the alarm_wkalm_set_minute test has had a chance to
complete:

	# #  RUN           rtc.alarm_wkalm_set_minute ...
	# # rtctest.c:294:alarm_wkalm_set_minute:Alarm time now set to 11/01/2022 23:03:00.
	#
	not ok 1 selftests: rtc: rtctest # TIMEOUT 90 seconds

With the increased timeout, as shown in [2], the alarm_wkalm_set_minute test
does complete its run:

	# #  RUN           rtc.alarm_wkalm_set_minute ...
	# # rtctest.c:294:alarm_wkalm_set_minute:Alarm time now set to 12/01/2022 15:54:00.
	# #            OK  rtc.alarm_wkalm_set_minute
	# ok 7 rtc.alarm_wkalm_set_minute
	# # FAILED: 6 / 7 tests passed.

The fact that the alarm_alm_set_minute test times out on its own is probably an
issue with the rtc-cros-ec driver. Still, since the tests are independent, all
of them should be able to run regardless of how long each one takes (so,
assuming the worst case scenario).

[1] https://lava.collabora.co.uk/scheduler/job/5409783
[2] https://lava.collabora.co.uk/scheduler/job/5415176

 tools/testing/selftests/rtc/settings | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/rtc/settings b/tools/testing/selftests/rtc/settings
index ba4d85f74cd6..a953c96aa16e 100644
--- a/tools/testing/selftests/rtc/settings
+++ b/tools/testing/selftests/rtc/settings
@@ -1 +1 @@
-timeout=90
+timeout=180
-- 
2.34.1

Re: [PATCH] selftests: rtc: Increase test timeout so that all tests run

[Alexandre Belloni]

On 12/01/2022 11:53:59-0500, Nícolas F. R. A. Prado wrote:
> The timeout setting for the rtc kselftest is currently 90 seconds.
> However, two of the tests set alarms, which take one minute to complete
> each. So the timeout should be at least 120. Set it to 180, so that all
> tests are able to complete and still have some slack.

Hum, I don't get the logic from that, a test takes up to 60 seconds and
the timeout is 90s per test so we already have plenty of slack there.

> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> 
> This issue was discovered as part of adding the rtc kselftest to run on KernelCI
> for the rk3399-gru-kevin device, which uses rtc-cros-ec as the RTC driver.
> 
> The output log with the current timeout is shown in [1]. As can be seen, the
> whole test times out before the alarm_wkalm_set_minute test has had a chance to
> complete:
> 
> 	# #  RUN           rtc.alarm_wkalm_set_minute ...
> 	# # rtctest.c:294:alarm_wkalm_set_minute:Alarm time now set to 11/01/2022 23:03:00.
> 	#
> 	not ok 1 selftests: rtc: rtctest # TIMEOUT 90 seconds
> 
> With the increased timeout, as shown in [2], the alarm_wkalm_set_minute test
> does complete its run:
> 
> 	# #  RUN           rtc.alarm_wkalm_set_minute ...
> 	# # rtctest.c:294:alarm_wkalm_set_minute:Alarm time now set to 12/01/2022 15:54:00.
> 	# #            OK  rtc.alarm_wkalm_set_minute
> 	# ok 7 rtc.alarm_wkalm_set_minute
> 	# # FAILED: 6 / 7 tests passed.
> 
> The fact that the alarm_alm_set_minute test times out on its own is probably an
> issue with the rtc-cros-ec driver. Still, since the tests are independent, all
> of them should be able to run regardless of how long each one takes (so,
> assuming the worst case scenario).
> 
> [1] https://lava.collabora.co.uk/scheduler/job/5409783
> [2] https://lava.collabora.co.uk/scheduler/job/5415176
> 
>  tools/testing/selftests/rtc/settings | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/rtc/settings b/tools/testing/selftests/rtc/settings
> index ba4d85f74cd6..a953c96aa16e 100644
> --- a/tools/testing/selftests/rtc/settings
> +++ b/tools/testing/selftests/rtc/settings
> @@ -1 +1 @@
> -timeout=90
> +timeout=180
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] selftests: rtc: Increase test timeout so that all tests run

[Nícolas F. R. A. Prado]

On Wed, Jan 12, 2022 at 05:57:25PM +0100, Alexandre Belloni wrote:
> On 12/01/2022 11:53:59-0500, Nícolas F. R. A. Prado wrote:
> > The timeout setting for the rtc kselftest is currently 90 seconds.
> > However, two of the tests set alarms, which take one minute to complete
> > each. So the timeout should be at least 120. Set it to 180, so that all
> > tests are able to complete and still have some slack.
> 
> Hum, I don't get the logic from that, a test takes up to 60 seconds and
> the timeout is 90s per test so we already have plenty of slack there.

Actually I did re-test locally and it is an issue since the 90s timeout is not
per-test, but to the whole rtc test suite. Also it requires running the test
through the selftest runner, and since the alarm is set to the next top of the
minute, you might get lucky and it might all take less than 90 seconds.

However none of this was clear in my original commit message, so I'll send a v2
with a better explanation.

Thanks,
Nícolas