Skip to content

Fix datacollectors temporary files cleanup.#1483

Merged
smadala merged 3 commits intomicrosoft:masterfrom
smadala:fix-itrace-files-issue
Mar 16, 2018
Merged

Fix datacollectors temporary files cleanup.#1483
smadala merged 3 commits intomicrosoft:masterfrom
smadala:fix-itrace-files-issue

Conversation

@smadala
Copy link
Contributor

@smadala smadala commented Mar 16, 2018

Description

  • Call Datacollectors dispose before sending attachments to vstest.console.
  • Otherwise datacollector temporary files consuming disk space.

Related issue

microsoft/azure-pipelines-tasks#6694
microsoft/testfx#354 (comment)

@smadala smadala changed the title Call DataCollector dispose on AfterTestRunEnd Fix datacollectors temporary files cleanup. Mar 16, 2018
Copy link
Contributor

@mayankbansal018 mayankbansal018 left a comment

Choose a reason for hiding this comment

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

:shipit:

}

if (EqtTrace.IsVerboseEnabled)
{
Copy link
Contributor

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

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.


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

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

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

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.


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

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

Copy link
Contributor

@acesiddhu acesiddhu left a comment

Choose a reason for hiding this comment

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

:shipit:

@smadala
Copy link
Contributor Author

smadala commented Mar 16, 2018

@dotnet-bot please test Windows_NT / Debug Build

@smadala
Copy link
Contributor Author

smadala commented Mar 16, 2018

@dotnet-bot please test Windows_NT / Release Build

1 similar comment
@smadala
Copy link
Contributor Author

smadala commented Mar 16, 2018

@dotnet-bot please test Windows_NT / Release Build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants