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

Should we continue to build NUnit.ConsoleRunner.NetCore? #1045

Closed
CharliePoole opened this issue Dec 1, 2021 · 28 comments
Closed

Should we continue to build NUnit.ConsoleRunner.NetCore? #1045

CharliePoole opened this issue Dec 1, 2021 · 28 comments
Labels
Duplicate Feature High Priority NetCoreRunner Issue related to the .NET Core console runner

Comments

@CharliePoole
Copy link
Collaborator

Now that NUnit.ConsoleRunner is able to run .NET Core tests, it may be useful to focus our efforts there and stop producing a console runner with reduced capabilities, which runs under .NET Core 3.1.

That's not to say we shouldn't look toward a future in which the console runs under a more modern runtime, like .NET 6.0 for example. But keeping the 3.1 build working and having to deal with issues about it's limitations seems to only slow down our progress.

Let's discuss this and make a decision and plan for .NET 4.0.

@mikkelbu
Copy link
Member

mikkelbu commented Dec 9, 2021

This is probably quite telling for how much I've missed in the console project, but until I saw the release notes of 3.13 I had not understod that it was now possible to run .NET Core tests using the NUnit.ConsoleRunner. Since this is the case, then I think we should stop building NUnit.ConsoleRunner.NetCore (and probably also make more promotion for version 3.13).

@rprouse
Copy link
Member

rprouse commented Dec 10, 2021

I'm for stopping building it. Personally, I run all my tests with the adapter and dotnet test these days, but I might switch back now 😄

@CharliePoole
Copy link
Collaborator Author

@mikkelbu That was the one of the last bits of work I did on the console runner master branch, back in May. It wasn't announced because it was not released until now.

If we're agreed to stop producing it, that means a number of issues related to it can be closed.

@jnm2 We haven't heard from you on this yet and since it's a big decision... :-)

@CharliePoole
Copy link
Collaborator Author

I added requests for feedback on this to all the issues I could find referencing the .NET Core runner and also posted on Gitter. If anyone can think of other channels where this should be posted, please do so.

@Prodigio
Copy link

@CharliePoole asked me for some feedback. Since I don't use the .NET tool, but only the NUnit.Engine NuGet package, I do not have a strong opinion on this. If it's slowing down your development progress, I'd consider dropping it.

@Learfin
Copy link

Learfin commented Dec 14, 2021

@CharliePoole, regarding #916: we will switch to NUnit.ConsoleRunner 3.13.0 (we are using NUnit.ConsoleRunner.NetCore 3.13.0-dev-05308). It’s ok for me if you want to drop it.

@CharliePoole
Copy link
Collaborator Author

@nunit/engine-team I just invented a new label. If folks don't like it, I'll remove it but I think it's useful.

We have good visibility of all issues, which are blocked. Seems like we want even better visibility of any issue, which is the cause of other issues being blocked. So... "blocking"

@CharliePoole
Copy link
Collaborator Author

@jnm2 PING!

@CharliePoole
Copy link
Collaborator Author

I put this issue in the .NET 4.0 milestone originally, but now I'm thinking that no longer updating an existing program may not necessarily be a breaking change. What do y'all think @nunit/engine-team ?

@jnm2
Copy link
Collaborator

jnm2 commented Dec 14, 2021

My answer is let's stop building it. And I don't think it can be considered a breaking change, based on how I understand breaking changes to be defined (= obtaining a different version and getting an unwanted change along with it).

@CharliePoole
Copy link
Collaborator Author

Moving this to hypothetical release 3.14... (I have both 3.13.1 and 3.14 as possible next releases, depending on what stuff gets done first)

@CharliePoole CharliePoole modified the milestones: 4.0, 3.14 Dec 14, 2021
@bouchraRekhadda
Copy link

@CharliePoole, regarding #939, as @Learfin mentioned we have switched to NUnit.ConsoleRunner 3.13.0. It's OK on my side as well.
Thank you

@k15tfu
Copy link
Contributor

k15tfu commented Dec 17, 2021

Hi! We also use NUnit.ConsoleRunner.NetCore on our CI, specifically we run dotnet nunit3-console.dll path/to/assembly, need to check how to use it now. We are also preparing some patches to fix isolation issues with ACLs, btw.

@NikolayPianikov
Copy link
Member

Hi. I guess we could stop building it, we could add a detailed description somewhere on how to replace it with a different approach. I can provide recommendations related to TeamCity CI.

@k15tfu
Copy link
Contributor

k15tfu commented Dec 20, 2021

@CharliePoole @ChrisMaddock @NikolayPianikov Hi again! I'm not sure if this will be a smooth switch for us, we have many tests running on Windows under .NET Framework, and on Linux/macOS under .NET 6 using nunit3-console.dll from NUnit.ConsoleRunner.NetCore package (with roll-forward enabled). In addition, sometimes we have to deal with custom assembly resolving on Linux/macOS, because at the moment we build them under .NET Framework due to our limitations, and having managed entry point help us a lot. I guess the current approach is to use dotnet test with nunit adapter, but we definitely need time to investigate the possibility of using it instead of NUnit.ConsoleRunner.NetCore. I see that the PR is ready, but can we postpone the end of support and not remove it now?

@CharliePoole
Copy link
Collaborator Author

@k15tfu Can you explain why the proposed substitution of NUnit.ConsoleRunner for NUnit.ConsoleRunner.NetCore doesn't work for you?

@nunit/engine-team Let's hold off on actually merging for a bit, but I'd still appreciate the code review. :-)

@stokescompchurch
Copy link

