-
Notifications
You must be signed in to change notification settings - Fork 323
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
TestPluginCache and other dependencies refactoring #2537
TestPluginCache and other dependencies refactoring #2537
Conversation
…ng to create TestResults folder
…r path in that message
… be loaded from that assembly.Removed unnecessary FileHelper instance.
/azp run |
No pipelines are associated with this pull request. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
…/sayuzbas/test-plugin-cache
/azp run |
No pipelines are associated with this pull request. |
src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Properties/AssemblyInfo.cs
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
…/sayuzbas/test-plugin-cache
…/sayuzbas/test-plugin-cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have some measurements on this? I would like to see it do it's magic. How much time should this save? And how to best test it?
src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginDiscoverer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.ObjectModel/Utilities/TypesToLoadAttribute.cs
Outdated
Show resolved
Hide resolved
@nohwnd Do you mean measurements of using custom attributes? If assembly has custom attributes, on average it takes 1.5-2ms to get that "interesting" types, compared to 30ms which takes calling |
Yeah, just wanted the benefit to be documented here for future reference. :) |
…/sayuzbas/test-plugin-cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I have a nit-picking comment.
@@ -18,6 +17,6 @@ public TypesToLoadAttribute(params Type[] types) | |||
Types = types; | |||
} | |||
|
|||
public IEnumerable<Type> Types { get; } | |||
public Type[] Types { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer immutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IEnumerable, is also not immutable isn't it? But does it matter? How would this collection change to cause problems? We would need a static ctor to find it and modify it on assembly load?
Description
Refactoring of TestPluginCache and other classes responsible for test discovery and test execution
Fix : #2634