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

Jump to bottom

Wait for Discovery to initialize before Cancelling it #5177

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Wait for Discovery to initialize before Cancelling it #5177

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
194 changes: 110 additions & 84 deletions src/vstest.console/TestPlatformHelpers/TestRequestManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ internal class TestRequestManager : ITestRequestManager
/// Assumption: There can only be one active discovery request.
/// </summary>
private IDiscoveryRequest? _currentDiscoveryRequest;
/// <summary>
/// Guards cancellation of the current discovery request, by resetting when the request is received,
/// because the request needs time to setup and populate the _currentTestRunRequest. This might take a relatively
/// long time when the machine is slow, because the setup is called as an async task, so it needs to be processed by thread pool
/// and there might be a queue of existing tasks.
/// </summary>
private readonly ManualResetEvent _discoveryStarting = new(true);

/// <summary>
/// Maintains the current active test run attachments processing cancellation token source.
Expand Down Expand Up @@ -163,106 +170,119 @@ public void DiscoverTests(
ITestDiscoveryEventsRegistrar discoveryEventsRegistrar,
ProtocolConfig protocolConfig)
{
EqtTrace.Info("TestRequestManager.DiscoverTests: Discovery tests started.");

// TODO: Normalize rest of the data on the request as well
discoveryPayload.Sources = KnownPlatformSourceFilter.FilterKnownPlatformSources(discoveryPayload.Sources?.Distinct().ToList());
discoveryPayload.RunSettings ??= "<RunSettings></RunSettings>";

var runsettings = discoveryPayload.RunSettings;

if (discoveryPayload.TestPlatformOptions != null)
try
{
_telemetryOptedIn = discoveryPayload.TestPlatformOptions.CollectMetrics;
}
// Flag that that discovery is being initialized, so all requests to cancel discovery will wait till we set the discovery up.
_discoveryStarting.Reset();

var requestData = GetRequestData(protocolConfig);
if (UpdateRunSettingsIfRequired(
runsettings,
discoveryPayload.Sources.ToList(),
discoveryEventsRegistrar,
isDiscovery: true,
out string updatedRunsettings,
out IDictionary<string, Architecture> sourceToArchitectureMap,
out IDictionary<string, Framework> sourceToFrameworkMap))
{
runsettings = updatedRunsettings;
}
// Make sure to run the run request inside a lock as the below section is not thread-safe.
// There can be only one discovery or execution request at a given point in time.
lock (_syncObject)
{
EqtTrace.Info("TestRequestManager.DiscoverTests: Discovery tests started.");

var sourceToSourceDetailMap = discoveryPayload.Sources.Select(source => new SourceDetail
{
Source = source,
Architecture = sourceToArchitectureMap[source],
Framework = sourceToFrameworkMap[source],
}).ToDictionary(k => k.Source!);
// TODO: Normalize rest of the data on the request as well
discoveryPayload.Sources = KnownPlatformSourceFilter.FilterKnownPlatformSources(discoveryPayload.Sources?.Distinct().ToList());
discoveryPayload.RunSettings ??= "<RunSettings></RunSettings>";

var runConfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(runsettings);
var batchSize = runConfiguration.BatchSize;
var testCaseFilterFromRunsettings = runConfiguration.TestCaseFilter;
var runsettings = discoveryPayload.RunSettings;

if (requestData.IsTelemetryOptedIn)
{
// Collect metrics.
CollectMetrics(requestData, runConfiguration);
if (discoveryPayload.TestPlatformOptions != null)
{
_telemetryOptedIn = discoveryPayload.TestPlatformOptions.CollectMetrics;
}

// Collect commands.
LogCommandsTelemetryPoints(requestData);
}
var requestData = GetRequestData(protocolConfig);
if (UpdateRunSettingsIfRequired(
runsettings,
discoveryPayload.Sources.ToList(),
discoveryEventsRegistrar,
isDiscovery: true,
out string updatedRunsettings,
out IDictionary<string, Architecture> sourceToArchitectureMap,
out IDictionary<string, Framework> sourceToFrameworkMap))
{
runsettings = updatedRunsettings;
}

// Create discovery request.
var criteria = new DiscoveryCriteria(
discoveryPayload.Sources,
batchSize,
_commandLineOptions.TestStatsEventTimeout,
runsettings,
discoveryPayload.TestSessionInfo)
{
TestCaseFilter = _commandLineOptions.TestCaseFilterValue
?? testCaseFilterFromRunsettings
};
var sourceToSourceDetailMap = discoveryPayload.Sources.Select(source => new SourceDetail
{
Source = source,
Architecture = sourceToArchitectureMap[source],
Framework = sourceToFrameworkMap[source],
}).ToDictionary(k => k.Source!);

// Make sure to run the run request inside a lock as the below section is not thread-safe.
// There can be only one discovery or execution request at a given point in time.
lock (_syncObject)
{
try
{
EqtTrace.Info("TestRequestManager.DiscoverTests: Synchronization context taken");
var runConfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(runsettings);
var batchSize = runConfiguration.BatchSize;
var testCaseFilterFromRunsettings = runConfiguration.TestCaseFilter;

_currentDiscoveryRequest = _testPlatform.CreateDiscoveryRequest(
requestData,
criteria,
discoveryPayload.TestPlatformOptions,
sourceToSourceDetailMap,
new EventRegistrarToWarningLoggerAdapter(discoveryEventsRegistrar));
discoveryEventsRegistrar?.RegisterDiscoveryEvents(_currentDiscoveryRequest);
if (requestData.IsTelemetryOptedIn)
{
// Collect metrics.
CollectMetrics(requestData, runConfiguration);

// Notify start of discovery start.
_testPlatformEventSource.DiscoveryRequestStart();
// Collect commands.
LogCommandsTelemetryPoints(requestData);
}

// Start the discovery of tests and wait for completion.
_currentDiscoveryRequest.DiscoverAsync();
_currentDiscoveryRequest.WaitForCompletion();
}
finally
{
if (_currentDiscoveryRequest != null)
// Create discovery request.
var criteria = new DiscoveryCriteria(
discoveryPayload.Sources,
batchSize,
_commandLineOptions.TestStatsEventTimeout,
runsettings,
discoveryPayload.TestSessionInfo)
{
// Dispose the discovery request and unregister for events.
discoveryEventsRegistrar?.UnregisterDiscoveryEvents(_currentDiscoveryRequest);
_currentDiscoveryRequest.Dispose();
_currentDiscoveryRequest = null;
}
TestCaseFilter = _commandLineOptions.TestCaseFilterValue
?? testCaseFilterFromRunsettings
};

EqtTrace.Info("TestRequestManager.DiscoverTests: Discovery tests completed.");
_testPlatformEventSource.DiscoveryRequestStop();

// Posts the discovery complete event.
_metricsPublisher.Result.PublishMetrics(
TelemetryDataConstants.TestDiscoveryCompleteEvent,
requestData.MetricsCollection.Metrics!);
try
{
EqtTrace.Info("TestRequestManager.DiscoverTests: Synchronization context taken");

_currentDiscoveryRequest = _testPlatform.CreateDiscoveryRequest(
requestData,
criteria,
discoveryPayload.TestPlatformOptions,
sourceToSourceDetailMap,
new EventRegistrarToWarningLoggerAdapter(discoveryEventsRegistrar));
// Discovery started, allow cancellations to proceed.
_currentDiscoveryRequest.OnDiscoveryStart += (s, e) => _discoveryStarting.Set();
discoveryEventsRegistrar?.RegisterDiscoveryEvents(_currentDiscoveryRequest);

// Notify start of discovery start.
_testPlatformEventSource.DiscoveryRequestStart();

// Start the discovery of tests and wait for completion.
_currentDiscoveryRequest.DiscoverAsync();
_currentDiscoveryRequest.WaitForCompletion();
}
finally
{
if (_currentDiscoveryRequest != null)
{
// Dispose the discovery request and unregister for events.
discoveryEventsRegistrar?.UnregisterDiscoveryEvents(_currentDiscoveryRequest);
_currentDiscoveryRequest.Dispose();
_currentDiscoveryRequest = null;
}

EqtTrace.Info("TestRequestManager.DiscoverTests: Discovery tests completed.");
_testPlatformEventSource.DiscoveryRequestStop();

// Posts the discovery complete event.
_metricsPublisher.Result.PublishMetrics(
TelemetryDataConstants.TestDiscoveryCompleteEvent,
requestData.MetricsCollection.Metrics!);
}
}
}
finally
{
_discoveryStarting.Set();
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -624,6 +644,12 @@ public void CancelTestRun()
public void CancelDiscovery()
{
EqtTrace.Info("TestRequestManager.CancelDiscovery: Sending cancel request.");

// Wait for discovery request to initialize, before cancelling it, otherwise the
// _currentDiscoveryRequest might be null, because discovery did not have enough time to
// initialize and did not manage to populate _currentDiscoveryRequest yet, leading to hanging run
// that "ignores" the cancellation.
_discoveryStarting.WaitOne(3000);
_currentDiscoveryRequest?.Abort();
}

Expand Down