Skip to content

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

Closed
wants to merge 6 commits into from
Closed

Do not use SIGTERM #49352

wants to merge 6 commits into from

Conversation

tmat
Copy link
Member

@tmat tmat commented Jun 11, 2025

Workaround for #49307

Proper fix requires change in runtime: dotnet/runtime#116556

@Copilot Copilot AI review requested due to automatic review settings June 11, 2025 19:18
@tmat tmat requested review from arunchndr and a team as code owners June 11, 2025 19:18
Copy link
Contributor

@Copilot Copilot AI left a 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 use SIGKILL 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]

@tmat tmat closed this Jun 12, 2025
@tmat
Copy link
Member Author

tmat commented Jun 12, 2025

Superseded by #49336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant