Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FIX: macos: Add update capability to interval/singleshot timer properties#29018
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
9a99838 toe60da20Compare| # GTK4 on macos runners produces about 3x as many calls as expected | ||
| # It works locally and on Linux though, so only skip when running on CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is really annoying to wrestle with CI sometimes. I have no idea why the macos CI runners would have ~60 calls, but my local version and the linux runners get the correct ~20 calls.
If anyone has better ideas for how to test these timers let me know too. Each new test/"pause" we add here adds2 seconds * nbackends / njobs to the test runs and it looks like this test is currently one of the longest running in the suite at around ~10s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Didplt.pause not run for the expectedpause_time, perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That is a good thought! That led me to a few refactors here which I pushed up as separate commits.
- I removed
plt.pause()and went directly tostart_event_loop()now in case there was additional overhead in any of the pause draw calls. - I parallelized the tests now so we only run two "pauses" and test multiple things during each pause.
- I noticed that GTK doesn't implement its own
start_event_loop()and our current default one inbackend_bases.pycould run longer than expected ifflush_events()added time to the loop. So I changed this in the final commit to be dependent on the run-time, not the number of iterations of sleep.
f6e282b tob98b898CompareThe implementation of start_event_loop would previously just countthe number of sleeps that occurred. But this could lead to longerevent loop times if flush_events() added time into the loop. Wewant the condition to be dependent on the end-time so we don'trun our loop longer than necessary.
87bfe62 to2d7a86fCompareClosing in favor of doing this all in one consolidated PR:#29062 |
PR summary
As noted by@dopplershift here#28997 (comment)
The macos timers didn't get updated when a user would update the timer interval
timer.interval = newvalue(in that example a new timer is created andthen an interval is set with the delay).This adds tests for the timer updates as well.
PR checklist