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

Split Engine Extensions into separarate projects #2

Closed
CharliePoole opened this issue Aug 15, 2016 · 7 comments
Closed

Split Engine Extensions into separarate projects #2

CharliePoole opened this issue Aug 15, 2016 · 7 comments

Comments

@CharliePoole
Copy link
Collaborator

@CharliePoole commented on Tue Jun 21 2016

It's conceivable that we will want to keep some extensions in the engine project, so the first step is to decide whether to split off all or some of them. Once this is done we can create separate issues.

Currently, we maintain five extensions:

  • nunit-project-loader
  • vs-project-loader
  • nunit-v2-result-writer
  • nunit v2 driver
  • teamcity-event-listener

The first four have been around since the beginning of NUnit 3.0 and represent fairly fundamental capabilities. They could go either way.

The Teamcity event listener is clearly something that should be separate, since it relates to a third-party project. It's probably our highest priority for splitting off.

NOTE: The VS loader is not actually product-specific, since the format is used by a number of IDEs in addition to Visual Studio.

NOTE: I'm calling this high priority with respect to the decision-making. Once that's done we can create separate issues for whatever has to be split off.


@alastairtree commented on Tue Jun 28 2016

It looks like this has been completed for the teamcity extenion and released as stable to nuget for 3.4? However the default behaviour of detecting teamcity no longer seems to work in 3.4.0, nor does the --teamcity console option. Both the docs and the -help conisole option list "--teamcity" as supported but the code suggests it is no longer. Could the docs/console help be updated to either support teamcity automatically as before or give information on how devs should use the new extension please as it seems like this will break lots of builds, mine included?

Thanks for the great library.


@CharliePoole commented on Tue Jun 28 2016

No, this has not yet happened. We did convert the teamcity code, which was previously internal, to an extension but it's still included with NUnit 3.4. Most likely you are running into issue #1623 (fixed in source) or #1626 (still in process). We will do a 3.4.1 release today or tomorrow.

If you are using the msi or one of the nuget packages that bundles common extensions, you have the teamcity extension automatically. If you are installing the NUnit.ConsoleRunner nuget package, which doesn't include extensions, then you will have to install NUnit.Extension.TeamCityEventListener as well. I'll add that info to the docs.


@CharliePoole commented on Tue Jun 28 2016

@alastairtree BTW, what this issue is about is developing framework, console, extensions, etc. separately and releasing updates on different schedules as needed. But we will still bundle collections of the latest versions in various ways for those who want it.


@CharliePoole commented on Tue Jul 19 2016

@rprouse @NikolayPianikov I'm moving ahead with creation of a separate repo for the teamcity extension. This will be a repo under the NUnit organization until we finsh all the work. Then we'll decide where it belongs.


@CharliePoole commented on Tue Jul 26 2016

The teamcity extension has been split off into issue #1702 - now ready for merge.

That leaves the four remaining "internal" extensions, built as part of the engine. We need to decide what their immediate and longer-term future should be.

The choices are basically per-extension but we shouldn't do different things for different extensions without a good reason...

  1. Continue to support these as we now do, building them as a part of the engine and distributing them with the engine as "standard" extensions.
  2. Separate the extensions into a different solution within the repo but continue to bundle them in certain packages. Note that they are already built in separate C# projects, with only a reference to the api assembly. This would simply create a greater degree of separation and would also force us to make our nunit tests run without any extensions present.
  3. Separate the extensions (or each extension) into a different repo with it's own solution and use the nunit.engine.api package to isolate them from the engine build. While we could do this and still maintain a common release schedule, it doesn't make much sense to do so. If all the standard extensions were in a single repo, then that repo could take responsibility for the packages that bundle extensions with the engine (NUnit.Runners and NUnit.Console).
  4. Possibly in combination with option 3, create a separate project for bundling the deconstructed pieces of NUnit software in various ways, based on the needs of different audiences. I'm pretty high on this as a longer-term goal and I'm considering starting it under my own GitHub account.

Please contribute your ideas here so we can set up specific steps to finish off this issue before splitting the framework and engine/console.


@ChrisMaddock commented on Wed Jul 27 2016

Charlie - can you run through what we'd be hoping to gain by splitting them up?

I think one solution per repo would be preferable when approaching a new project - whether than means we keep everything in one repo, or in many separate repos.


@CharliePoole commented on Wed Jul 27 2016

Chris - good question...

We have had an historical problem with NUnit, which arises because we started out small and grew to be OneBigProject. Things would often work in our NUnit tests, but not work for users. Sometimes, they would only work in the NUnit build itself!

With 3.0, I wanted to resolve this by ensuring that our own tests would run in an environment similar to that of our users. One aspect of this is to keep the various layers separate, both in the development stage and the testing stage.

When you are working in a solution that includes both the engine and it's extensions, it's very easy to create something that works when everything is in the same directory, or in known locations, but fails in the wild, when engine and extensions get updated at different times, are installed in different places, etc.

Further reasons for separating are to follow the principle of separating things that will be released at different times and things that will be worked on by different teams.

