Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase bsim tests watchdog deadline - again #80916

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

aescolar
Copy link
Member

@aescolar aescolar commented Nov 5, 2024

Very much like it was done in #70750
Increase tests timeouts to ensure they do not trigger in CI.
A test timeout has been increased if the time it took in one recent CI run was > 1/3 of the timeout (or what it took in my own machine > 1/30 of the timeout)
If anyhow, I was increasing the timing I have increased it by quite a bit, so we are far from the 3x limit.
(Note the default timeout is 30 seconds)

Sits on top of

This PR is not urgent, as the urgent timeout increases should be in #80896, but all changes are trivial and harmless enough.

cvinayak and others added 5 commits November 5, 2024 09:53
Fix spurious ISO Sync Receiver stall due to uninitialised
value accessed due to regression introduced by
commit 64facee ("Bluetooth: controller: Stop Sync ISO
ticker when establishment fails").

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
These tests have been seen failing in CI due to the
real time execution timeout. Let's increase it so it does
not happen.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
Increase the EXECUTION_TIMEOUT (real time timeout at which the test
will be killed), so we have more margin for CI.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
Increase the EXECUTION_TIMEOUT (real time timeout at which the test
will be killed), so we have more margin for CI.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
Increase the EXECUTION_TIMEOUT (real time timeout at which the test
will be killed), so we have more margin for CI.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
Copy link
Collaborator

@Thalley Thalley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is our CI just getting slower, or is Zephyr/BSIM getting slower? :D

We discussed it earlier where I suggest to increase the default timeout. At this point a default of 300 seconds is starting to look better. We already have increased the timeout for more than a hundred tests, and this is increasing them even further.

@aescolar
Copy link
Member Author

aescolar commented Nov 5, 2024

Is our CI just getting slower, or is Zephyr/BSIM getting slower? :D

Don't know. One of the 3 tests that was timing out is newish, the other 2 got realencryption enabled recently (which adds a bit of overhead). Zephyr is not getting leaner. And CI nodes seem to be quite variable in performance. bsim itself, specially for the 52 should be quite unchanged for the last few months. So I'd blame bloat in your code :p

@Thalley
Copy link
Collaborator

Thalley commented Nov 5, 2024

So I'd blame bloat in your code :p

:(

Jokes aside, we are adding more and more testing and more criteria, which of course also increases the runtime, but shouldn't affect them this much. Would be interesting to have some timing benchmarks on some audio tests so we could track that

@aescolar
Copy link
Member Author

aescolar commented Nov 5, 2024

Jokes aside, we are adding more and more testing and more criteria, which of course also increases the runtime, but shouldn't affect them this much. Would be interesting to have some timing benchmarks on some audio tests so we could track that

It would be. Out of curiosity I just checked for tests/bsim/bluetooth/audio/test_scripts/bap_unicast_audio.sh, the server in 3.6.0 and as it is now in main. The difference is relatively small

main

Total inst: 753,523,067
Logger: ~12.5 M
HW encryption: ~2.7M (calc) + ~1.5M (library load)

3.6.0

Total inst: 714,650,574
Logger: ~6 M
HW encryption: ~0M

There is lots of other minor differences, but it is difficult to see a clear pattern, some things use more time, some less.

@aescolar
Copy link
Member Author

aescolar commented Nov 5, 2024

One thing though, we are still running in CI with coverage enabled, but I don't think we really do much with that data for the last couple of years. That takes a significant chunk of time.

@Thalley
Copy link
Collaborator

Thalley commented Nov 5, 2024

One thing though, we are still running in CI with coverage enabled, but I don't think we really do much with that data for the last couple of years. That takes a significant chunk of time.

I think there was some effort a while ago to actually publish this, so it would be part of https://app.codecov.io/gh/zephyrproject-rtos/zephyr or at least accessible which I don't think it really is anywhere (and not very visible). #56421

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

5 participants