Skip to content

support logging in CommandRunner when tests are executed in parallel #3732

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.

Sign up for GitHub

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

Merged
kartheekp-ms merged 1 commit into from
Nov 4, 2020
Merged

support logging in CommandRunner when tests are executed in parallel #3732

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 26 additions & 30 deletions test/TestUtilities/Test.Utility/CommandRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ public static CommandRunnerResult Run(
bool waitForExit,
int timeOutInMilliseconds = 60000,
Action<StreamWriter> inputAction = null,
bool shareProcessObject = false,
Copy link
Contributor Author

@kartheekp-ms kartheekp-ms Oct 26, 2020

Choose a reason for hiding this comment

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

I found no callers in the entire codebase who is passing this optional parameter. Hence removed it. If there are any concerns from others, I can add it back.

IDictionary<string, string> environmentVariables = null)
{
var psi = new ProcessStartInfo(Path.GetFullPath(process), arguments)
Expand Down Expand Up @@ -60,27 +59,33 @@ public static CommandRunnerResult Run(

Process p = null;

try
using (p = new Process())
Copy link
Member

@nkolev92 nkolev92 Oct 26, 2020

Choose a reason for hiding this comment

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

Can you shortly elaborate in more detail how your change helps?

I know you linked the issues and I appreciate that, but I was hoping you could save us some time digging through the linked docs if possible.

Copy link
Contributor Author

@kartheekp-ms kartheekp-ms Oct 27, 2020

Choose a reason for hiding this comment

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

Updated PR description with additional information. Pls let me know your feedback.

{
p = new Process();
p.OutputDataReceived += OutputHandler;
p.ErrorDataReceived += ErrorHandler;

p.StartInfo = psi;
p.Start();

var outputTask = ConsumeStreamReaderAsync(p.StandardOutput, output);
var errorTask = ConsumeStreamReaderAsync(p.StandardError, errors);

inputAction?.Invoke(p.StandardInput);

p.BeginOutputReadLine();
p.BeginErrorReadLine();

if (waitForExit)
{
#if DEBUG
var processExited = true;
#if DEBUG
p.WaitForExit();
var processExited = true;
#else
var processExited = p.WaitForExit(timeOutInMilliseconds);
#endif
if (!processExited)
if (processExited)
{
p.WaitForExit();
exitCode = p.ExitCode;
}
else
{
Kill(p);
WaitForExit(p);
Expand All @@ -89,34 +94,25 @@ public static CommandRunnerResult Run(

throw new TimeoutException($"{processName} timed out: " + psi.Arguments);
}

if (processExited)
{
Task.WaitAll(outputTask, errorTask);
exitCode = p.ExitCode;
}
}

p.CancelOutputRead();
p.CancelErrorRead();
}
finally

void OutputHandler(object sendingProcess, DataReceivedEventArgs e)
{
if (!shareProcessObject)
{
p.Dispose();
}
if (!string.IsNullOrEmpty(e.Data))
output.AppendLine(e.Data);
}

return new CommandRunnerResult(p, exitCode, output.ToString(), errors.ToString());
}

private static async Task ConsumeStreamReaderAsync(StreamReader reader, StringBuilder lines)
{
await Task.Yield();

string line;
while ((line = await reader.ReadLineAsync()) != null)
void ErrorHandler(object sendingProcess, DataReceivedEventArgs e)
{
lines.AppendLine(line);
if (!string.IsNullOrEmpty(e.Data))
errors.AppendLine(e.Data);
}

return new CommandRunnerResult(p, exitCode, output.ToString(), errors.ToString());
}

private static void Kill(Process process)
Expand Down