Re-implement Reload in the engine

[CharliePoole]

Fixes #609

[CharliePoole]

Not sure why the Azure build is failing... seems unrelated.

[CharliePoole]

The implementation of Reload works in this PR provided a reload is possible. However, if settings are changed that cause it to require a different runner, the Reload will fail. For example, if you were to change the target runtime from .NET 2.0 to 4.5, while running in a separate process, the existing process would not be able to load the tests.

This makes the caller responsible for knowing whether it's possible to do a reload. That's how the GUI does it, by recognizing cases like the above that require an Unload / Load rather than a Reload.

I'm not sure this is a very good API however. Thoughts on how we should do this?

[mikkelbu]

Not sure why the Azure build is failing... seems unrelated.

@CharliePoole It is unrelated. I just triggered a build on master and that also failed. The failure is

2019-04-18T19:35:11.3863893Z D:\a\1\s\src\NUnitEngine\nunit.engine.tests\nunit.engine.tests.csproj :
error NU3005: Package 'System.Xml.XPath 4.3.0' from source
'C:\Program Files\dotnet\sdk\NuGetFallbackFolder':
The package signature file entry is invalid.
The central directory header field 'compression method' has an invalid value (8). [D:\a\1\s\NUnitConsole.sln]

which is similar to the problem we had on Appveyor in #556, but there we only solved the problem for mock-assembly.csproj, as this was restored first in appveyor.yml. So my guess is that Azure has been updated and that we now run into the same problem here (just in nunit.engine.tests.csproj). I'll create a PR for both builds.

[CharliePoole]

Here is our existing ITestRunnerFactory.CanReuse method, pretty much as inherited from V2. It only deals with changes to the process model andis never called.

nunit-console/src/NUnitEngine/nunit.engine/Services/DefaultTestRunnerFactory.cs

Line 128 in 8ca1ac7

// TODO: Review this method once we have a gui - not used by console runner

In NUnit V2, it is called by TestLoader in the ReloadTests method. If it returns false, then a new runner is created, changing the reload to a load.

So what do we want to do when the user asks us to reload after changing some settings that make it impossible to reuse the same runner? We could do as V2 does or just go ahead and let it error out.

[CharliePoole]

I added a description of where I'd like to take this next in #609. I'll wait for feedback before proceeding.

[CharliePoole]

Rebased to take advantage of @mikkelbu 's fix for the travis build.

[ChrisMaddock]

This makes the caller responsible for knowing whether it's possible to do a reload. That's how the GUI does it, by recognizing cases like the above that require an Unload / Load rather than a Reload.

I'm not totally keen on merging an implementation that differs so fundamentally from the behaviour we eventually want to implement...even though the current functionality is broken!

I assume your aim here is to implement a half-way house, until we can implement the "silent-full-reloads" you describe in #609? If that is the case, I think I'd be more comfortable with implementing a full reload every time for now, and then later enhancing that as a 'performance improvement'.

[CharliePoole]

Yeah... I actually meant that sentence to indicate that I didn't want to merge this as is, even though it would be an incremental improvement. 😄

I've done some further work, which goes a bit farther than you suggest. Since only the GUI currently uses this API I think it helps to get something in that works for most cases, so that new bugs can be discovered and worked on incrementally.

New commit coming "soon."

[CharliePoole]

Setting this aside for a few days while we move from Croatia to Sicily.

[ChrisMaddock]

Hi @CharliePoole - what's the status on this one? 🙂 If I'm reading correctly 'Reload' as a whole was never finished, but priorities have changed. Are we better closing this PR, and revisiting this at a later point?

[CharliePoole]

@ChrisMaddock I'll put this on my list as the next thing to check (and answer) when I reach a break point on some other work I'm doing.

[CharliePoole]

Replaced by PR #927