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 37 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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class DesignModeClient : IDesignModeClient
private readonly ICommunicationManager _communicationManager;
private readonly IDataSerializer _dataSerializer;

private readonly ProtocolConfig _protocolConfig = ObjectModel.Constants.DefaultProtocolConfig;
private readonly ProtocolConfig _protocolConfig = Constants.DefaultProtocolConfig;
private readonly IEnvironment _platformEnvironment;
private readonly TestSessionMessageLogger _testSessionMessageLogger;
private readonly object _lockObject = new();
Expand Down
16 changes: 13 additions & 3 deletions src/Microsoft.TestPlatform.Client/Discovery/DiscoveryRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ namespace Microsoft.VisualStudio.TestPlatform.Client.Discovery;
public sealed class DiscoveryRequest : IDiscoveryRequest, ITestDiscoveryEventsHandler2
{
private readonly IDataSerializer _dataSerializer;
internal /* for testing purposes */ readonly ProtocolConfig _protocolConfig = Constants.DefaultProtocolConfig;
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Initializes a new instance of the <see cref="DiscoveryRequest"/> class.
Expand Down Expand Up @@ -70,7 +71,7 @@ public void DiscoverAsync()
{
if (_disposed)
{
throw new ObjectDisposedException("DiscoveryRequest");
throw new ObjectDisposedException(nameof(DiscoveryRequest));
}

// Reset the discovery completion event
Expand Down Expand Up @@ -112,12 +113,21 @@ public void Abort()
{
if (_disposed)
{
throw new ObjectDisposedException("DiscoveryRequest");
throw new ObjectDisposedException(nameof(DiscoveryRequest));
}

if (DiscoveryInProgress)
{
DiscoveryManager.Abort();
// If testhost has old version, we should use old cancel logic
// to be consistent and not create regression issues
if (_protocolConfig.Version < Constants.MinimumProtocolVersionWithCancelDiscoveryEventHandlerSupport)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
DiscoveryManager.Abort();
}
else
{
DiscoveryManager.Abort(this);
}
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,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 @@ -37,7 +37,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 @@ -49,7 +49,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 @@ -63,7 +63,7 @@ void RunTests(
/// <summary>
/// Processes test run attachments.
/// </summary>
///
///
/// <param name="testRunAttachmentsProcessingPayload">
/// Test run attachments processing payload.
/// </param>
Expand All @@ -79,7 +79,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 Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;

/// <summary>
/// Enums for indicating discovery status of source
/// </summary>
public enum DiscoveryStatus
{
/// <summary>
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
/// Indicates the sources which were not touched during discovery.
/// </summary>
NotDiscovered,

/// <summary>
/// Indicates that we started discovery of the source but something happened (cancel/abort) and we stopped processing it.
/// </summary>
PartiallyDiscovered,

/// <summary>
/// Indicates that source was fully discovered.
/// </summary>
FullyDiscovered,
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;
/// </summary>
public interface IParallelProxyDiscoveryManager : IParallelOperationManager, IProxyDiscoveryManager
{
/// <summary>
/// Indicates if user requested an abortion
/// </summary>
bool IsAbortRequested { get; }

/// <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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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 @@ -31,4 +31,10 @@ public interface IDiscoveryManager
/// Aborts the test discovery.
/// </summary>
void Abort();

/// <summary>
/// Aborts the test discovery with eventHandler.
/// </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
Expand Up @@ -6,3 +6,10 @@ static Microsoft.VisualStudio.TestPlatform.Common.Telemetry.TelemetryDataConstan
static Microsoft.VisualStudio.TestPlatform.Common.Telemetry.TelemetryDataConstants.StopTestSessionCompleteEvent -> string
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IProxyTestSessionManager.StartSession(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.ITestSessionEventsHandler eventsHandler, Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.IRequestData requestData) -> bool
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IProxyTestSessionManager.StopSession(Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.IRequestData requestData) -> bool
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.IParallelProxyDiscoveryManager.IsAbortRequested.get -> bool
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
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.DiscoveryStatus
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.DiscoveryStatus.NotDiscovered = 0 -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.DiscoveryStatus
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.DiscoveryStatus.PartiallyDiscovered = 1 -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.DiscoveryStatus
Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.DiscoveryStatus.FullyDiscovered = 2 -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.DiscoveryStatus
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ public interface ITestRequestSender : IDisposable
/// </summary>
void SendTestRunAbort();

/// <summary>
/// Sends the request to abort the discovery.
/// </summary>
void SendDiscoveryAbort();

/// <summary>
/// Handle client process exit
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,15 @@ private JsonSerializer GetPayloadSerializer(int? version)
version = 1;
}

// 0: the original protocol with no versioning (Message). It is used during negotiation.
// 1: new protocol with versioning (VersionedMessage).
// 2: changed serialization because the serialization of properties in bag was too verbose,
// so common properties are considered built-in and serialized without type info.
// 3: introduced because of changes to allow attaching debugger to external process.
// 4: introduced because 3 did not update this table and ended up using the serializer for protocol v1,
// which is extremely slow. We negotiate 2 or 4, but never 3 unless the flag above is set.
// 5: ???
// 6: accepts abort and cancel with handlers that report the status.
return version switch
{
// 0 is used during negotiation.
Expand All @@ -216,9 +225,9 @@ 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,
_ => throw new NotSupportedException($"Protocol version {version} is not supported. " +
"Ensure it is compatible with the latest serializer or add a new one."),
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."),
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,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.
/// </summary>
public IList<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).
/// </summary>
public IList<string> PartiallyDiscoveredSources { get; set; } = new List<string>();

/// <summary>
/// Gets or sets list of sources which were not discovered at all.
/// </summary>
public IList<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.IList<string>
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.FullyDiscoveredSources.set -> void
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.NotDiscoveredSources.get -> System.Collections.Generic.IList<string>
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.NotDiscoveredSources.set -> void
Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel.DiscoveryCompletePayload.PartiallyDiscoveredSources.get -> System.Collections.Generic.IList<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 @@ -60,7 +60,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 @@ -276,6 +276,19 @@ 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;
}

EqtTrace.Verbose("TestRequestSender.SendDiscoveryAbort: Sending discovery abort.");
_channel?.Send(_dataSerializer.SerializeMessage(MessageType.CancelDiscovery));
}
#endregion

#region Execution Protocol
Expand Down Expand Up @@ -543,7 +556,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 @@ -97,6 +97,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 @@ -7,6 +7,9 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel;

using Common.Telemetry;

using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Discovery;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
Expand Down Expand Up @@ -37,6 +40,17 @@ public ParallelDiscoveryDataAggregator()
/// </summary>
public long TotalTests { get; private set; }


/// <summary>
/// Dictionary which stores source with corresponding discoveryStatus
/// </summary>
private readonly ConcurrentDictionary<string, DiscoveryStatus> _sourcesWithDiscoveryStatus = new();

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

/// <summary>
/// Returns the Aggregated Metrics.
/// </summary>
Expand Down Expand Up @@ -120,4 +134,27 @@ public void AggregateDiscoveryDataMetrics(IDictionary<string, object> metrics)
}
}

/// <summary>
/// Aggregate the source as fully discovered
/// </summary>
/// <param name="sorce">Fully discovered source</param>
internal void MarkSourcesWithStatus(ICollection<string> sources, DiscoveryStatus status)
=> DiscoveryManager.MarkSourcesWithStatus(sources, status, _sourcesWithDiscoveryStatus);

/// <summary>
/// Aggregates the value indicating if we already sent message to IDE.
/// </summary>
/// <param name="isMessageSent">Boolean value if we already sent message to IDE</param>
internal void AggregateIsMessageSent(bool isMessageSent)
{
IsMessageSent = IsMessageSent || isMessageSent;
}

/// <summary>
/// Returns sources with particular discovery status.
/// </summary>
/// <param name="status">Status to filter</param>
/// <returns></returns>
internal List<string> GetSourcesWithStatus(DiscoveryStatus status)
=> DiscoveryManager.GetSourcesWithStatus(status, _sourcesWithDiscoveryStatus);
}
Loading