Skip to content
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

Fix race condition in EndpointInfoSourceTests. #776

Merged
merged 3 commits into from
Aug 31, 2021
Merged

Fix race condition in EndpointInfoSourceTests. #776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f1b1da
Fix race condition in EndpointInfoSourceTests.
jander-msft Aug 26, 2021
0cb2328
Add comments.
jander-msft Aug 31, 2021
4747c45
Remove non-async ProcessId property from LoggingRunnerAdapter.
jander-msft Aug 31, 2021
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
2 changes: 1 addition & 1 deletion src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Runners/AppRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public sealed class AppRunner : IAsyncDisposable

public int ExitCode => _adapter.ExitCode;

public int ProcessId => _adapter.ProcessId;
public Task<int> ProcessIdTask => _adapter.ProcessIdTask;

/// <summary>
/// Name of the scenario to run in the application.
Expand Down
17 changes: 12 additions & 5 deletions src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Runners/LoggingRunnerAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public sealed class LoggingRunnerAdapter : IAsyncDisposable
{
private readonly CancellationTokenSource _cancellation = new();
private readonly ITestOutputHelper _outputHelper;
private readonly TaskCompletionSource<int> _processIdSource =
new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly DotNetRunner _runner;
private readonly List<string> _standardErrorLines = new();
private readonly List<string> _standardOutputLines = new();
Expand All @@ -30,8 +32,7 @@ public sealed class LoggingRunnerAdapter : IAsyncDisposable
public int ExitCode => _exitCode.HasValue ?
_exitCode.Value : throw new InvalidOperationException("Must call WaitForExitAsync before getting exit code.");

public int ProcessId => _processId.HasValue ?
_processId.Value : throw new InvalidOperationException("Process was not started.");
public Task<int> ProcessIdTask => _processIdSource.Task;

public event Action<string> ReceivedStandardErrorLine;

Expand All @@ -56,6 +57,8 @@ public async ValueTask DisposeAsync()

_cancellation.Cancel();

_processIdSource.TrySetCanceled(_cancellation.Token);

// Shutdown the runner
_outputHelper.WriteLine("Stopping...");
_runner.ForceClose();
Expand Down Expand Up @@ -96,11 +99,15 @@ public async Task StartAsync(CancellationToken token)
}
_outputHelper.WriteLine("End Environment:");

_outputHelper.WriteLine("Starting...");
await _runner.StartAsync(token).ConfigureAwait(false);
using (var _ = token.Register(() => _processIdSource.TrySetCanceled(token)))
{
_outputHelper.WriteLine("Starting...");
await _runner.StartAsync(token).ConfigureAwait(false);
}

_processId = _runner.ProcessId;
_outputHelper.WriteLine("Process ID: {0}", _runner.ProcessId);
_processId = _runner.ProcessId;
_processIdSource.TrySetResult(_runner.ProcessId);

_standardErrorTask = ReadLinesAsync(_runner.StandardError, _standardErrorLines, ReceivedStandardErrorLine, _cancellation.Token);
_standardOutputTask = ReadLinesAsync(_runner.StandardOutput, _standardOutputLines, ReceivedStandardOutputLine, _cancellation.Token);
Expand Down
7 changes: 7 additions & 0 deletions src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TaskExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,12 @@ public static async Task WithCancellation(this Task task, CancellationToken toke
localTokenSource.Cancel();
}
}

