ConsoleRunner spawns too many agents about nunit-console

[CharliePoole]

https://github.com/nunit/nunit-console/milestone/2 😄

[shift-evgeny]

@CharliePoole this is a problem with 3.4.1. I tried the latest master after finding that #1712 was fixed after 3.4.1 and found the same behaviour, namely: running with --agents=4 (or --agents=X) starts 13 agent EXEs (when testing 13 assemblies), though it's true that only X of them are using the CPU at any given time. The same thing happens with and without --dispose-runners. (None of my test classes implement IDisposable, by the way.)

What I would expect to happen is that only X agent processes exist at any given time. I don't care whether NUnit re-uses the same agent process or one process exits and a new one starts - that's an implementation detail. The issue is the amount of memory all these agent processes take, which is already slowing our build servers down a lot with 13 assemblies, but with 20 or more will become completely unmanageable. Then we would probably be forced to use --process=Separate.

By the way, what's the difference between --process=Separate and --process=Single ? They both seem to start only one agent process.

[CharliePoole]

@shift-evgeny I promise you that it worked at one time! But that fix broke some other combinations of options and the change to fix them ended up breaking this again. I'm trying to figure out how to create a test for this so it doesn't break again. Of course, I could do an integration-style test easily, but I'd prefer to do some mocking and just have a unit test verifying that we don't try to load assemblies before we run them.

It's interesting to consider the distinction between a feature and an implementation detail. When I created --agents I was trying to solve a problem of CPU utilization, so whether the processes existed or not was a minor detail for me. I just didn't want them all to run tests at the same time!

Considering memory, it's clear we don't even want the processes to exist simultaneously. That's what we will achieve when this is again fixed.

Someone, someday will probably want to minimize the process creation overhead and then the number of times we create and destroy a process will become a feature to implement. Not today, however, since that's a bit complicated. Each process you created would have to be tagged in terms of the .NET framework version targeted as well as it's bitness in order to decide whether to reuse it.

Regarding the other options: --process=Separate creates a single process and agent to run all tests while --process=Single runs them all in the NUnit process, without creating any agent. The latter option is poorly named and I added --inprocess as a hopefully clearer synonym.

[rprouse]

The fix was post-3.4.1 in nunit/nunit#1718, but I think it got broken again in nunit/nunit#1736 where some of the changes were undone to fix bugs that were introduced by nunit/nunit#1718.

[CharliePoole]

@shift-evgeny Based on @rprouse 's comment, it looks like you should be able to use the 3.5.0-dev-03113 build until I get this fixed.

[shift-evgeny]

@CharliePoole where can I find that?

[elbaloo]

@shift-evgeny https://ci.appveyor.com/project/CharliePoole/nunit/build/3.5.0-dev-03113

[shift-evgeny]

@CharliePoole thanks, I tested that and confirmed that --agents limits the number of agent processes, as expected! Could we get that fix in a release soon, please?

[CharliePoole]

@shift-evgeny I just had to change this code based on another issue. Would you test using the latest dev build, which is currently 3.5.0-dev-03158, available on Appveyor or from MyGet.

[shift-evgeny]

@CharliePoole I only see builds of nunit.framework.dll and nunitlite.dll there, not nunit3-console.exe, nunit.engine.api.dll, etc. Could you confirm which assembly is supposed to contain the fix?

[CharliePoole]

@shift-evgeny I guess you looked on AppVeyor for the nunit project. However, we just split the engine and console into a separate repo called nunit-console. See https://ci.appveyor.com/project/CharliePoole/nunit-console/build/3.5.0-dev-03158

For myget, see https://www.myget.org/feed/Packages/nunit

[shift-evgeny]

OK, tested it and it still works - the number of agents is limited to 3 with --agents=3, as I expect. So when do you expect to publish a release that includes this fix?

[shift-evgeny]

(I replaced only the files in NUnit.ConsoleRunner.3.5.0-dev-03158.nupkg, so I hope that's what contained the fix. Other files are still from the earlier build.)

[CharliePoole]

The 3.5 release is scheduled for the end of September. If there's a critical bug fix to release, we could include this one as well, but I don't see any issue right now that would lead to that.

[CharliePoole]

@shift-evgeny Did that include nunit.engine.dll? That's what has changed.

[shift-evgeny]

Yes, nunit.engine.dll was in the package. So looks like it's all good. OK, we'll wait for 3.5 then.

[Trass3r]

Tested 3.5.0-ci-03195-pr-52 and then 3.5.0-dev-03179 (master d31ebef) from https://ci.appveyor.com/project/CharliePoole/nunit-console/build/artifacts.
The former did not work while the latter does 👍
Only #43 left.

[CharliePoole]

FYI, only the dev suffix builds have actually made it to master. PR builds are "hypothetical" builds of master.

[shift-evgeny]

I see NUnit 3.5 has been released, but is there a release of NUnit Console that contains this fix? If not, when is it expected?

[CharliePoole]

We are working on the nunit-console release right now and you may see it today. Note that in future, releases may not come out at the same time and version numbers will probably get out of sync. Separating releases was the main reason for splitting repositories.

[shift-evgeny]

I just downloaded and tested the 3.5.0 release and with 17 assemblies and --agents=3 I get 17 nunit-agent.exe processes starting up straight away, but only 3 at a time are using any CPU. They seem to exit as tests in an assembly finish running. This is an improvement on the old behaviour where all processes kept running until the end, but still, it takes extra time at start-up and extra memory. What I'd expect is that --agents=X means only X agent processes exist at any given time.

[sonicflare]

I can confirm this too.
Version from this https://ci.appveyor.com/project/CharliePoole/nunit-console/build/artifacts worked like a charm, while Nunit.Console 3.5 from nuget is having the old behaviour

[shift-evgeny]

This really needs a unit test. It seems to keep getting fixed and re-broken.

[CharliePoole]

What a bummer! I'll check to see how this happened.

@shift-evgeny We had unit tests for it. But for the final proof, we need a true integration test.

[hbsis-fernandomedeiros]

Setting up a CI environment, this feature is much appreciated :)

[shift-evgeny]

Thanks! So what release will contain this fix and when is it expected to be released?

[ChrisMaddock]

@shift-evgeny - This will be in 3.6, which I believe is planned for January, cc @rprouse?