Eliminate In-Process Execution

[CharliePoole]

Allowing the user to select in-process execution gives no specific benefit although it has the side-effect of making debugging easier. We have discussed removing it in version 4.0. This issue is created as a place for the @nunit/engine-team to discuss the two options I see for this change. I've listed some notes on each option.

Option 1 - Just eliminate In-process execution

1. This is a test runner, which has to deal with code that may behave badly. It's really not reasonable to expect the runner to survive and report all bad behavior by the tests if the tests are running in-process.
2. The test itself benefits by being isolated in a separate process.
3. The change would simplify the engine because it no longer requires verifying that a particular assembly is capable of running in the same process as the runner, based on its target runtime.
4. The main use of in-process is to allow easier debugging, so loosing the option will make debugging something we need to focus on. There is already a way to debug the tests inside the agent process, which we should work to improve.

Option 2 - Eliminate the ProcessModel setting entirely

1. If we drop in-process execution, then all tests run in separate assemblies. The only remaining issue is whether multiple assemblies run in the same process or in separate processes. It's arguable that there is no reason for running in one process. That setting is only available for historical reasons: it was developed before we developed multiple process execution.
2. As with AppDomains, the user has no need to control how the runner allocates assemblies to processes.
3. Multiple Process execution (one process per assembly) is now the default. It's how the engine actually works best.
4. We could simply present Agents (rather than processes) to the user as the main conceptual entity used in running tests. The runner can decide in many cases what those agents are - processes, separate machines, etc.

[ChrisMaddock]

I'm rather attached to --inprocess, but like you say - almost entirely for debugging reasons, which isn't a good reason to keep a feature! I so almost wonder if it's worth considering how much complexity is added by retaining the ability to run tests in process, purely for debugging reasons.

I'm not sure I fully understand the history enough here to make a sensible decision. Why did the project originally give users such a range of choices over how to isolate their test assemblies? Is there a sensible requirement for being able to run inprocess and/or multiple test assemblies in a single agent process? Would be interested in other people's opinions here.

[ChrisMaddock]

There's a good point raised on TestCentric/TestCentric-Gui#444. How do runners like the VS adapter, which are already launched in a test-assembly-specific-process, cope with the removal of inprocess? Do then end up launching a process to launch a process to launch a third process? Sounds like unnecessary overhead, especially to click and run a single test. Maybe inprocess has a specific performance-based usecase for fast-feedback GUI runners, but we should consider limiting use through e.g. the console runner.

[CharliePoole]

@ChrisMaddock Good question about the adapter. I think we need @OsirisTerje involved in this discussion.

Do we really know that the process launched by VS is specific to the test assembly? I know it was not that way originally. Frankly, if I were writing an adapter today, I'm not sure I'd use the engine at all. If I did, I'd probably link it directly and come into it at a low level that bypasses the top levels of the engine.

That said, Terje is working on his own V4.0, so maybe we should coordinate. 😄

[CharliePoole]

I think the project (at the time I) just gave all those options because it was possible, without too much consideration of maintenance or sustainability. 😕 In fairness, running in a separate process was initially experimental and later multiple processes was an experimental feature and even later parallel multiple processes. At each step, we just kept the old options as well.

Here's how I'm now thinking. You want to run a test. You need to run it somewhere. That somewhere is an agent, which is most likely a process but as a user you don't care. You just want an agent with some capabilities, like being able to run .Net Core 3.1 tests.

As now implemented, btw, we don't start a new process each time you select a single test. The entire assembly is loaded in a process with which the runner communicates. Currently, you only need to reload if the number of agents is less than the number of assemblies but even that is something I think we could fix.

[OsirisTerje]

It is the vstest host that controls this, the adapter grabs some information and forwards that to the engine/framework. Whether removing inprocess may harm that, I am unsure - I do see that we have datacollectors that might need inprocess - if I get that right. I think someone from MS should answer on this part - e.g. @kendrahavens.

About V4 and if the adapter should call lower levels: Yes, it would be cool - the V4 now have a separate internal engine adapter, so it can handle changes in how the adapter interacts with the engine without affecting the rest of adapter code - to a certain degree - there are at least some more separation there now.

[ChrisMaddock]

As now implemented, btw, we don't start a new process each time you select a single test. The entire assembly is loaded in a process with which the runner communicates.

This is accurate for how the engine was intended to be used, and how runners like TestCentric use it, but I don't believe it is for runners like the VS adapter (in GUI form) and ReSharper, which load the assembly for each individual user-click, AFAIK?

Of course we can argue that both pieces of software should be using the engine as intended - but I think there's a balance to be struck in terms of considering the constraints of two of the most popular runners in the ecosystem. 🙂

[CharliePoole]

@OsirisTerje IIRC you load the engine assembly itself, which is part of the adapter package, so you have access to classes within the engine beyond the published API. That would seem to mean that you can run tests any way you choose, so long as we keep the Types and Methods you need public.

That said, and as I mentioned in another note, it seems to me that the entire engine is more than you really need. Maybe you should just be using the engine core or an adapter you create over it. The main purpose of the full engine is to decide where tests will be run and I think that VS really makes that decision for you anyway.

[ChrisMaddock]

I'm not sure I agree with that approach sorry, unless we create another supported interface at a lower level of the engine. Terje and I have both lost time over the years to strange issues caused by the adapter integrating with the engine at a lower level than the main API. For v4, I was hoping we would make all other methods internal, and only allow integrating through the main API.

[CharliePoole]

@ChrisMaddock

Not 100% sure what "that approach" refers to, but if it relates to using the core directly, I think there are things there that need to be exposed in an interface that we support. I'm not sure how you would write a pluggable agent, for example, without that.

Regarding "strange issues" I guess that's after my time on the adapter, so I defer to the two of you. Some of that stuff I know came from doing special things for Microsoft's benefit.

But what's the outcome of this comment? Do we mark the issue of eliminating in-process execution as blocked? That would be too bad, IMO.

[ChrisMaddock]

Not 100% sure what "that approach" refers to, but if it relates to using the core directly,

Yes, it did sorry. Exposing additional things through the interface seems like just the write approach to me however, and I'd expect something like pluggable agents to involve that. 🙂 I'm just keen we have a single supported interface, which we can maintain

But what's the outcome of this comment? Do we mark the issue of eliminating in-process execution as blocked? That would be too bad, IMO.

I think we need to ensure a few things committing to this:

1. The Adapter can function while running out of process.
2. Other major runners (e.g. ReShaper) won't be impacted.
3. We have a solution to continue easily debugging the engine.

On the other hand, would pluggable agents potentially give us a clean solution to running inprocess, so we could maintain the functionality, but remove the complexity? We mentioned that briefly on the TestCentric PR.

[CharliePoole]

I'm satisfied personally WRT 1 and 3. What does it take to resolve them from your point of view?

WRT 2, we have no idea how ReSharper works. They are not a company that shares information readily. And the adapter is called the Visual Studio adapter. So again, how will we resolve this without actually doing it?