-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
improve resiliency of process tests #35584
Conversation
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
Outdated
Show resolved
Hide resolved
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.
LGTM thanks for adding those. Couple of optional comments
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
Outdated
Show resolved
Hide resolved
@@ -2119,7 +2119,7 @@ public void Kill_ExitedChildProcess_DoesNotThrow(bool killTree) | |||
Process process = CreateProcess(); | |||
process.Start(); | |||
|
|||
process.WaitForExit(); | |||
Assert.True(process.WaitForExit(Helpers.PassingTestTimeoutMilliseconds)); |
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.
We already have WaitInMS
for this (its 5 mins but that should still be OK)
Process[] all = Process.GetProcesses(); | ||
foreach (Process p in all) | ||
{ | ||
Console.WriteLine("{0,8} {1}", p.Id, p.ProcessName); |
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.
Nit:
p.Dispose();
I suspect Kill_ExitedChildProcess_DoesNotThrow is hanging in the WaitForExit so I modify it to use overload with timeout. I also add time limit and some more instrumentation to the Unix version.
Now the test should fail instead of hanging and we should hopefully collect more information.
fixes #35506