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

Reduce usage of bang + reduce usage of throw/catch #3771

Merged
merged 1 commit into from
Jun 16, 2022
Merged

Reduce usage of bang + reduce usage of throw/catch #3771

Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions src/AttachVS/AttachVS.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,8 @@
<AdditionalFiles Include="PublicAPI/PublicAPI.Shipped.txt" />
<AdditionalFiles Include="PublicAPI/PublicAPI.Unshipped.txt" />
</ItemGroup>
<ItemGroup>
<Compile Include="..\..\shared\NullableAttributes.cs" Link="NullableAttributes.cs" />
</ItemGroup>
<Import Project="$(TestPlatformRoot)scripts\build\TestPlatform.targets" />
</Project>
5 changes: 3 additions & 2 deletions src/AttachVS/AttachVs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -196,13 +197,13 @@ private static bool AttachVs(Process vs, int pid)
var parent = process;
while (!IsVsOrNull(parent))
{
parent = GetParentProcess(parent!);
parent = GetParentProcess(parent);
}

return parent;
}

private static bool IsVsOrNull(Process? process)
private static bool IsVsOrNull([NotNullWhen(false)] Process? process)
{
if (process == null)
{
Expand Down
2 changes: 1 addition & 1 deletion ...taCollectors/Microsoft.TestPlatform.Extensions.EventLogCollector/EventLogDataCollector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ private void ConfigureEventSources(CollectorNameValueConfigurationManager collec
string? eventSourcesStr = collectorNameValueConfigurationManager[EventLogConstants.SettingEventSources];
if (!eventSourcesStr.IsNullOrEmpty())
{
EventSources = ParseCommaSeparatedList(eventSourcesStr!);
EventSources = ParseCommaSeparatedList(eventSourcesStr);
EqtTrace.Verbose(
$"EventLogDataCollector configuration: {EventLogConstants.SettingEventSources}={EventSources}");
}
Expand Down
189 changes: 116 additions & 73 deletions src/Microsoft.TestPlatform.Client/DesignMode/DesignModeClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,52 +180,43 @@ private void ProcessRequests(ITestRequestManager testRequestManager)
case MessageType.StartTestSession:
{
var testSessionPayload = _communicationManager.DeserializePayload<StartTestSessionPayload>(message);
TPDebug.Assert(testSessionPayload is not null, "testSessionPayload is null");
StartTestSession(testSessionPayload, testRequestManager);
break;
}

case MessageType.StopTestSession:
{
var testSessionPayload = _communicationManager.DeserializePayload<StopTestSessionPayload>(message);
TPDebug.Assert(testSessionPayload is not null, "testSessionPayload is null");
StopTestSession(testSessionPayload, testRequestManager);
break;
}

case MessageType.StartDiscovery:
{
var discoveryPayload = _dataSerializer.DeserializePayload<DiscoveryRequestPayload>(message);
TPDebug.Assert(discoveryPayload is not null, "discoveryPayload is null");
StartDiscovery(discoveryPayload, testRequestManager);
break;
}

case MessageType.GetTestRunnerProcessStartInfoForRunAll:
case MessageType.GetTestRunnerProcessStartInfoForRunSelected:
{
var testRunPayload =
_communicationManager.DeserializePayload<TestRunRequestPayload>(
message);
TPDebug.Assert(testRunPayload is not null, "testRunPayload is null");
var testRunPayload = _communicationManager.DeserializePayload<TestRunRequestPayload>(message);
StartTestRun(testRunPayload, testRequestManager, shouldLaunchTesthost: true);
break;
}

case MessageType.TestRunAllSourcesWithDefaultHost:
case MessageType.TestRunSelectedTestCasesDefaultHost:
{
var testRunPayload =
_communicationManager.DeserializePayload<TestRunRequestPayload>(
message);
var testRunPayload = _communicationManager.DeserializePayload<TestRunRequestPayload>(message);
StartTestRun(testRunPayload, testRequestManager, shouldLaunchTesthost: false);
break;
}

case MessageType.TestRunAttachmentsProcessingStart:
{
var testRunAttachmentsProcessingPayload =
_communicationManager.DeserializePayload<TestRunAttachmentsProcessingPayload>(message);
var testRunAttachmentsProcessingPayload = _communicationManager.DeserializePayload<TestRunAttachmentsProcessingPayload>(message);
StartTestRunAttachmentsProcessing(testRunAttachmentsProcessingPayload, testRequestManager);
break;
}
Expand Down Expand Up @@ -462,6 +453,12 @@ private void StartTestRun(TestRunRequestPayload? testRunPayload, ITestRequestMan
{
testRequestManager.ResetOptions();

if (testRunPayload is null)
{
OnError(null);
return;
}

// We must avoid re-launching the test host if the test run payload already
// contains test session info. Test session info being present is an indicative
// of an already running test host spawned by a start test session call.
Expand All @@ -476,88 +473,118 @@ private void StartTestRun(TestRunRequestPayload? testRunPayload, ITestRequestMan
}
catch (Exception ex)
{
EqtTrace.Error("DesignModeClient: Exception in StartTestRun: " + ex);

var testMessagePayload = new TestMessagePayload { MessageLevel = TestMessageLevel.Error, Message = ex.ToString() };
_communicationManager.SendMessage(MessageType.TestMessage, testMessagePayload);
var runCompletePayload = new TestRunCompletePayload()
{
TestRunCompleteArgs = new TestRunCompleteEventArgs(null, false, true, ex, null, null, TimeSpan.MinValue),
LastRunTests = null
};

// Send run complete to translation layer
_communicationManager.SendMessage(MessageType.ExecutionComplete, runCompletePayload);
OnError(ex);
}
});

void OnError(Exception? ex)
{
EqtTrace.Error("DesignModeClient.StartTestRun: " + ex ?? "payload was null");

var testMessagePayload = new TestMessagePayload { MessageLevel = TestMessageLevel.Error, Message = ex?.ToString() };
_communicationManager.SendMessage(MessageType.TestMessage, testMessagePayload);
var runCompletePayload = new TestRunCompletePayload()
{
TestRunCompleteArgs = new TestRunCompleteEventArgs(null, false, true, ex, null, null, TimeSpan.MinValue),
LastRunTests = null
};

// Send run complete to translation layer
_communicationManager.SendMessage(MessageType.ExecutionComplete, runCompletePayload);
}
}

private void StartDiscovery(DiscoveryRequestPayload discoveryRequestPayload, ITestRequestManager testRequestManager)
private void StartDiscovery(DiscoveryRequestPayload? discoveryRequestPayload, ITestRequestManager testRequestManager)
{
Task.Run(
() =>
Task.Run(() =>
{
try
{
try
testRequestManager.ResetOptions();
if (discoveryRequestPayload is null)
{
testRequestManager.ResetOptions();
testRequestManager.DiscoverTests(discoveryRequestPayload, new DesignModeTestEventsRegistrar(this), _protocolConfig);
OnError(null);
return;
}
catch (Exception ex)
{
EqtTrace.Error("DesignModeClient: Exception in StartDiscovery: " + ex);

var testMessagePayload = new TestMessagePayload { MessageLevel = TestMessageLevel.Error, Message = ex.ToString() };
_communicationManager.SendMessage(MessageType.TestMessage, testMessagePayload);
testRequestManager.DiscoverTests(discoveryRequestPayload, new DesignModeTestEventsRegistrar(this), _protocolConfig);
}
catch (Exception ex)
{
OnError(ex);
}
});

void OnError(Exception? ex)
{
EqtTrace.Error("DesignModeClient.StartDiscovery: " + ex ?? "payload is null");

var payload = new DiscoveryCompletePayload()
{
IsAborted = true,
LastDiscoveredTests = null,
TotalTests = -1
};
var testMessagePayload = new TestMessagePayload { MessageLevel = TestMessageLevel.Error, Message = ex?.ToString() };
_communicationManager.SendMessage(MessageType.TestMessage, testMessagePayload);

// Send run complete to translation layer
_communicationManager.SendMessage(MessageType.DiscoveryComplete, payload);
}
});
var payload = new DiscoveryCompletePayload()
{
IsAborted = true,
LastDiscoveredTests = null,
TotalTests = -1
};

// Send run complete to translation layer
_communicationManager.SendMessage(MessageType.DiscoveryComplete, payload);
}
}

private void StartTestRunAttachmentsProcessing(TestRunAttachmentsProcessingPayload? attachmentsProcessingPayload, ITestRequestManager testRequestManager)
{
Task.Run(
() =>
Task.Run(() =>
{
try
{
try
if (attachmentsProcessingPayload is null)
{
// TODO: Avoid throwing/catching NRE
testRequestManager.ProcessTestRunAttachments(attachmentsProcessingPayload!, new TestRunAttachmentsProcessingEventsHandler(_communicationManager), _protocolConfig);
OnError(null);
return;
}
catch (Exception ex)
{
EqtTrace.Error("DesignModeClient: Exception in StartTestRunAttachmentsProcessing: " + ex);

var testMessagePayload = new TestMessagePayload { MessageLevel = TestMessageLevel.Error, Message = ex.ToString() };
_communicationManager.SendMessage(MessageType.TestMessage, testMessagePayload);
testRequestManager.ProcessTestRunAttachments(attachmentsProcessingPayload, new TestRunAttachmentsProcessingEventsHandler(_communicationManager), _protocolConfig);
}
catch (Exception ex)
{
OnError(ex);
}
});

void OnError(Exception? ex)
{
EqtTrace.Error("DesignModeClient.StartTestRunAttachmentsProcessing: " + ex ?? "payload is null");

var payload = new TestRunAttachmentsProcessingCompletePayload()
{
Attachments = null
};
var testMessagePayload = new TestMessagePayload { MessageLevel = TestMessageLevel.Error, Message = ex?.ToString() };
_communicationManager.SendMessage(MessageType.TestMessage, testMessagePayload);

// Send run complete to translation layer
_communicationManager.SendMessage(MessageType.TestRunAttachmentsProcessingComplete, payload);
}
});
var payload = new TestRunAttachmentsProcessingCompletePayload()
{
Attachments = null
};

// Send run complete to translation layer
_communicationManager.SendMessage(MessageType.TestRunAttachmentsProcessingComplete, payload);
}
}

private void StartTestSession(StartTestSessionPayload payload, ITestRequestManager requestManager)
private void StartTestSession(StartTestSessionPayload? payload, ITestRequestManager requestManager)
{
Task.Run(() =>
{
var eventsHandler = new TestSessionEventsHandler(_communicationManager);

try
{
if (payload is null)
{
OnError(eventsHandler, null);
return;
}

var customLauncher = payload.HasCustomHostLauncher
? DesignModeTestHostLauncherFactory.GetCustomHostLauncherForTestRun(this, payload.IsDebuggingEnabled)
: null;
Expand All @@ -567,15 +594,20 @@ private void StartTestSession(StartTestSessionPayload payload, ITestRequestManag
}
catch (Exception ex)
{
EqtTrace.Error("DesignModeClient: Exception in StartTestSession: " + ex);

eventsHandler.HandleLogMessage(TestMessageLevel.Error, ex.ToString());
eventsHandler.HandleStartTestSessionComplete(new());
OnError(eventsHandler, ex);
}
});

static void OnError(TestSessionEventsHandler eventsHandler, Exception? ex)
{
EqtTrace.Error("DesignModeClient.StartTestSession: " + ex ?? "payload is null");

eventsHandler.HandleLogMessage(TestMessageLevel.Error, ex?.ToString());
eventsHandler.HandleStartTestSessionComplete(new());
}
}

private void StopTestSession(StopTestSessionPayload payload, ITestRequestManager requestManager)
private void StopTestSession(StopTestSessionPayload? payload, ITestRequestManager requestManager)
{
Task.Run(() =>
{
Expand All @@ -584,16 +616,27 @@ private void StopTestSession(StopTestSessionPayload payload, ITestRequestManager
try
{
requestManager.ResetOptions();
if (payload is null)
{
OnError(eventsHandler, null);
return;
}

requestManager.StopTestSession(payload, eventsHandler, _protocolConfig);
}
catch (Exception ex)
{
EqtTrace.Error("DesignModeClient: Exception in StopTestSession: " + ex);

eventsHandler.HandleLogMessage(TestMessageLevel.Error, ex.ToString());
eventsHandler.HandleStopTestSessionComplete(new(payload.TestSessionInfo));
OnError(eventsHandler, ex);
}
});

void OnError(TestSessionEventsHandler eventsHandler, Exception? ex)
{
EqtTrace.Error("DesignModeClient.StopTestSession: " + ex ?? "payload is null");

eventsHandler.HandleLogMessage(TestMessageLevel.Error, ex?.ToString());
eventsHandler.HandleStopTestSessionComplete(new(payload?.TestSessionInfo));
}
}

#region IDisposable Support
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.TestPlatform.Client/Discovery/DiscoveryRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,9 @@ public void HandleRawMessage(string rawMessage)
? _dataSerializer.DeserializeMessage(rawMessage)
: null;

if (string.Equals(message?.MessageType, MessageType.DiscoveryComplete))
if (MessageType.DiscoveryComplete.Equals(message?.MessageType))
{
var discoveryCompletePayload = _dataSerializer.DeserializePayload<DiscoveryCompletePayload>(message!);
var discoveryCompletePayload = _dataSerializer.DeserializePayload<DiscoveryCompletePayload>(message);
rawMessage = UpdateRawMessageWithTelemetryInfo(discoveryCompletePayload, message) ?? rawMessage;
HandleLoggerManagerDiscoveryComplete(discoveryCompletePayload);
}
Expand Down
Loading