Bugfix/cli tests win fix event loop freeze#4961
Conversation
Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
EvtClose can wait for in-flight callbacks, stalling pytest daemon teardown.Close the subscription on a detached daemon thread so pytest daemon teardown is not stalled if it blocks. Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4961 +/- ##
=======================================
Coverage 87.84% 87.84%
=======================================
Files 271 271
Lines 14669 14669
=======================================
Hits 12884 12884
Misses 1785 1785 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Win10 is timing out in some instances. Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a Windows-specific pytest teardown hang by avoiding blocking the asyncio event loop thread during Windows Event Log subscription cleanup.
Changes:
- Add a
run_detached_thread()helper to run best-effort blocking cleanup on a daemon thread. - Update the Windows service controller’s Event Log follower to close Event Log handles asynchronously during generator teardown.
- Increase the default CLI test timeout for the
startcommand.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/cli/utilities/threadutils.py |
Adds run_detached_thread() helper for detached daemon-thread execution. |
tests/cli/utilities/__init__.py |
Re-exports run_detached_thread from utilities. |
tests/cli/controller/winsvc_multipassd_controller.py |
Uses detached thread cleanup for Event Log subscription close; adds shutdown signaling. |
tests/cli/conftest.py |
Increases default start command timeout from 90s to 180s. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ricab
left a comment
ricab
left a comment
There was a problem hiding this comment.
I am OK with this modulo copilot flaggings.
xmkg
force-pushed
the
bugfix/cli-tests-win-fix-event-loop-freeze
branch
from
20d56a0 to
ab3228c
Compare
ricab
left a comment
ricab
left a comment
There was a problem hiding this comment.
Small suggestion/question inline.
|
Also, the last commit is unverified. Would that be easy to fix @xmkg ? |
Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
xmkg
force-pushed
the
bugfix/cli-tests-win-fix-event-loop-freeze
branch
from
ab3228c to
160516e
Compare
Fixed |
ricab
left a comment
ricab
left a comment
There was a problem hiding this comment.
Another detail I didn't notice before. Otherwise alright.
| # scheduling in case the loop is already closing. | ||
| if closing.is_set(): | ||
| return | ||
| with suppress(RuntimeError): |
There was a problem hiding this comment.
I don't suppose there is a more specific type this could catch, to avoid sinking other problems?
Win: Avoid blocking the event loop on close
EvtClose can wait for in-flight callbacks, stalling pytest daemon teardown. Close the subscription on a detached daemon thread so pytest daemon teardown is not stalled if it blocks.