I have been using nunit.consolerunner.netcore 3.12.0-beta2 with the dotnet-tools.json to run it from the commandline and that has been exactly what I need. The feature that I like it the ability to run test lists from a text file. As long as that feature gets to nunit console runner 4 I will like it.
Does this mean that NUnit.ConsoleRunner will support .Net6?

@CharliePoole
Copy link
Collaborator Author

@stokescompchurch and others...

I want to call attention to the description of this issue, beginning "Now that NUnit.ConsoleRunner is able to run .NET Core Tests...", which is the key to understanding what we are discussing.

Before 3.13, NUnit.ConsoleRunner and NUnit.ConsoleRunner.NetCore were separate runners. The first could only run .NET Framework tests, while the second could only run .NET Core tests. Now, with 3.13, NUnit.ConsoleRunner can run both types of tests... separately or in combination... while NUnit.ConsoleRunner.NetCore is still only able to run .NET Core tests.

So it seems to us that NUnit.ConsoleRunner.NetCore is no longer needed. However, we do need users to try out NUnit.ConsoleRunner with their .NET Core tests and let us know if we are correct!

Thanks all for your feedback!

@Prodigio
Copy link

Prodigio commented Dec 22, 2021

However, we do need users to try out NUnit.ConsoleRunner with their .NET Core tests and let us know if we are correct!

I can confirm, they do run with NUnit.ConsoleRunner. We have a project using only .NET Core 3.1, with more than 4000 tests, and they are (most of the time) green. ;)

@CharliePoole
Copy link
Collaborator Author

One big difference... NUnit.ConsoleRunner is not a dotnet tool. You can't install with the dotnet command, for example. My recommended install approach is to use Chocolatey, but that's not everyone's cup of tea. :-)

@k15tfu
Copy link
Contributor

k15tfu commented Dec 23, 2021

@CharliePoole For now I can name a few reasons: 1. Mono is not officially supported on macOS ARM64 so we cannot use NUnit.ConsoleRunner there, unlike nunit3-console.dll (from NUnit.ConsoleRunner.NetCore) which we already succesfully use on M1, and 2. Periodically we run into some issues in Mono runtime (e.g. mono/mono#19098, mono/mono#21371, mono/mono#21249, and mono/mono#21001 are open) so we plan to migrate to .NET Core and prefer not to use Mono where possible.

Moreover, Mono is now one of .NET runtimes, wouldn't it be better to run nunit3-console under .NET Core by default, and communicate with an agent to run .NET Framework tests under Mono on Linux/macOS?

@CharliePoole
Copy link
Collaborator Author

@k15tfu Indeed... that would be better in the long run. In the short run, we had hoped to get rid of this particular build, because it can only run .NET Core tests and doesn't handle engine extensions fully.

@nunit/engine-team Let's hold up on merging this for a bit. I'll change the PR to a draft.

@CharliePoole
Copy link
Collaborator Author

After some team discussion at https://github.com/orgs/nunit/teams/engine-team/discussions/11, we have decided:

  1. We need to continue to have a command, which runs on .NET Core and allows running .NET Core tests. Otherwise, folks can't run tests on some platforms.
  2. It needs to either be a tool or we need a separate tool, which wraps that command. We'll need to look into it further to decide which way to go.

We also don't know whether the best approach to developing such a tool is to continue with the existing NUnit.ConsoleRunner.NetCore package or to start over. Currently, we use the same code base to run tests using a .NET Framework application versus a .NET Core app. We may need to do a skunkworks project in order to decide.

Meanwhile, PR #1052, which was intended to drop the NetCore package is marked as a draft. We'll try to keep it somewhat up to date as other changes come along and hopefully make a decision in January.

@CharliePoole CharliePoole added the NetCoreRunner Issue related to the .NET Core console runner label Jan 1, 2022
@CharliePoole
Copy link
Collaborator Author

@nunit/engine-team I'd like to resolve this issue with some kind of statement of what we intend to do. Not sure where to put such a statement, however. :-)

Since we don't yet have a future plan for running on systems with only .NET Core available, I think we should (for now) have a policy of limited support for the net core runner:

  1. Fix bugs
  2. Some minor enhancements
  3. No new features or major enhancements

Item 3 would seem to preclude support for extensions and running in a separate process for the time being.

We should come up with a longer range plan sometime in the first quarter or so and then (depending on what it is) either retire the netcore runner or start building it up with more features.

@CharliePoole
Copy link
Collaborator Author

One side-effect of having the two runners be builds of the same source, is that we use the same assembly name for both. There are issues where it's not immediately clear which runner is being used. We could change the name of the executable. Any ideas?

@mikkelbu
Copy link
Member

mikkelbu commented Jan 1, 2022

@nunit/engine-team I'd like to resolve this issue with some kind of statement of what we intend to do. Not sure where to put such a statement, however. :-)

Possible locations:

  • Add the conclusion to the first comment in this issue and perhaps also in
    • the readme of this project
    • the nunit.console-runner.netcore.nuspec file

One side-effect of having the two runners be builds of the same source, is that we use the same assembly name for both. There are issues where it's not immediately clear which runner is being used. We could change the name of the executable. Any ideas?

As long as the runner is only for NetCore, then I don't mind including NetCore in the name for the time being

@CharliePoole
Copy link
Collaborator Author

Keeping the runner at least for the 3.14 release. We'll discuss further for the next release.

@CharliePoole CharliePoole removed this from the 3.14 milestone Jan 11, 2022
@CharliePoole
Copy link
Collaborator Author

Issue #1101 has replaced this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate Feature High Priority NetCoreRunner Issue related to the .NET Core console runner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants