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.

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

Enable parallel discovery #3349

Merged
merged 40 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
01df3aa
initial implem
Sanan07 Nov 22, 2021
3e0a4b1
temp
Sanan07 Nov 22, 2021
d114afb
initila
Sanan07 Nov 23, 2021
9bc76c7
Stopping discovery after abort
Sanan07 Nov 23, 2021
5f6ba48
removing old logic test
Sanan07 Nov 23, 2021
a239149
Fixing test
Sanan07 Nov 23, 2021
fcdd4b6
fixing last chunk logic
Sanan07 Nov 24, 2021
524448b
temp commit
Sanan07 Nov 25, 2021
e8aa241
Merge branch 'main' of https://github.com/microsoft/vstest into dev/s…
Sanan07 Nov 26, 2021
059181e
Fixing last chunk is empty case. Some refactoring
Sanan07 Nov 26, 2021
0fb8aff
writing first part of tests
Sanan07 Nov 29, 2021
a220af5
more tests
Sanan07 Dec 1, 2021
bb13111
Merge branch 'main' of https://github.com/microsoft/vstest into dev/s…
Sanan07 Dec 1, 2021
adfdabb
fixing public api
Sanan07 Dec 2, 2021
c094306
more public api
Sanan07 Dec 2, 2021
31ebaa0
changing public api files
Sanan07 Dec 3, 2021
35e7060
Merge branch 'dev/sayuzbas/par-discovery' into dev/sayuzbas/parallel-…
Sanan07 Dec 17, 2021
0a2bce3
Merge branch 'main' into pr/3226
Evangelink Feb 10, 2022
50c713e
Update src/Microsoft.TestPlatform.Common/Interfaces/Engine/ClientProt…
Evangelink Feb 10, 2022
29377a3
Apply suggestions from code review
Evangelink Feb 10, 2022
f1c872b
Fix incorrect merges + address analyzer warnings
Evangelink Feb 10, 2022
8dbbd31
Simpliy EqtTrace calls
Evangelink Feb 10, 2022
2ff5ce4
Merge branch 'main' into pr/3226
Evangelink Feb 11, 2022
e7af727
Merge branch 'main' into pr/3226
Evangelink Feb 12, 2022
36db669
Remove API duplication
Evangelink Feb 12, 2022
03eab00
Apply some code review comments
Evangelink Feb 12, 2022
fffcca5
Merge branch 'main' into pr/3226
Haplois Feb 14, 2022
46d5eef
Address some code review comments
Evangelink Feb 14, 2022
56ab677
Update src/Microsoft.TestPlatform.Client/Discovery/DiscoveryRequest.cs
Evangelink Feb 14, 2022
5271c97
Address part of the review comments
Evangelink Feb 15, 2022
226544d
Simpler API
Evangelink Feb 15, 2022
a9115b5
Merge branch 'main' into pr/3226
Evangelink Feb 15, 2022
485cf0b
Rework method
Evangelink Feb 15, 2022
547c9f2
More refactorings
Evangelink Feb 15, 2022
373510a
Fix test
Evangelink Feb 16, 2022
473360d
Remove duplications and race issues
Evangelink Feb 16, 2022
59d2563
Merge branch 'main' into pr/3226
Evangelink Feb 16, 2022
d29f007
Make public as class is internal
Evangelink Feb 16, 2022
47ff337
Fix naming
Evangelink Feb 16, 2022
1f496bd
Improves tests and remove un-needed field
Evangelink Feb 17, 2022
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
11 changes: 10 additions & 1 deletion src/Microsoft.TestPlatform.Client/DesignMode/DesignModeClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,16 @@ private void ProcessRequests(ITestRequestManager testRequestManager)

case MessageType.CancelDiscovery:
{
testRequestManager.CancelDiscovery();
// If testhost has old version, we should use old cancel logic
// to be consistent and not create regression issues
if (_protocolConfig.Version < ObjectModel.Constants.MinimumProtocolVersionWithCancelDiscoveryEventHandlerSupport)
{
testRequestManager.CancelDiscovery();
}
else
{
testRequestManager.CancelDiscoveryWithEventHandler();
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
}
break;
}

Expand Down
36 changes: 36 additions & 0 deletions src/Microsoft.TestPlatform.Client/Discovery/DiscoveryRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,42 @@ bool IRequest.WaitForCompletion(int timeout)
return _discoveryCompleted == null || _discoveryCompleted.WaitOne(timeout);
}

/// <inheritdoc/>
public void AbortWithEventHandler()
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
if (EqtTrace.IsVerboseEnabled)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
EqtTrace.Verbose("DiscoveryRequest.AbortWithEventHandler: Aborting.");
}

lock (_syncObject)
{
if (_disposed)
{
throw new ObjectDisposedException("DiscoveryRequest");
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
}

if (DiscoveryInProgress)
{
DiscoveryManager.Abort(this);
}
else
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DiscoveryRequest.AbortWithEventHandler: No operation to abort.");
}

return;
}
}

if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DiscoveryRequest.AbortWithEventHandler: Aborted.");
}
}

/// <summary>
/// Raised when the test discovery starts.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Microsoft.VisualStudio.TestPlatform.Client.Discovery.DiscoveryRequest.AbortWithEventHandler() -> void
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
Microsoft.VisualStudio.TestPlatform.Client.RequestHelper.ITestRequestManager.CancelDiscoveryWithEventHandler() -> void
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public interface ITestRequestManager : IDisposable
/// <summary>
/// Initializes the extensions while probing additional paths.
/// </summary>
///
///
/// <param name="pathToAdditionalExtensions">Paths to additional extensions.</param>
/// <param name="skipExtensionFilters">Skip extension filtering by name if true.</param>
void InitializeExtensions(
Expand All @@ -35,7 +35,7 @@ void InitializeExtensions(
/// <summary>
/// Discovers tests given a list of sources and some run settings.
/// </summary>
///
///
/// <param name="discoveryPayload">Discovery payload.</param>
/// <param name="disoveryEventsRegistrar">Discovery events registrar.</param>
/// <param name="protocolConfig">Protocol related information.</param>
Expand All @@ -47,7 +47,7 @@ void DiscoverTests(
/// <summary>
/// Runs tests given a list of sources and some run settings.
/// </summary>
///
///
/// <param name="testRunRequestPayLoad">Test run request payload.</param>
/// <param name="customTestHostLauncher">Custom test host launcher for the run.</param>
/// <param name="testRunEventsRegistrar">Run events registrar.</param>
Expand All @@ -61,7 +61,7 @@ void RunTests(
/// <summary>
/// Processes test run attachments.
/// </summary>
///
///
/// <param name="testRunAttachmentsProcessingPayload">
/// Test run attachments processing payload.
/// </param>
Expand All @@ -77,7 +77,7 @@ void ProcessTestRunAttachments(
/// <summary>
/// Starts a test session.
/// </summary>
///
///
/// <param name="payload">The start test session payload.</param>
/// <param name="testHostLauncher">The custom test host launcher.</param>
/// <param name="eventsHandler">The events handler.</param>
Expand All @@ -103,6 +103,11 @@ void StartTestSession(
/// </summary>
void CancelDiscovery();

/// <summary>
/// Cancels the current discovery request with discovery complete event handler
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
void CancelDiscoveryWithEventHandler();
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Cancels the current test run attachments processing request.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;
/// </summary>
public interface IParallelProxyDiscoveryManager : IParallelOperationManager, IProxyDiscoveryManager
{
/// <summary>
/// Indicates if user requested an abortion
/// </summary>
bool IsAbortRequested { get; set; }

/// <summary>
/// Handles Partial Discovery Complete event coming from a specific concurrent proxy discovery manager
/// Each concurrent proxy discovery manager will signal the parallel discovery manager when its complete
Expand All @@ -24,4 +29,14 @@ bool HandlePartialDiscoveryComplete(
long totalTests,
IEnumerable<TestCase> lastChunk,
bool isAborted);

/// <summary>
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// Enums for indicating discovery status of source
/// </summary>
public enum DiscoveryStatus
{
FullyDiscovered, // Indicates that source was fully discovered
PartiallyDiscovered, // Indicates that we started discovery of the source but something happened (cancel/abort) and we stopped processing it
NotDiscovered // Indicates the sources which were not touched during discovery
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ public interface IProxyDiscoveryManager
/// </summary>
void Abort();

/// <summary>
/// Aborts discovery operation with EventHandler.
/// </summary>
/// <param name="eventHandler">EventHandler for handling discovery events from Engine</param>
void Abort(ITestDiscoveryEventsHandler2 eventHandler);
nohwnd marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Closes the current test operation.
/// Send a EndSession message to close the test host and channel gracefully.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,10 @@ public interface IDiscoveryManager
/// Aborts the test discovery.
/// </summary>
void Abort();

/// <summary>
/// Aborts the test discovery with eventHandler
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="eventHandler">EventHandler for handling discovery events from Engine</param>
void Abort(ITestDiscoveryEventsHandler2 eventHandler);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.IsAbortRequested.get -> bool
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.IsAbortRequested.set -> void
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IProxyDiscoveryManager.Abort(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.ITestDiscoveryEventsHandler2 eventHandler) -> void
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.TesthostProtocol.IDiscoveryManager.Abort(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.ITestDiscoveryEventsHandler2 eventHandler) -> void
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.DiscoveryStatus
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.DiscoveryStatus.FullyDiscovered = 0 -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.DiscoveryStatus
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.DiscoveryStatus.NotDiscovered = 2 -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.DiscoveryStatus
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.DiscoveryStatus.PartiallyDiscovered = 1 -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.DiscoveryStatus
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ public interface ITestRequestSender : IDisposable
/// </summary>
void SendTestRunAbort();

/// <summary>
/// Send the request to abort the discovery
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
void SendDiscoveryAbort();

/// <summary>
/// Handle client process exit
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ private JsonSerializer GetPayloadSerializer(int? version)
// unless this is disabled by VSTEST_DISABLE_PROTOCOL_3_VERSION_DOWNGRADE
// env variable.
0 or 1 or 3 => s_payloadSerializer,
2 or 4 or 5 => s_payloadSerializer2,
2 or 4 or 5 or 6 => s_payloadSerializer2,
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
_ => throw new NotSupportedException($"Protocol version {version} is not supported. " +
"Ensure it is compatible with the latest serializer or add a new one."),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,19 @@ public class DiscoveryCompletePayload
/// Gets or sets the Metrics
/// </summary>
public IDictionary<string, object> Metrics { get; set; }

/// <summary>
/// Gets or sets list of sources which were fully discovered
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public IReadOnlyCollection<string> FullyDiscoveredSources { get; set; } = new List<string>();
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets or sets list of sources which were partially discovered (started discover tests, but then discovery aborted)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public IReadOnlyCollection<string> PartiallyDiscoveredSources { get; set; } = new List<string>();

/// <summary>
/// Gets or sets list of sources which were not discovered at all
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public IReadOnlyCollection<string> NotDiscoveredSources { get; set; } = new List<string>();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces.ITestRequestSender.SendDiscoveryAbort() -> void
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.FullyDiscoveredSources.get -> System.Collections.Generic.IReadOnlyCollection<string>
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.FullyDiscoveredSources.set -> void
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.NotDiscoveredSources.get -> System.Collections.Generic.IReadOnlyCollection<string>
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.NotDiscoveredSources.set -> void
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.PartiallyDiscoveredSources.get -> System.Collections.Generic.IReadOnlyCollection<string>
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.PartiallyDiscoveredSources.set -> void
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TestRequestSender.SendDiscoveryAbort() -> void
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class TestRequestSender : ITestRequestSender

// Must be in sync with the highest supported version in
// src/Microsoft.TestPlatform.CrossPlatEngine/EventHandlers/TestRequestHandler.cs file.
private readonly int _highestSupportedVersion = 5;
private readonly int _highestSupportedVersion = 6;

private readonly TestHostConnectionInfo _connectionInfo;

Expand Down Expand Up @@ -295,6 +295,23 @@ public void DiscoverTests(DiscoveryCriteria discoveryCriteria, ITestDiscoveryEve

_channel.Send(message);
}

/// <inheritdoc/>
public void SendDiscoveryAbort()
{
if (IsOperationComplete())
{
EqtTrace.Verbose("TestRequestSender.SendDiscoveryAbort: Operation is already complete. Skip error message.");
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
return;
}

if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose("TestRequestSender.SendDiscoveryAbort: Sending discovery abort.");
}

_channel?.Send(_dataSerializer.SerializeMessage(MessageType.CancelDiscovery));
}
#endregion

#region Execution Protocol
Expand Down Expand Up @@ -594,7 +611,12 @@ private void OnDiscoveryMessageReceived(ITestDiscoveryEventsHandler2 discoveryEv
case MessageType.DiscoveryComplete:
var discoveryCompletePayload =
_dataSerializer.DeserializePayload<DiscoveryCompletePayload>(data);
var discoveryCompleteEventArgs = new DiscoveryCompleteEventArgs(discoveryCompletePayload.TotalTests, discoveryCompletePayload.IsAborted);
var discoveryCompleteEventArgs = new DiscoveryCompleteEventArgs(
discoveryCompletePayload.TotalTests,
discoveryCompletePayload.IsAborted,
discoveryCompletePayload.FullyDiscoveredSources,
discoveryCompletePayload.PartiallyDiscoveredSources,
discoveryCompletePayload.NotDiscoveredSources);
discoveryCompleteEventArgs.Metrics = discoveryCompletePayload.Metrics;
discoveryEventsHandler.HandleDiscoveryComplete(
discoveryCompleteEventArgs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ public void Abort()
Task.Run(() => _testHostManagerFactory.GetDiscoveryManager().Abort());
}

/// <inheritdoc/>
public void Abort(ITestDiscoveryEventsHandler2 eventHandler)
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
Task.Run(() => _testHostManagerFactory.GetDiscoveryManager().Abort(eventHandler));
}

private void InitializeExtensions(IEnumerable<string> sources)
{
var extensionsFromSource = _testHostManager.GetTestPlatformExtensions(sources, Enumerable.Empty<string>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel;

using Common.Telemetry;

using Microsoft.VisualStudio.TestPlatform.ObjectModel;

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;

using static Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager;

/// <summary>
/// ParallelDiscoveryDataAggregator aggregates discovery data from parallel discovery managers
/// </summary>
Expand Down Expand Up @@ -43,6 +47,20 @@ public ParallelDiscoveryDataAggregator()

#endregion

#region Internal Properties
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Dictionary which stores source with corresponding discoveryStatus
/// </summary>
internal ConcurrentDictionary<string, DiscoveryStatus> SourcesWithDiscoveryStatus = new();
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Indicates if discovery complete payload already sent back to IDE
/// </summary>
internal bool IsMessageSent { get; set; }

#endregion

#region Public Methods

/// <summary>
Expand Down Expand Up @@ -128,5 +146,57 @@ public void AggregateDiscoveryDataMetrics(IDictionary<string, object> metrics)
}
}

/// <summary>
/// Aggregate the source as fully discovered
/// </summary>
/// <param name="sorce">Fully discovered source</param>
internal void AggregateTheSourcesWithDiscoveryStatus(IEnumerable<string> sources, DiscoveryStatus status)
{
if (sources == null || sources.Count() == 0) return;
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

foreach (var source in sources)
{
if (status == DiscoveryStatus.NotDiscovered) SourcesWithDiscoveryStatus[source] = status;
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

if (!SourcesWithDiscoveryStatus.ContainsKey(source))
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
EqtTrace.Warning($"ParallelDiscoveryDataAggregator.AggregateTheSourcesWithDiscoveryStatus : " +
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
$"{source} is not present in SourcesWithDiscoveryStatus dictionary");
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
SourcesWithDiscoveryStatus[source] = status;

if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info($"ParallelDiscoveryDataAggregator.AggregateTheSourcesWithDiscoveryStatus : " +
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
$"{source} is marked with {status} status");
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

/// <summary>
/// Aggregate the value indicating if we already sent message to IDE
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="isMessageSent">Boolean value if we already sent message to IDE</param>
internal void AggregateIsMessageSent(bool isMessageSent)
{
IsMessageSent = IsMessageSent || isMessageSent;
}

/// <summary>
/// Returning sources with particular discovery status
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="status">Status to filter</param>
/// <returns></returns>
internal ICollection<string> GetSourcesWithStatus(DiscoveryStatus status)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
if (SourcesWithDiscoveryStatus == null || SourcesWithDiscoveryStatus.IsEmpty) return new List<string>();

return SourcesWithDiscoveryStatus.Where(source => source.Value == status)
.Select(source => source.Key).ToList();
}

#endregion
}
Loading