Skip to content

Commit

Permalink
Fix race condition in EndpointInfoSourceTests. (#776)
Browse files Browse the repository at this point in the history 
* Fix race condition in EndpointInfoSourceTests.
  • Loading branch information
@jander-msft
jander-msft authored Aug 31, 2021
1 parent afd53f0 commit 26e427c
Show file tree
Hide file tree
Showing 11 changed files with 169 additions and 81 deletions.
2 changes: 1 addition & 1 deletion src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Runners/AppRunner.cs
View file
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
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
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
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
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
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
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
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

0 comments on commit 26e427c

Please sign in to comment.