nunit / nunit-project-loader

The NUnit project loader extension allows the NUnit engine to load nunit projects
MIT License
2 stars 6 forks source link

NUnit project loader requires nunit.engine.api to be present but nuget package does not provide it #55

Closed CharliePoole closed 3 months ago

CharliePoole commented 3 months ago

The package contains only the nunit-project-loader assembly and works with the console runner because nunit.engine.apil is a dependency of the runner itself. Up to now, all NUnit extensions have been distributed this way since they were not intended to be used in any other environment.

I found this to be a problem when trying to use the project loader extension in running tests under the TestCentric Gui runner, which does not reference the nunit engine api.

There are various ways I could solve this on the testcentric side but I'm asking here first in order to find out if you consider use of the extension in other environments than the console runner a feature you want to support. At one time we had discussed loading extensions from nunitlite as well, for example.

I'd appreciate a quick answer on that point and if you want to support it I could provide a PR. If not, I'll either create my own package or figure out some way to reference the nunit engine api without creating naming conflicts.

CharliePoole commented 3 months ago

Adding to the problem... the extension does not specify the path for its extension point. All our original extensions rely on the host being able to figure out where to install them based on the Type of the extension. This is done by searching all extension nodes. Maybe I can figure out how to do this by mapping the engine api interface name to the equivalent TestCentric name.

@OsirisTerje if you guys could come back with some comments, it would help. If you're willing to support use of the extensions for other runners, then I'll do some PRs. If not, I'll do something else. FWIW, I think either decision is valid but should probably be stated somewhere. Like on this issue. :-)

OsirisTerje commented 3 months ago

It makes sense to me to support it.

CharliePoole commented 3 months ago

OK. I'll start with NUnit Project Loader. Testing this requires a build on MyGet so I'll clone into the TestCentric org, where I have authority to publish it.

Of the existing NUnit-published extensions, two include the api assembly and three don't. So I doubt it will do any harm to include it in the project loader, but we'll find out from testing it.

I'll also add a path to the extension. Anyone wanting to use it can simply duplicate that path in their own application.

CharliePoole commented 3 months ago

So... can of worms here... The dev version of nunit4-console used for testing this extension, 4.0.0-dev00051, is no longer available. The default myget retention rule is to keep the last 20 versions and it looks as if nobody has been working on this since I last did, two years ago.

For 4.0, what's available is dev00137 through dev00164. I tried the last and the first and neither of those work. I downloaded a clone of that project and I'll take a look.

I do notice that there has been no successful merge since a year ago.

CharliePoole commented 3 months ago

Probably should pin whatever dev versions the console is dependent on.

OsirisTerje commented 3 months ago

You should work against the version3X branch. That holds the latest release which is 3.17, based off the 3.15.X series.
The fixes in 3.16.X is slowly being migrated into the version3X branch, and trying to keep whatever crashes in the 3.16.X off that.
When that is done, I'll do the same with the 4.X, move that into a version4 branch and rename version3X to main, probably at some time jump it up to version 5.
Neither version 3.16.X nor 4 works as they should, and I haven't had time to figure out why.

CharliePoole commented 3 months ago

I didn't see that, so I started in main. I'll take a look at version3X, which may be easier for me. In that case, I'd have to make a version 3 branch for the project loader as well, starting from the 3.7.1 tag.

My local main for nunit-console is almost running - still having failures in the msi package tests. I'll hold on to those changes for now, and maybe submit them later.

One question... For main, i eliminated all the .netcoreapp2.1 builds rather than suppressing the errors. Is that what you prefer? For version3X as well?

OsirisTerje commented 3 months ago

Yes, there should be no .netcoreapp2.1, netcoreapp3.1 should be the lowest now

PS: I know there are some 2.1 remains here and there.

CharliePoole commented 3 months ago

@OsirisTerje Here's a fundamental problem with project loader extensions.

in the version 3 console runner, the presence of the --project setting implies that the agent may need to run multiple test assemblies. In version 4, I eliminated that setting and each assembly runs in a separate process. Therefore, any project must be broken down to its individual assemblies before launching the agent.

The consequences for the project loader are...

Frankly, I had not thought this through completely till now. I think the conclusion is that the project loader extensions under version 4 will be incompatible with version 3 - a breaking change.

IOW, I think that we need to have V4 project loaders unless you want to drop them in v4, which I believe was Rob's plan.

I think there could be some logic in dropping the VS project loader, since the user can run projects under the dotnet command. The extisting VS loader doesn't work terribly well anyway.

OTOH, it seems to me that NUnit projects are still an important feature for some users. For TestCentric, I decided to keep supporting them, even though I have a native format. I think it's important for NUnit as well, but it would have to be a v4 release because of the breaking changes.

CharliePoole commented 3 months ago

@OsirisTerje I'm ready to do a PR for this issue but I'm unsure as to where it should be merged.

I created the branch based on the 3.7.1 release tag, so we don't want to merge it to main as it now stands. As per my previous comment, I think we need separate version 3 and version 4 branches here. I see two ways to go...

  1. Create a version3 branch based on the 3.7.1 tag and merge to that.
  2. Rename main to version4, make a new main based on 3.7.1 and merge to that.

Option 2 has an extra step but it seems close to the approach you are planning to take for nunit-console.

I can carry out either approach if you want to give me the authority in GitHub. Otherwise, you can do one of them and then I'll create a PR.

OsirisTerje commented 3 months ago

@CharliePoole Option 2.

I'm on the road for three days now (going back to Oslo) , so best if you do this. I've given you Maintain role.

PS: For each significant change added to the new main branch, I'll need to verify that the adapter still works with the engine at that point. If anything goes awry, I can check this on a commit-by-commit basis.

CharliePoole commented 3 months ago

@OsirisTerje I'll be glad to do it but you gave me the role in the console/engine repo only. I'll need to be a maintainer in the nunit-project-loader repo as well for this particular issue.

CharliePoole commented 3 months ago

@OsirisTerje For extensions, back when I was working on NUnit, I used the following "rules"...

This was all automated in the package tests. Before the PR on this issue, for example, the nunit project loader was constantly tested against versions 3.10, 3.11.2 and 3.12. After the PR is merged that will change to 3.15.5 and 3.17.0.

On the console side, there is a list of latest versions of all the nunit-published extensions. There were once package tests for all of them but some got commented out due to persistent problems and might need fixing.

It seems to me that your point about checking that the adapter works could be handled in the same way. That is, the engine should have a package / acceptance test that proves it still works with the latest adapter release. The adapter, of course, already verifies that it keeps working with the engine version it is using. Probably, we should discuss this on an issue for the engine project.

OsirisTerje commented 3 months ago

@charliepoole

I'll need to be a maintainer in the nunit-project-loader repo as well for this particular issue.

Done