Introduction of Microsoft.Extensions.DependencyModel causes reflection crashes

[OsirisTerje]

The engine version 3.16.3 is used in the NUnit3TestAdapter 4.4.0.
Vesion 3.16.2 introduced a new way of loading assemblies, including reading of the deps.json file, and this is using the Microsoft.Extensions.DependencyModel version 3.1.0.
When using .net 6 this crashes when the software under test is using reflection. There are several cases of this, reported in issues nunit/nunit3-vs-adapter#1065 and nunit/nunit3-vs-adapter#1066.
It may be that newer versions of this package, or related dependency injection packages, are not compatible with the version 3.1.0.

We need to either remove Microsoft.Extensions.DependencyModel and then rewrite the assembly loading, or ensure it works for all frameworks and for the different cases of reflection loading, as described in the issues referred to.

[CharliePoole]

@OsirisTerje Version 3.1.0 of Microsoft.Extensions.DependencyModel was the latest one I was able to make work under .NET Core 3.1, which is the latest build used by nunit.engine.core. My plan was to start building nunit.engine.core for .NET 6.0 in addition to the existing builds and to use a later version of the DependencyModel for that version. The older runtimes are still needed, of course, so that users can run tests under them if they need to.

It occurs to me that the versions used when running under Visual Studio may give you a hint as to what is needed, since this area of dependency loading is almost completely undocumented.

[OsirisTerje]

Yes, I assumed that could be the case, but then we need to add .net 6 and possibly .net 7 and .net 8 later, all depending on how much the Dependency packages from MS changes, and what breaking changes they may introduce.
Also, it might be a sideeffect but it seems that the loadingcontext might confuse possible casts.

Also, we should have [acceptance] tests that ensure it works on all .net versions. I would expect the 3.16.2 to also crash for the same repros, haven't tested that yet.

[rprouse]

Should we be including the correct version for each version of .NET Core here, 3f854b2#diff-b82806cbe6520a13c308f28d24d01cdbd2639024609d411d529b5022bbe9401bR30 instead of just using the 3.1.0 version for all? There is a version that matches each major .NET release, https://www.nuget.org/packages/Microsoft.Extensions.DependencyModel/#versions-body-tab

[manfred-brands]

I suggest to use the 7.0.0 version for all. It is compatible with netstandard2.0, net462 and net6.0. Assuming it works for all nunit's targets.

[OsirisTerje]

@manfred-brands @rprouse Awesome to just have that updated, and a 3.16.4 version out. We have repros to check it out , and there are nine people who have reported in about this. Their issues are all slightly different, so I hope it is just this package and not something else in the assembly loading that is failing. But adding in that package version could be a way to check it.

But, I assume adding it all to the nuspec file(s) are also needed.

[OsirisTerje]

Another issue of the same type with a good repro too: nunit/nunit3-vs-adapter#1069

[CharliePoole]

@manfred-brands It would be possible, of course, to stop supporting running under runtimes which have reached microsoft eol. That wasn't our policy in the past, although some have argued it should be. It definitely would make things easier for development, although not for users who may be forced by circumstances to run under those runtimes. A big decision for the core team, I think. :-)

[OsirisTerje]

The major issue here is the assembly loading. What was the reason for the change to use the Microsoft.Extensions.DependencyModel? Can that change be reverted?
That would simplify all.

[CharliePoole]

@OsirisTerje It was added to support loading packae dependencies when using the engine under runners other than the VS adapter. The adapter, of course, has no need for that, since the Microsoft environment loads dependencies on its behalf.

[OsirisTerje]

@manfred-brands Can you check out if your suggestion of using net 7 version for all works properly on the issues we have now?

[CharliePoole]

@OsirisTerje I'll jump in with my 2 cents worth here. :-) When I added the dependency model, and other code around it, I also added package tests, which failed without those changes and passed with them. I've seen other changes going in since then without any tests to prove that they actually fix something.

Of course, that's exactly how we worked before the Package Tests existed. Most of this stuff can't be tested via unit tests - at least not easily - and it caused lots of regressions. That's why package tests are there. See for example the simple test added to prove that we can load a windows forms app. At the very least, having package tests which fail (run only locally, of course) gives you visibility of the precise use cases that are not working.

[CharliePoole]

@OsirisTerje @rprouse @manfred-brands

This issue started during my previous temporary stint working on the engine and is still here. I'd like to do something with it but I'm not seeing any use cases I can work with. I'm also not clear which console runner is causing the issue and whether it is still a problem in the latest (3.18.2) release.