Skip to content
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

Where should TestPackage be defined? #919

Open
CharliePoole opened this issue Mar 15, 2021 · 2 comments
Open

Where should TestPackage be defined? #919

CharliePoole opened this issue Mar 15, 2021 · 2 comments

Comments

@CharliePoole
Copy link
Contributor

Question for the team for V4...

Currently, TestPackage is defined in the nunit.engine.api assembly. I'm working on issue #889 to remove some confusion about the constructor for TestPackage and simplify some other things I'm working on.

But it seems to me that there's a bigger question... Should TestPackage really be defined as a class in this assembly, which is made up almost entirely of interfaces?

So what are the alternatives? I can think of two:

  1. Define an ITestPackage interface in the api assembly and define TestPackage itself in the engine, probably in engine.core. That would mean giving the user some new interface (maybe a service) to create test packages.
  2. Define an ITestPackage interface that exposes all the functionality we need and have the user define it in their application.

Or we could keep doing it as we now are.

Throwing this out there for consideration, since we are planning on changing the engine interface anyway.

@ChrisMaddock
Copy link
Member

Interesting point. I'm not sure I can come up with a reason to feel particularly strongly either way.

I agree its inconsistent, however TestPackage is such a fundamental class I almost feel like it's allowed to be inconsistent? From my memory there's only two classes a runner should instantiate - TestPackage and TestEngine (Indirectly through TestEngineActivator).

Other than consistency, I'm not sure I can think of a significant advantage to putting TestPackage behind an Interface - I believe the exposed methods are quite limited, and I can't think of a particular need to mock it in any way.

My only thinking is that changing this would be a breaking change for even the simplest of runners. No problem at all with making breaking changes in 4.0, but I'm less sure on making such a fundamental one unless we have a significant need to do so.

@ChrisMaddock
Copy link
Member

Here's a thought - will this be affected at all by our plans to split up the API assembly for extensions to help with the versioning?

We talked about whether we'd need a nunit.engine.api.code.dll assembly. If we have a concrete TestPackage class, that would be big enough and change enough regularly that we definitely would, and also require all the associated extension.api.dll assemblies to be rereleased to reflect those changes. If we had a super simple ITestPackage interface, it wouldn't.

You might be onto something here, Charlie! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants