Skip to content

Fix datacollectors temporary files cleanup. #1483

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

Merged
smadala merged 3 commits into from
Mar 16, 2018
Merged

Fix datacollectors temporary files cleanup. #1483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f64c96
Call DataCollector dispose on AfterTestRunEnd
smadala Mar 16, 2018
c5cedc5
Merge branch 'master' into fix-itrace-files-issue
smadala Mar 16, 2018
61981f9
Merge branch 'master' into fix-itrace-files-issue
smadala Mar 16, 2018
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
20 changes: 20 additions & 0 deletions src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,32 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
CleanupPlugins();
}

this.disposed = true;
}
}

private void CleanupPlugins()
{
EqtTrace.Info("DataCollectionManager.CleanupPlugins: CleanupPlugins called");
Copy link
Contributor

@acesiddhu acesiddhu Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Info [](start = 21, length = 4)

and here


if (!this.isDataCollectionEnabled)
{
return;
}

if (EqtTrace.IsVerboseEnabled)
{
Copy link
Contributor

@acesiddhu acesiddhu Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we do this?

Copy link
Contributor Author

@smadala smadala Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid new string allocation.

EqtTrace.Verbose("DataCollectionManager.CleanupPlugins: Cleaning up {0} plugins", this.RunDataCollectors.Count);
}

RemoveDataCollectors(new List<DataCollectorInformation>(this.RunDataCollectors.Values));

EqtTrace.Info("DataCollectionManager.CleanupPlugins: CleanupPlugins finished");
Copy link
Contributor

@acesiddhu acesiddhu Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EqtTrace.Info [](start = 12, length = 13)

why not here then

Copy link
Contributor Author

@smadala smadala Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is compile time string. already created.

}

#region Load and Initialize DataCollectors

/// <summary>
Expand Down
202 changes: 85 additions & 117 deletions src/Microsoft.TestPlatform.CommunicationUtilities/DataCollectionRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,140 +168,29 @@ public void ProcessRequests()
{
var message = this.communicationManager.ReceiveMessage();

if (EqtTrace.IsVerboseEnabled)
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Verbose("DataCollectionRequestHandler.ProcessRequests : Datacollector received message: {0}", message);
EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : Datacollector received message: {0}", message);
}

switch (message.MessageType)
{
case MessageType.BeforeTestRunStart:
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : DataCollection starting.");
}

// Initialize datacollectors and get enviornment variables.
var settingXml = this.dataSerializer.DeserializePayload<string>(message);
this.AddExtensionAssemblies(settingXml);

var envVariables = this.dataCollectionManager.InitializeDataCollectors(settingXml);
var areTestCaseLevelEventsRequired = this.dataCollectionManager.SessionStarted();

// Open a socket communication port for test level events.
var testCaseEventsPort = 0;
if (areTestCaseLevelEventsRequired)
{
testCaseEventsPort = this.dataCollectionTestCaseEventHandler.InitializeCommunication();

this.testCaseEventMonitorTask = Task.Factory.StartNew(
() =>
{
try
{
if (
this.dataCollectionTestCaseEventHandler.WaitForRequestHandlerConnection(
DataCollectionCommTimeOut))
{
this.dataCollectionTestCaseEventHandler.ProcessRequests();
}
else
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info(
"DataCollectionRequestHandler.ProcessRequests: TestCaseEventHandler timed out while connecting to the Sender.");
}

this.dataCollectionTestCaseEventHandler.Close();
throw new TimeoutException();
}
}
catch (Exception e)
{
if (EqtTrace.IsErrorEnabled)
{
EqtTrace.Error(
"DataCollectionRequestHandler.ProcessRequests : Error occured during initialization of TestHost : {0}",
e);
}
}
},
this.cancellationTokenSource.Token);
}

this.communicationManager.SendMessage(
MessageType.BeforeTestRunStartResult,
new BeforeTestRunStartResult(envVariables, testCaseEventsPort));
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : DataCollection started.");
}

this.HandleBeforeTestRunStart(message);
break;

case MessageType.AfterTestRunEnd:
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DataCollection completing.");
}

var isCancelled = this.dataSerializer.DeserializePayload<bool>(message);

if (isCancelled)
{
this.cancellationTokenSource.Cancel();
}

try
{
this.testCaseEventMonitorTask?.Wait(this.cancellationTokenSource.Token);
this.dataCollectionTestCaseEventHandler.Close();
}
catch (Exception ex)
{
if (EqtTrace.IsErrorEnabled)
{
EqtTrace.Error("DataCollectionRequestHandler.ProcessRequests : {0}", ex.ToString());
}
}

var attachmentsets = this.dataCollectionManager.SessionEnded(isCancelled);
this.communicationManager.SendMessage(MessageType.AfterTestRunEndResult, attachmentsets);
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info(
"DataCollectionRequestHandler.ProcessRequests : Session End message received from server. Closing the connection.");
}

this.HandleAfterTestRunEnd(message);
isSessionEnded = true;
this.Close();

if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : DataCollection completed");
}

break;

case MessageType.TestHostLaunched:
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : Test host initialized.");
}

var testHostLaunchedPayload = this.dataSerializer.DeserializePayload<TestHostLaunchedPayload>(message);

this.dataCollectionManager.TestHostLaunched(testHostLaunchedPayload.ProcessId);

