-
Notifications
You must be signed in to change notification settings - Fork 152
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
Simplify or remove loading of engine by reflection via TestEngineActivator #948
Comments
Having an API assembly does a few things...
There's no doubt that all of the above are potentially useful. The question is how useful and at what cost. |
This issue has been dormant for more than three years. At this point, it's clear that we either want to simplify TestEngineActivator significantly or eliminate it. I've changed the issue title accordingly. At one time, we were planning to allow drop-in replacement of the engine without updating any runners. This is not something we have ever implemented and there really doesn't seem to be a need for it. Generally, any runner comes with an engine build that matches it. This means that searching for the engine anywhere except in the same directory that contains the API is no longer useful. It also means that having a runner require a certain minimum version of the engine is redundant. It will use the engine that is packaged with it. The above-mentioned simplification is already in place, as a part of other changes we have made. TestEngineActivator simply creates an instance of ITestEngine without any other changes. The question to resolve here is whether to go to the next step and eliminate TestEngineActivator entirely. This would leave the choice up to the runner as to whether to only use the interface or to call the TestEngine class directly. |
@nunit/engine-team Options as I see them...
Option 2 requires either merging the API into the engine assembly or allowing the api to reference the engine. Merging the API into the engine assembly may also be considered in combination with options 1 and 3. |
Having just lost yet another day to weird reflection issues, I'd like to have a look at whether we can remove this structure for v4.0, inline with our "Improved Developer Experience" objective.
If I understand the history correctly, the reason this was implemented in v3 was with the expectation that users would install a single centralised engine, and have runners discover and make use of that. We've found instead that runners have chosen to each distribute their own local copy of the engine, meaning this has become a source of extra complication in the engine which isn't well used.
Having made a start on #885, I'm now looking at the below structure for the API assemblies:
nunit.engine.extensions.api.dll
ExtensionAttributes
ITestPackage
nunit.extension.project-loader.api.dll (And 4 other similar assemblies)
API for individual extensions. Broken into individual assemblies to allow us to make any necessary breaking changes in a less disruptive manner.
nunit.engine.api.dll
API's for runners to interact with the engine.
Once I'd got that far, I'm now thinking that nunit.engine.api.dll doesn't serve much purpose except to expose the public types from nunit.engine.dll.
How about we merge what's left of nunit.engine.api.dll into the nunit.engine assembly, and simply expose only the API types that we want people to use. (As we planned in #886 anyway). This would also mean that TestEngineActivator could simply create a TestEngine via the constructor, and we'd remove a good amount of the reflection complexity, impeded debugging ability and type loading complications we currently face.
What do people think? 🙂
The text was updated successfully, but these errors were encountered: