V2 Driver does not support id filters about nunit-v2-framework-driver

[CharliePoole]

This looks doable in the driver. I'll transfer the issue to that project and assign it to myself. Drivers will just need to deal with this filter type. It should not be hard since they have to generate ids anyway.

[CharliePoole]

@nunit/engine-team I've made a fix for this in the TestCentric project, to the extent that I"m reporting the error rather than failing silently. It would be cool if some of you would take a look at that PR (TestCentric/testcentric-gui#354) to see if it makes sense to port it here.

For a more final fix, either the V2 adapter or the V2 framework itself should be changed so that an id filter actually works. Then, anyone needing the correct behavior (currently only in the testcentric GUI) would simply upgrade the driver.

NOTE: if it's fixed in the legacy nunitv2 release, then the v2 driver would just get a package upgrade. If fixed in the driver, then the code to convert from an id filter to a testname filter would be in the driver itself.

I'd prefer not to work on this until I know which approach suits you guys best, since I could then provide the PR to NUnit. OTOH, I have to do something soon, since this is a 1.0 release issue.

[mikkelbu]

I don't think you are referring to the right PR, don't you mean TestCentric/testcentric-gui#354 ? Looking at the code I don't see a problem in porting it here (perhaps introducing a helper method to reduce the duplication of creating the attributes).

Regarding the final fix, then I think it should be solved in the framework, if the change is not too complex, but note that I know next to nothing about this part of the code. I see the solution in the adapter as the second option.

[CharliePoole]

@mikkelbu That's correct. I'll edit the description.

[ChrisMaddock]

I'm not familiar with the v2 driver, does it reference the framework itself? As opposed to how the v3 driver works?

If it was v3, I would have assumed the fix should be in the driver, so the engine will continue to work with all 3.x versions of the framework. Does the v2 driver work differently?

[CharliePoole]

The V2 driver uses two V2 assemblies, nunit.core and nunit.core.interfaces, but not the framework. Originally it had its own binary copies but now it uses a package from the nunit legacy project. When I say it could be fixed in the legacy release, I mean in nunit core, which is the part that works with all old versions of the framework.

I guess it makes the most sense to do this in the v2 driver if possible. It depends on whether the core code that needs to be called is all public, which I could only find out by trying.

[rprouse]

To me it seems like the driver is the best place to fix this since it is a V3 to V2 interface issue. Am I correct in thinking this would never be an issue with a legacy V2 runner?

[CharliePoole]

I'd say that's correct. Legacy runners can use the UniqueName of a test to identify it. That's what the V2 GUI does. I'll take a look at the driver. To me it's really a question of whether enough info is exposed by the V2 framework to allow the conversion. Basically, we need roundtrip conversion between V2 unique names and V3 IDs.

[CharliePoole]

@mikkelbu @ChrisMaddock @rprouse

OK, not as easily doable in the driver alone as I hadhoped. I'll run through the options I've tried.

1. It would be doable in the driver in one way: The driver would save an index to the loaded tree of tests, with the ID as key. Whenever an id filter was used, it would look up the test and then create an entirely different type of filter that selects that test alone. The V2 NameFilter would do the job. Once I saw that a complete search of the tree would be needed before even beginning to apply the filters, I rejected this approach.

2. I came up with a hybrid approach. The driver would create a NameFilter, with only runner id and test id. The fullname of the test would be omitted. NUnit core would be modified so that a null fullname matched everything. I tried this out and found it made a visible breaking change in nunit v2, so I think it's out as well.

3. Finally, I have this option... create an id filter for the V2 framework. It isn't breaking because it has never existed before. Only the V2 driver would use it, translating from NUnit 3 IdFilter to the new NUnit V2 IdFilter.

This is the best I can come up with, although it seems crazy to be adding a new "feature" to V2! What do you guys think?

[rprouse]

The third approach seems reasonable. It is really just down to if you want to do the work or not and how much you need it for the GUI. NUnit 3.7 has 65K downloads since last October which shows there is still some use, but the tide has definitely turned with 3.11 getting over 4.5 million downloads in a similar timeframe and the latest release getting five times the downloads in a month. You've been maintaining the v2 framework, so please do what you feel is best.

[CharliePoole]

Thanks. Amount of work is the least for 2, which breaks V2. Option 3 is next.

BTW, I asked because you all are the biggest consumer of NUnit V2. Each copy of the V2 framework driver gets the updated V2 core, including those bundled with NUnit.Console. In a similar period, there were about 1 million downloads of the extension alone and 4.2 million downloads of NUnit Console. Of course, we don't know how many of the latter are in actual use running V2 tests, but all those folks are getting the V2 legacy code.

Key problem is to add this without any negative impact on you, on those users and on the direct users of V2. Ideally, the tests will tell us that.