Filter during discovery

[nohwnd]

Add filtering during discovery to be able to list only tests that pass the given filter.

[nohwnd]

@clairernovotny This is a draft implementation to solve microsoft/vstest#2273.

I implemented this in a way that has the smallest impact on your adapter but it has some trade-offs. I would like to know your opinion before I commit to implement this 100%. At the moment the functionality works, but there are few things that I don't like:

• the filtering is done after the batching because the batches are done on ITestCase while the filter can only filter TestCase, this means that the batch might become smaller during the filtering. Allowing the fiter to run before the batches would require moving the generating the test cases outside of SendExistingTestCases.

• IDiscoveryContext does not expose filter expression, so I used the same approach as MSTest which uses reflection. But luckily this should be hit only once per assembly so the perf impact is hopefully offset by not sending all test cases when we use filter. I don't like using reflection here, and will add the filtering to IDiscoveryContext at some point, which will unfortunately mean publishing IDiscoveryContext2, because we need to support runtimes that don't understand default interface implementations, so I don't want to make that change before proper analysis 😔

• The biggest problem here are knownTraits. Known traits will become part of SupportedProperties, and during Run they are loaded from the list of all tests in the assembly. During discovery we have no such list because we need to filter the tests as they come and send them out in batches. In this implementation I chose to inspect the knownTraits always during discovery, but I think it would also work if the hashSet was a live reference and I added to it as we discover more and more traits. I did not do it in this implementation because passing a hashset in via constructor and then subsequently updating it seemed a bit fragile.

How the changes look to you?

[ViktorHofer]

cc @bradwilson

[bradwilson]

This looks okay to me.

the filtering is done after the batching because the batches are done on ITestCase while the filter can only filter TestCase, this means that the batch might become smaller during the filtering.

I suspect this isn't worth rearranging the order of things to solve, so I'd leave it to be done later if it's discovered that we really need it.

[bradwilson]

The biggest problem here are knownTraits.

I'm not 100% sure we should be trying to do validation during discovery anyway. You really don't want to interrupt discovery if you can help it, from a usability standpoint. Frankly, the fact that the discovered list suddenly becomes empty should be a pretty good sign you have a bad trait filter. 😉

[nohwnd]

@bradwilson I probably did not explain myself well with the trait filter. What I meant is that in the line above the filter checks for known traits. If this hashset is empty it will not consider any traits. I can either skip the Contains check as I do now, and make the filtering a bit slower, OR add to the known traits all traits from the current batch, before processing filtering that batch.

The latter solution seems to be optimal, but I would rather add to the known traits via method, than by passing a live reference to a hashset, because that seems more obvious and less fragile. Do you agree?

[bradwilson]

I don't obsess too much with optimization like this unless it's demonstrably poor. I usually rely on the .NET or Roslyn team to come back and tell me where they've hit performance issues. 😂

[ViktorHofer]

@clairernovotny is it possible for you to release a new version with this change, anytime soon? thanks