-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Do not use SIGTERM #49352
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
Do not use SIGTERM #49352
Conversation
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.
Pull Request Overview
This PR implements a workaround for #49307 by no longer using SIGTERM on Linux, updating tests and the process runner accordingly.
- Unskip tests previously blocked by SIGTERM issues and adjust assertions to only validate SIGKILL behavior.
- Comment out OS-specific branches in tests and update expected output messages.
- Change
ProcessRunner
to always useSIGKILL
instead of conditional SIGTERM/SIGKILL.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | Removed skips, commented out Windows/Linux branches, updated messages |
src/BuiltInTools/dotnet-watch/Internal/ProcessRunner.cs | Always invoke sys_kill with SIGKILL , removed SIGTERM usage |
Comments suppressed due to low confidence (2)
test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs:813
- Commenting out the Linux branch removes coverage for Unix termination behavior. Instead of deleting the branch, use runtime conditionals or platform-specific skips so both code paths remain tested.
// SIGTERM no longer works on Linux https://github.com/dotnet/sdk/issues/49307
test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs:423
- The test was unskipped despite the underlying SDK issue still open. This may cause test failures on CI. Ensure the root cause is resolved or mark the test as platform-specific skip until fixed upstream.
[Theory]
Superseded by #49336 |
Workaround for #49307
Proper fix requires change in runtime: dotnet/runtime#116556