Here's a shot at showing the benefits of each approach... at least as I see it...

Benefit Option 1 Option 2 Option 3 Option 4
Separate release cycles No No Yes Yes
Separate teams No No Yes Yes
Reduced coupling No Some Yes Yes
Error reduction No No Yes Yes
Ease of install Yes Less Less Yes

@rprouse commented on Wed Jul 27 2016

In the short term, I think option 3 is a reasonable goal leading to a possible longer term goal of option 4.

I would like to see each extension in a separate repository with it's own version number. The engine w/ extensions NuGet package can take a dependency on each package with a version >= so it always pulls in updates. We can then only release extensions when there have been bugs reported and we can make fixes without a full NUnit release.

I also think that the extensions are likely fairly stable at this point, so hopefully they won't require much maintenance.

I think we need to pull them into the engine solution somehow though so we can continue to test the engine with extensions present. Integration testing during development if you like. We can either pull in the released NuGet packages, or CI builds.


@CharliePoole commented on Wed Jul 27 2016

I agree.

Many projects on GitHub have a separate integration testing repo - so that's something we could consider in the future as well - maybe even the fairly near-term future.

We could make it the responsibility of the extensions to test themselves with various engines - probably last release and current master. All this can be pulled in by packages. That's what we are settling on for the teamcity extension at any rate. But for the shorter term, I guess we should pull the separate extensions into the master build and have some tests for them. These would have to be integration tests though, not unit tests, because the unit tests do the same thing whether run in the extension project or the nunit project. Our V2 driver tests are an integration test - our only one for the extensions btw.


@ChrisMaddock commented on Wed Jul 27 2016

Thanks for running through it Charlie. Separate repositories sounds sensible.


@CharliePoole commented on Wed Jul 27 2016

Before moving further, I'm going to switch to the teamcity extension, which is now separated, and see if I can get it back into our NUnit.Runners package. The JetBrains guys feel that's important and we'll need a similar solution for other extensions.


@ChrisMaddock commented on Wed Jul 27 2016

Before moving further, I'm going to switch to the teamcity extension, which is now separated, and see if I can get it back into our NUnit.Runners package.

What were you planning with this? Would it make sense to have a single repo for the installer/Runners package. i.e. One place that manages "combined" distributions?


@CharliePoole commented on Wed Jul 27 2016

That's my thinking for "bundles" of things. However, that's more on the long-term side of it. Right now, I'm looking for instant gratification for the teamcity users.

What I'll probably try to do is set up the NUnit.Runners package to use the latest distribution of a set of well-known extensions >= some value for each. It's not perfect but will do for a while.

@CharliePoole
Copy link
Collaborator Author

@nunit/core-team @NikolayPianikov

Before moving ahead on this, I'd like to collect your ideas about any lessons learned from having separated the teamcity extension. My own thoughts include:

  • Make sure we have unit tests.
  • Extension should include integration tests that use the latest console/engine release
  • Console/engine should include integration tests that use the latest extension releases and run them before packaging.

Your thoughts?

Next step will be to pick the next extension to unbundle.

@CharliePoole CharliePoole added this to the 3.5 milestone Aug 29, 2016
@CharliePoole
Copy link
Collaborator Author

@nunit/core-team @NikolayPianikov
No comments guys? I'm moving ahead then.

@CharliePoole
Copy link
Collaborator Author

I'm removing extensions from the nunit-console project one at a time. As I remove them, the console package still has dependencies on them and assumes that their version is the same as that of the console. That won't always be true. The final step in completing this issue after all the extensions are removed will be to determine how we will maintain a list of the versions to use for package dependencies.

@aateeque
Copy link

aateeque commented Oct 1, 2016

@CharliePoole looks like the nuget package NUnit.ConsoleRunner.3.4.1 went from one bringing all the extensions to now being being one without extensions in the same version of the package. Is this true?

@rprouse
Copy link
Member

rprouse commented Oct 1, 2016

@aateeque if you look back through the versions of NUnit.ConsoleRunner, you will see that it never had dependencies and I don't think it ever included the extensions. The one you want is NUnit.Console, https://www.nuget.org/packages/NUnit.Console/

@CharliePoole I wonder if we should update the description of NUnit.ConsoleRunner to let people know it is probably not the version they want. We get way too many issues reported around this. I am going to create an issue for the 3.5 release to update the wording in the NuGet package.

@aateeque
Copy link

aateeque commented Oct 1, 2016

@rprouse spot on, NUnit.Console is the one I want. I think at one point the past couple of weeks NUnit.ConsoleRunner had dependencies; because I setup a project with it. But then this week they silently seemed to have disappeared, which was quite frustrating for our team. Updating the descriptions would be great help

@rprouse
Copy link
Member

rprouse commented Oct 1, 2016

I created issue #67 for this and put it in the milestone so it will be included in the upcoming release. It is an easy fix, we just need to decide on the wording. If you have ideas or suggestions @aateeque, please head over to #67 and give us your feedback 😉

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

No branches or pull requests

3 participants