TestCentric can't use TestEngineActivator

[CharliePoole]

NUnit Compatibility Issue

Category: API
Visibility: Developers
Resolution: Unresolved

Description

The NUnit API includes a class, TestEngineActivator, which locates and instantiates a copy of the NUnit engine. Obviously, TestCentric cannot use this.

[ChrisMaddock]

I'd like to review this class for NUnit Engine v4, and make sure it works as we want. There's been some changes to the thinking behind instantiating engines since this was first written.

[CharliePoole]

I'm wondering if it even makes sense to have this class in the API assembly. I'm not sure, however, where it would go if it were not there.

Alternatively, it could take more input from the user about where to look for the actual engine, thereby allowing a substitution to be made.

I think it should definitely NOT know the name of the engine class or of the assembly in which it is located.

[ChrisMaddock]

I think it should definitely NOT know the name of the engine class or of the assembly in which it is located.

I think this depends on whether we see the API assembly as a two-way or one-way interface - which was a point I'm not sure we agreed on, last time we talked about it. 🙂

That said, I do agree it's inconsistent to have that amount of code in the API assembly. I'd be ok with slimming that code down to the bare minimum, and moving more of the logic to the engine.

[CharliePoole]

Well the last time we talked, I thought we were on opposite sides of the question. I was for plug-compatibility and you were not so sure. It seems to me that's one of the things people expect when you define an interface.

Here's what the API currently knows about loading the engine...

1. The assembly and file names
2. The ITestEngine interface
3. The TestEngine class
4. Rules for where to look for the engine
5. Minimum version of the engine required by the runner

This definitely feels like too much to me. For 1, 2 and 3 I would lean to making the names defaults, but allowing the user to provide them if needed. Items 4 and 5 are things we really have not made much use of up to now.

So I could see either very slim implementation or none at all - allowing runners to just create the engine through new.

[ChrisMaddock]

I definitely agree with it becoming a slimmer implementation.

The only points I'm less sure about are the extra constraints we put on the API assembly, by allowing it to become a two-way interface, and what the NUnit project would need to guarantee about that in terms of future maintainability. I'm not sure at this point what would be fair to guarantee. I don't want the NUnit project to end up either of the positions where:

a) We're further restricted in the progress we can make with engine development, or,
b) There is no guarantee of compatibility for 'implementors' of the interface, and every new minor version requires a raft of changes to a TestCentric engine.

Of course, this is a problem which goes away if we converge on a single engine. I wonder if we should instead be looking at that as our goal, and considering this an issue where we can't find a mutually suitable compromise?

[CharliePoole]

I see I ignored your one way / two way distinction rather than asking what you meant. 😄

So, the API seems to be "two-way" in the sense that the some (most) interfaces are implemented by the engine and used by the runner, while others (only a few) are defined in the API but implemented in the runner. So, ITestRunner vs ITestEventListener. But that's pretty normal for interfaces. Is that what you're talking about?

Or is it the potentially different overall goals you mentioned in another discussion... whether the API is intended to permit writing a drop-in replacement? If so, my confusion is that I would have thought that any API should allow that. If that's it, maybe give an example of something we might do in one case but not in the other and let's see if I catch on. 😄

[ChrisMaddock]

I'm talking about option 2 - the drop-in replacement. 🙂 Here's a nice real-world example...

Last week, I added a new method to IExtensionService - something like IExtension[] GetAllExtensions(). I did that because it was helpful for the NUnit Engine's testing and code structure, and seemed like a generally sensible method to expose. Right now, I can make that decision and change with little second thought.

If the TestCentric engine were to be an implementation of the same API however, I've just caused a breaking change for the TestCentric engine. My change now means that whenever TestCentric updates it's API package, the TestCentric engine also has to implement the method I just added for the NUnit Engine.

Does that help clarify the bit of process that I'm concerned about?

[CharliePoole]

I see what you mean. Originally, we thought that we could add methods to an interface without breaking compatibility. IIRC @jnm2 showed that isn't 100% true a while ago.

Still... I'm not getting it totally. The TestCentric engine is already an implementation of the same API. As such, if I update it to use a newer version, of course I have to implement new methods. But isn't that what we expect out of APIs? Can't I just choose not to update to the newer version if I like?

[CharliePoole]

Oh wait... what do we each mean by "drop in" replacement? Are you thinking "engine is guaranteed to work in place of the NUnit engine with any runner?" That's way more than I was thinking.

[ChrisMaddock]

@CharliePoole Does this remain an issue? If the goal is for the TestCentric GUI to use the NUnit Engine, I don't think this is an issue, is it? Or am I missing something here?

[CharliePoole]

Well, that goal is only achievable if we eliminate 100% of the incompatibility on one side or the other. So yes, if everything else becomes compatible, then this can easily be handled.

As a backup, maybe there should be an alternative way to do it? I can look at what I do now and see if it's generalizeable.

[CharliePoole]

Thinking about it, I guess this comes down to the overall question of what's visible in the test engine. We're already talking about simplifying the ITestEngine interface and making as much as possible of the implementation internal (for testing) or private.

So, here's an approach to instantiating the engine. Just make the class and it's constructor public. Make all other methods and properties internal, exposing those that need to be exposed in the interface. Anyone using the engine would simply reference it and construct an engine but would then use it through the interface.

This was not workable back when the idea was that the engine might be separate from the runner that uses it, but all our runners are tied to a copy of the engine that's present with them. If we wanted to make the engine drop-in-replaceable, we could stay away from new but just activate it where found... likely in the same directory as the runner, but that could be left up to the runner. In fact, maybe that's a better option.

Generalizing this...

• Only classes we want users to construct would be public, along with their constructors
• Runners would still be be constructed through an interface call.
• Services would be made accessible the same way.
• Only small classes that hold data (TestPackage, e.g.) would be fully public
• If a runner needed something like TestEngineActivator for flexibility, they would implement it themselves.

WRT the compatibility project, this approach would permit both the 100% solution, where I go back to using the NUnit engine but would also allow for an 80% solution, where I derive from and extend the engine but remain compatible with the interfaces.

Does this seem like a reasonable approach to replace TestEngineActivator? If so, I'll mark this as resolved.

[ChrisMaddock]

Would this require runners to have a direct reference to nunit.engine.dll? That would then become the only reason for a runner to directly reference that dll right, as well as the API dll?

Absolutely agree with the concept, but how about keeping a very simple wrapper around this constructor in the nunit.engine.api.dll class, just so that runners only need to reference the one assembly?

[CharliePoole]

Transferred from GUI issue 581

[CharliePoole]

We are no longer maintaining compatibility with NUnit but the question of how we create the engine still remains.

[CharliePoole]

Fixed in the latest code although further improvements may be needed.

[CharliePoole]

🎉 This issue has been resolved in version 2.0.0-beta3 🎉

The release is available on:

• GitHub Release
• NuGet Package