public static async Task<T> WithCancellation<T>(this Task<T> task, CancellationToken token)
{
await WithCancellation((Task)task, token);

return task.Result;
}
}
}
6 changes: 4 additions & 2 deletions src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ public Task DumpTest(DiagnosticPortConnectionMode mode, DumpType type)
TestAppScenarios.AsyncWait.Name,
appValidate: async (runner, client) =>
{
ProcessInfo processInfo = await client.GetProcessAsync(runner.ProcessId);
int processId = await runner.ProcessIdTask;

ProcessInfo processInfo = await client.GetProcessAsync(processId);
Assert.NotNull(processInfo);

using ResponseStreamHolder holder = await client.CaptureDumpAsync(runner.ProcessId, type);
using ResponseStreamHolder holder = await client.CaptureDumpAsync(processId, type);
Assert.NotNull(holder);

byte[] headerBuffer = new byte[64];
Expand Down
51 changes: 32 additions & 19 deletions src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ await ScenarioRunner.SingleTarget(
TestAppScenarios.AsyncWait.Name,
appValidate: async (appRunner, apiClient) =>
{
OperationResponse response = await apiClient.EgressTraceAsync(appRunner.ProcessId, durationSeconds: 5, FileProviderName);
int processId = await appRunner.ProcessIdTask;

OperationResponse response = await apiClient.EgressTraceAsync(processId, durationSeconds: 5, FileProviderName);
Assert.Equal(HttpStatusCode.Accepted, response.StatusCode);

OperationStatusResponse operationResult = await apiClient.PollOperationToCompletion(response.OperationUri);
Expand All @@ -81,7 +83,9 @@ await ScenarioRunner.SingleTarget(
TestAppScenarios.AsyncWait.Name,
appValidate: async (appRunner, apiClient) =>
{
OperationResponse response = await apiClient.EgressTraceAsync(appRunner.ProcessId, durationSeconds: -1, FileProviderName);
int processId = await appRunner.ProcessIdTask;

OperationResponse response = await apiClient.EgressTraceAsync(processId, durationSeconds: -1, FileProviderName);
Assert.Equal(HttpStatusCode.Accepted, response.StatusCode);

OperationStatusResponse operationResult = await apiClient.GetOperationStatus(response.OperationUri);
Expand Down Expand Up @@ -113,8 +117,10 @@ await ScenarioRunner.SingleTarget(
TestAppScenarios.AsyncWait.Name,
appValidate: async (appRunner, apiClient) =>
{
OperationResponse response1 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId);
OperationResponse response2 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId, delay: false);
int processId = await appRunner.ProcessIdTask;

OperationResponse response1 = await EgressTraceWithDelay(apiClient, processId);
OperationResponse response2 = await EgressTraceWithDelay(apiClient, processId, delay: false);
await CancelEgressOperation(apiClient, response2);

List<OperationSummary> result = await apiClient.GetOperations();
Expand Down Expand Up @@ -146,18 +152,20 @@ await ScenarioRunner.SingleTarget(
TestAppScenarios.AsyncWait.Name,
appValidate: async (appRunner, apiClient) =>
{
OperationResponse response1 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId);
OperationResponse response2 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId);
OperationResponse response3 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId);
int processId = await appRunner.ProcessIdTask;

ValidationProblemDetailsException ex = await Assert.ThrowsAsync<ValidationProblemDetailsException>(() => EgressTraceWithDelay(apiClient, appRunner.ProcessId));
OperationResponse response1 = await EgressTraceWithDelay(apiClient, processId);
OperationResponse response2 = await EgressTraceWithDelay(apiClient, processId);
OperationResponse response3 = await EgressTraceWithDelay(apiClient, processId);

ValidationProblemDetailsException ex = await Assert.ThrowsAsync<ValidationProblemDetailsException>(() => EgressTraceWithDelay(apiClient, processId));
Assert.Equal(HttpStatusCode.TooManyRequests, ex.StatusCode);
Assert.Equal((int)HttpStatusCode.TooManyRequests, ex.Details.Status.GetValueOrDefault());

await CancelEgressOperation(apiClient, response1);
await CancelEgressOperation(apiClient, response2);

OperationResponse response4 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId, delay: false);
OperationResponse response4 = await EgressTraceWithDelay(apiClient, processId, delay: false);

await CancelEgressOperation(apiClient, response3);
await CancelEgressOperation(apiClient, response4);
Expand All @@ -180,25 +188,28 @@ await ScenarioRunner.SingleTarget(
TestAppScenarios.AsyncWait.Name,
appValidate: async (appRunner, apiClient) =>
{
OperationResponse response1 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId);
OperationResponse response3 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId);
using HttpResponseMessage traceDirect1 = await TraceWithDelay(apiClient, appRunner.ProcessId);
int processId = await appRunner.ProcessIdTask;

OperationResponse response1 = await EgressTraceWithDelay(apiClient, processId);
OperationResponse response3 = await EgressTraceWithDelay(apiClient, processId);
using HttpResponseMessage traceDirect1 = await TraceWithDelay(apiClient, processId);
Assert.Equal(HttpStatusCode.OK, traceDirect1.StatusCode);

ValidationProblemDetailsException ex = await Assert.ThrowsAsync<ValidationProblemDetailsException>(() => EgressTraceWithDelay(apiClient, appRunner.ProcessId, delay: false));
ValidationProblemDetailsException ex = await Assert.ThrowsAsync<ValidationProblemDetailsException>(
() => EgressTraceWithDelay(apiClient, processId, delay: false));
Assert.Equal(HttpStatusCode.TooManyRequests, ex.StatusCode);

using HttpResponseMessage traceDirect = await TraceWithDelay(apiClient, appRunner.ProcessId, delay: false);
using HttpResponseMessage traceDirect = await TraceWithDelay(apiClient, processId, delay: false);
Assert.Equal(HttpStatusCode.TooManyRequests, traceDirect.StatusCode);

//Validate that the failure from a direct call (handled by middleware)
//matches the failure produces by egress operations (handled by the Mvc ActionResult stack)
using HttpResponseMessage egressDirect = await EgressDirect(apiClient, appRunner.ProcessId);
using HttpResponseMessage egressDirect = await EgressDirect(apiClient, processId);
Assert.Equal(HttpStatusCode.TooManyRequests, egressDirect.StatusCode);
Assert.Equal(await egressDirect.Content.ReadAsStringAsync(), await traceDirect.Content.ReadAsStringAsync());

await CancelEgressOperation(apiClient, response1);
OperationResponse response4 = await EgressTraceWithDelay(apiClient, appRunner.ProcessId, delay: false);
OperationResponse response4 = await EgressTraceWithDelay(apiClient, processId, delay: false);

await CancelEgressOperation(apiClient, response3);
await CancelEgressOperation(apiClient, response4);
Expand All @@ -224,19 +235,21 @@ await ScenarioRunner.SingleTarget(
TestAppScenarios.AsyncWait.Name,
appValidate: async (appRunner, appClient) =>
{
ProcessInfo processInfo = await appClient.GetProcessAsync(appRunner.ProcessId);
int processId = await appRunner.ProcessIdTask;

ProcessInfo processInfo = await appClient.GetProcessAsync(processId);
Assert.NotNull(processInfo);

// Dump Error Check
ValidationProblemDetailsException validationProblemDetailsExceptionDumps = await Assert.ThrowsAsync<ValidationProblemDetailsException>(
() => appClient.CaptureDumpAsync(appRunner.ProcessId, DumpType.Mini));
() => appClient.CaptureDumpAsync(processId, DumpType.Mini));
Assert.Equal(HttpStatusCode.BadRequest, validationProblemDetailsExceptionDumps.StatusCode);
Assert.Equal(StatusCodes.Status400BadRequest, validationProblemDetailsExceptionDumps.Details.Status);
Assert.Equal(DisabledHTTPEgressErrorMessage, validationProblemDetailsExceptionDumps.Message);

// Logs Error Check
ValidationProblemDetailsException validationProblemDetailsExceptionLogs = await Assert.ThrowsAsync<ValidationProblemDetailsException>(
() => appClient.CaptureLogsAsync(appRunner.ProcessId, TestTimeouts.LogsDuration, LogLevel.None, LogFormat.NDJson));
() => appClient.CaptureLogsAsync(processId, TestTimeouts.LogsDuration, LogLevel.None, LogFormat.NDJson));
Assert.Equal(HttpStatusCode.BadRequest, validationProblemDetailsExceptionLogs.StatusCode);
Assert.Equal(StatusCodes.Status400BadRequest, validationProblemDetailsExceptionLogs.Details.Status);
Assert.Equal(DisabledHTTPEgressErrorMessage, validationProblemDetailsExceptionLogs.Message);
Expand Down
34 changes: 22 additions & 12 deletions src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/LogsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public Task LogsDefaultLevelNoneNotSupportedViaQueryTest(DiagnosticPortConnectio
async () =>
{
using ResponseStreamHolder _ = await client.CaptureLogsAsync(
runner.ProcessId,
await runner.ProcessIdTask,
TestTimeouts.LogsDuration,
LogLevel.None,
logFormat);
Expand Down Expand Up @@ -219,7 +219,7 @@ public Task LogsDefaultLevelNoneNotSupportedViaBodyTest(DiagnosticPortConnection
async () =>
{
using ResponseStreamHolder _ = await client.CaptureLogsAsync(
runner.ProcessId,
await runner.ProcessIdTask,
TestTimeouts.LogsDuration,
new LogsConfiguration() { LogLevel = LogLevel.None },
logFormat);
Expand Down Expand Up @@ -406,11 +406,16 @@ private Task ValidateLogsAsync(
_httpClientFactory,
mode,
TestAppScenarios.Logger.Name,
appValidate: (runner, client) => ValidateResponseStream(
runner,
client.CaptureLogsAsync(runner.ProcessId, TestTimeouts.LogsDuration, logLevel, logFormat),
callback,
logFormat));
appValidate: async (runner, client) =>
await ValidateResponseStream(
runner,
client.CaptureLogsAsync(
await runner.ProcessIdTask,
TestTimeouts.LogsDuration,
logLevel,
logFormat),
callback,
logFormat));
}

private Task ValidateLogsAsync(
Expand All @@ -424,11 +429,16 @@ private Task ValidateLogsAsync(
_httpClientFactory,
mode,
TestAppScenarios.Logger.Name,
appValidate: (runner, client) => ValidateResponseStream(
runner,
client.CaptureLogsAsync(runner.ProcessId, TestTimeouts.LogsDuration, configuration, logFormat),
callback,
logFormat));
appValidate: async (runner, client) =>
await ValidateResponseStream(
runner,
client.CaptureLogsAsync(
await runner.ProcessIdTask,
TestTimeouts.LogsDuration,
configuration,
logFormat),
callback,
logFormat));
}

private async Task ValidateResponseStream(AppRunner runner, Task<ResponseStreamHolder> holderTask, Func<ChannelReader<LogEntry>, Task> callback, LogFormat logFormat)
Expand Down
18 changes: 13 additions & 5 deletions src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/ProcessTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,16 @@ public Task SingleProcessIdentificationTest(DiagnosticPortConnectionMode mode)
TestAppScenarios.AsyncWait.Name,
appValidate: async (runner, client) =>
{
int processId = await runner.ProcessIdTask;

// GET /processes and filter to just the single process
IEnumerable<ProcessIdentifier> identifiers = await client.GetProcessesWithRetryAsync(
_outputHelper,
new[] { runner.ProcessId });
new[] { processId });
Assert.NotNull(identifiers);
Assert.Single(identifiers);

await VerifyProcessAsync(client, identifiers, runner.ProcessId, expectedEnvVarValue);
await VerifyProcessAsync(client, identifiers, processId, expectedEnvVarValue);

await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue);
},
Expand Down Expand Up @@ -129,7 +131,7 @@ await appRunners.ExecuteAsync(async () =>
IList<int> unmatchedPids = new List<int>();
foreach (AppRunner runner in appRunners)
{
unmatchedPids.Add(runner.ProcessId);
unmatchedPids.Add(await runner.ProcessIdTask);
}

// Query for process identifiers
Expand Down Expand Up @@ -202,7 +204,7 @@ await appRunners.ExecuteAsync(async () =>
{
Assert.True(runner.Environment.TryGetValue(ExpectedEnvVarName, out string expectedEnvVarValue));

await VerifyProcessAsync(apiClient, identifiers, runner.ProcessId, expectedEnvVarValue);
await VerifyProcessAsync(apiClient, identifiers, await runner.ProcessIdTask, expectedEnvVarValue);

await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue);
}
Expand All @@ -218,9 +220,15 @@ await appRunners.ExecuteAsync(async () =>
Assert.NotNull(identifiers);

// Verify none of the apps are reported
List<int> runnerProcessIds = new(appCount);
for (int i = 0; i < appCount; i++)
{
Assert.Null(identifiers.FirstOrDefault(p => p.Pid == appRunners[i].ProcessId));
runnerProcessIds.Add(await appRunners[i].ProcessIdTask);
}

foreach (ProcessIdentifier identifier in identifiers)
{
Assert.DoesNotContain(identifier.Pid, runnerProcessIds);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ await appRunner.ExecuteAsync(async () =>

if (null != postAppValidate)
{
await postAppValidate(apiClient, appRunner.ProcessId);
await postAppValidate(apiClient, await appRunner.ProcessIdTask);
}
}
}
Expand Down
Loading