Skip to content

Conversation

tmds
Copy link
Member

@tmds tmds commented Sep 6, 2021

Catches wrong mappings as in #58658.

cc @stephentoub @JamesWTruher

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks.

@tmds
Copy link
Member Author

tmds commented Sep 6, 2021

The CI fails on Debian because kill is not an executable. I'll update the test for it.

@tmds
Copy link
Member Author

tmds commented Sep 7, 2021

CI fail is unrelated. This is good to merge.

using var process = Process.Start(new ProcessStartInfo
{
FileName = "/bin/sh", // Use a shell because not all platforms include a 'kill' executable.
ArgumentList = { "-c", $"kill -s {sigArg} {Environment.ProcessId.ToString()}" }
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
ArgumentList = { "-c", $"kill -s {sigArg} {Environment.ProcessId.ToString()}" }
ArgumentList = { "-c", $"kill -s {sigArg} {Environment.ProcessId}" }

@stephentoub stephentoub merged commit 27628d2 into dotnet:main Sep 7, 2021
stephentoub pushed a commit that referenced this pull request Sep 7, 2021
* PosixSignalRegistrationTests.Unix: validate signal pal mapping

* Use InvariantCulture for pid ToString

* Invoke kill through shell.

* Remove InvariantCulture
danmoseley pushed a commit that referenced this pull request Sep 8, 2021
…managed code. (#58682)

* The signal enum in the native library should match the managed code.

* PosixSignalRegistrationTests.Unix: validate signal pal mapping (#58711)

* PosixSignalRegistrationTests.Unix: validate signal pal mapping

* Use InvariantCulture for pid ToString

* Invoke kill through shell.

* Remove InvariantCulture

Co-authored-by: James Truher <[email protected]>
Co-authored-by: Tom Deseyn <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Oct 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants