-
Notifications
You must be signed in to change notification settings - Fork 326
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
Filtering sources based on assembly type #1537
Filtering sources based on assembly type #1537
Conversation
if (discoverer.Metadata.AssemblyType == AssemblyType.Native || | ||
discoverer.Metadata.AssemblyType == AssemblyType.Managed) | ||
{ | ||
assemblyTypeToSoucesMap = assemblyTypeToSoucesMap ?? GetAssemblyTypeToSoucesMap(sources); |
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.
spelling
* Correcting Tests
@@ -31,6 +34,7 @@ public class DiscovererEnumeratorTests | |||
private DiscoveryResultCache discoveryResultCache; | |||
private Mock<IRequestData> mockRequestData; | |||
private Mock<IMetricsCollection> mockMetricsCollection; | |||
private Mock<IAssemblyProperties> mockPEReaderHelper; |
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.
renaming mockPEReaderHelper to mockAssemblyProperties
} | ||
|
||
// Added this to make class testable, needed a PEHeader mocked Instance |
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.
Nit: needed a AssemblyProperties
or remove comment.
var result = new Dictionary<LazyExtension<ITestDiscoverer, ITestDiscovererCapabilities>, IEnumerable<string>>(); | ||
var sourcesForWhichNoDiscovererIsAvailable = new List<string>(sources); | ||
|
||
foreach (var discoverer in allDiscoverers) | ||
{ | ||
var sourcesToCheck = sources; | ||
|
||
if (discoverer.Metadata.AssemblyType == AssemblyType.Native || |
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.
Is there a chance discoverer can support both managed and native?
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.
No. adapter can't mention both
assemblyTypeToSoucesMap[assemblyProperties.GetAssemblyType(source)] : | ||
assemblyTypeToSoucesMap[AssemblyType.None]; | ||
|
||
((List<string>)sourcesList).Add(source); |
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.
Can we use IList
here?
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.
done
} | ||
finally | ||
{ | ||
this.ResetDiscoverers(); |
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.
Add this to cleanup.
Part 3 of RFC:
Contains changes for filtering sources based on assembly type.