Run after Reload reports assemblies multipletimes

[CharliePoole]

This is pretty strange! The following sequence of calls to the master test runner...

runner.Load();
runner.Reload();
var result = runner.Run(listener, TestFilter.Empty);

... returns a result in which the test assembly appears twice. If the reload is repeated, then the result will have an additional copy of the assembly for each repetition.

This only appears after a Run. The result of the reload will seem quite normal.

Note that we don't use Reload in any of our runners, which is why this has gone undetected up to now.

[CharliePoole]

Well... this seems to be due to the fact that Reload is basically not implemented in the engine... it just points to Load! I guess it was waiting for when we actually needed it.

[CharliePoole]

I'm outlining here how I'd like to finish the implementation of Reload. I'd appreciate comments from @ChrisMaddock @mikkelbu and other @nunit/engine-team members.

The ITestRunner interface has a Reload method. This is the only way that an external program can invoke reload. It's up to the internal code as to what Reload actually does.

I think we should externally define reload as "reloading all tests, preserving as much of the current state as possible."

The internal definition of Reload, as used in the ITestEngineRunner interface, is "reload all tests, using the existing runners."

The implication here is that an external call to Reload may or may not translate to an internal Reload depending on whether and how the package settings have changed. When an internal Reload is not possible, we should "re-load" using a new runner.

Situations where an internal Reload is not possible...

1. Settings have changed in such a way that different runner types are needed. A change in ProcessModel could lead to this situation.

2. Settings have changed so that the same process can no longer be used. A change in the target runtime would cause this as would a change in the X86 setting.

3. Settings have changed so that the same AppDomain can no longer be used. This would, for example, apply if test assemblies loaded in the same AppDomain subsequently needed to be assigned to different AppDomains.

In all of the above situations, the engine would simply do the best it could, performing an internal reload if possible but otherwise doing a full load. Other than performance differences, the result should not be distinguishable by the caller.

The current implementation, taken from NUnit V2 only deals with the first situation - changes to the ProcessModel. We should implement handling of the other two.

The current implementation does either a full reload or a full unload/load. Since we have a chain of runners involved, it's possible that we might be able to reuse some runners but not others. For example, if only the DomainUsage had changed, we should be able to reuse the same process and merely create new AppDomains and runners within that process. Currently, we would use an entire new process. We should eventually try to refine that, but it's probably not a high priority.

Since I'm motivated to get this done for use by the GUI, I'll mention that the GUI already has some code to only call for a reload when it knows a full reload is possible. Ideally that would be removed and the GUI would rely completely on the engine to do what's right.

[ChrisMaddock]

I was about to disagree with your current PR, but I agree with your comment here instead!

I agree that the engine should silently handle a call to reload, and silently workout whether a full load/unload is required, or just a reload of the test assemblies.

[CharliePoole]

Restarting work on this. Will do a simpler solution that eliminates the problem in the title without completely re-implementing Reload.