break;

default:
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : Invalid Message types");
}

EqtTrace.Error("DataCollectionRequestHandler.ProcessRequests : Invalid Message types: {0}", message.MessageType);
break;
}
}
Expand All @@ -325,7 +214,6 @@ public void SendDataCollectionMessage(DataCollectionMessageEventArgs args)
public void Dispose()
{
this.communicationManager?.StopClient();
this.dataCollectionManager?.Dispose();
}

/// <summary>
Expand Down Expand Up @@ -385,5 +273,85 @@ private void AddExtensionAssemblies(string runSettings)
}
}
}

private void HandleBeforeTestRunStart(Message message)
{
// Initialize datacollectors and get enviornment variables.
var settingXml = this.dataSerializer.DeserializePayload<string>(message);
this.AddExtensionAssemblies(settingXml);

var envVariables = this.dataCollectionManager.InitializeDataCollectors(settingXml);
var areTestCaseLevelEventsRequired = this.dataCollectionManager.SessionStarted();

// Open a socket communication port for test level events.
var testCaseEventsPort = 0;
if (areTestCaseLevelEventsRequired)
{
testCaseEventsPort = this.dataCollectionTestCaseEventHandler.InitializeCommunication();

this.testCaseEventMonitorTask = Task.Factory.StartNew(
() =>
{
try
{
if (this.dataCollectionTestCaseEventHandler.WaitForRequestHandlerConnection(
DataCollectionCommTimeOut))
{
this.dataCollectionTestCaseEventHandler.ProcessRequests();
}
else
{
EqtTrace.Error("DataCollectionRequestHandler.ProcessRequests: TestCaseEventHandler timed out while connecting to the Sender.");
this.dataCollectionTestCaseEventHandler.Close();
throw new TimeoutException();
}
}
catch (Exception e)
{
EqtTrace.Error("DataCollectionRequestHandler.ProcessRequests : Error occured during initialization of TestHost : {0}", e);
}
},
this.cancellationTokenSource.Token);
}

this.communicationManager.SendMessage(
MessageType.BeforeTestRunStartResult,
new BeforeTestRunStartResult(envVariables, testCaseEventsPort));

EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : DataCollection started.");
}

private void HandleAfterTestRunEnd(Message message)
{
var isCancelled = this.dataSerializer.DeserializePayload<bool>(message);

if (isCancelled)
{
this.cancellationTokenSource.Cancel();
}

try
{
this.testCaseEventMonitorTask?.Wait(this.cancellationTokenSource.Token);
this.dataCollectionTestCaseEventHandler.Close();
}
catch (Exception ex)
{
EqtTrace.Error("DataCollectionRequestHandler.ProcessRequests : {0}", ex.ToString());
}

var attachmentsets = this.dataCollectionManager.SessionEnded(isCancelled);

// Dispose all datacollectors before sending attachements to vstest.console process.
// As datacollector process exits itself on parent process(vstest.console) exits.
this.dataCollectionManager?.Dispose();

this.communicationManager.SendMessage(MessageType.AfterTestRunEndResult, attachmentsets);
EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : Session End message received from server. Closing the connection.");

this.Close();

EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : DataCollection completed");
}
}
}
1 change: 1 addition & 0 deletions test/Microsoft.TestPlatform.AcceptanceTests/DataCollectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ private void VaildateDataCollectorOutput()
this.StdOutputContains("Data collector 'SampleDataCollector' message: SessionEnded");
this.StdOutputContains("Data collector 'SampleDataCollector' message: my warning");
this.StdErrorContains("Data collector 'SampleDataCollector' message: Data collector caught an exception of type 'System.Exception': 'my exception'. More details:");
this.StdOutputContains("Data collector 'SampleDataCollector' message: Dispose called.");

// Verify attachments
var isTestRunLevelAttachmentFound = false;
Expand Down
10 changes: 10 additions & 0 deletions ...rosoft.TestPlatform.CommunicationUtilities.UnitTests/DataCollectionRequestHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,16 @@ public void ProcessRequestsShouldProcessRequests()
this.mockCommunicationManager.Verify(x => x.SendMessage(MessageType.AfterTestRunEndResult, It.IsAny<Collection<AttachmentSet>>()), Times.Once);
}

[TestMethod]
public void ProcessRequestsShouldDisposeDataCollectorsOnAfterTestRunEnd()
{
this.mockCommunicationManager.SetupSequence(x => x.ReceiveMessage()).Returns(new Message() { MessageType = MessageType.AfterTestRunEnd, Payload = "false" });

this.requestHandler.ProcessRequests();

this.mockDataCollectionManager.Verify(x => x.Dispose());
}

[TestMethod]
public void ProcessRequestsShouldThrowExceptionIfThrownByCommunicationManager()
{
Expand Down
5 changes: 5 additions & 0 deletions test/TestAssets/OutOfProcDataCollector/SampleDataCollector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,10 @@ public IEnumerable<KeyValuePair<string, string>> GetTestExecutionEnvironmentVari
{
return new List<KeyValuePair<string, string>> { new KeyValuePair<string, string>("key", "value") };
}

protected override void Dispose(bool disposing)
{
this.logger.LogWarning(this.context.SessionDataCollectionContext, "Dispose called.");
}
}
}