Update Mono.Cecil to latest

[rprouse]

Mono.Cecil 10.0 is no longer in preview. Update the .NET Standard build to use it. We cannot update the full framework engine because Mono.Cecil has dropped support for .NET 2.0.

[jnm2]

If we thought it was worthwhile, we could move all the full framework builds to 0.10.0 except net20.

[rprouse]

We only use it in the engine which only targets 2.0

[jnm2]

@rprouse Oh that's right lol, we don't have (or need) a net35 build or higher. Never mind!

[rprouse]

I had thought about upping to 3.5 though so that we could continue to update Cecil as needed, we could then run 2.0 in compatibility mode. We would need to think through the consequences though, but the root of my thinking is that you have not been able to install 2.0 on its own for years now. I'd love to hear from users that this would affect because they only have 2.0.

[jnm2]

Are the mailing lists and maybe twitter the best way to get people's attention? Or maybe a NuGet.org preview release?

[ChrisMaddock]

If we're multitargeting the engine soon anyway, would it not be pretty trivial to have a net20 and net35 build of the engine, and just have then reference different versions of Mono.Cecil? Then, so long as the API doesn't change, we can update the dependency for the majority, and pin net20 to an older version?

That said - that introduces an overhead, and I'm all for trying to gauge some interest in .NET 2.0 support. Worth bearing in mind we provide the NUnitLite runner too - which will provide 'simple runner' support. Between Cecil, and the desire to change the inter-process communication solution, 2.0 support is starting to hold things back for the majority.

[bording]

@jnm2 @rprouse What's the current thinking about the Mono.Cecil dependency you have here? This is becoming a larger and larger problem for me. If I have a test project that uses Mono.Cecil (and I do have a lot of them), I'm forced to use 3.8.0-alpha1 of the test adapter to even have a chance of the tests running correctly.

I would love to see one of the options discussed in nunit/nunit3-vs-adapter#366 (comment) actually happen.

Is there something I can do to help with this?

[jnm2]

@rprouse I'm happy to help with this. I'm pretty agnostic about our short-term approach.

[rprouse]

For the .NET Standard build of the engine, the PR I am working on has updated to 0.10.0. I don't know what to do about the full framework version of the engine though. It compiles to .NET 2.0, but Mono.Cecil dropped support for .NET 2.0 in 0.10.0. We also can't do the binding redirects to a newer version there because there are breaking API changes in Mono.Cecil.

Maybe it's time to update the console and the engine to 3.5? We would still support running 2.0 tests, but they would be run from an Engine/Console compiled to 3.5. That is the minimum version the VSTest runner is compiled to, so no loss on the adapter end. You also can't install 2.0 without 3.5 anymore.

[jnm2]

I'd be supportive of that. Everything I know that targets CLR v2 is using .NET Framework 3.5.

[rprouse]

I just did a bit of spelunking through the xUnit repo since they went all the way to .NET 4.5. I only found one user complaining and they only wanted to go back to .NET 4.0 support because they are still running tests on CI servers running Windows 2003!

[bording]

While getting the engine updated to 0.10.0 is a good first step, it doesn't really address the underlying issue: you're copying your dependencies into the bin folder of a test project, potentially overwriting the version actually referenced by the project.

Any thoughts on how to address that?

[rprouse]

Thinking out loud,

Once we get everything on the same version, I think we will be able to do @jnm2's option one or two as long as consuming libraries use 0.10.0 or newer. We could also compile in as a sub-module, but we might need to rewrite the namespaces to prevent conflicts when not running in separate app domains.

[CharliePoole]

@bording @rprouse The original intent (and implementation) of the engine had it in a separate directory, with only the nunit.engine.api assembly copied to the output directory. Subsequent changes in packaging put everything into the output directory.

As the team kniows, I favor returning to the original model. That would, I believe, completely solve this problem along with a few others, the sole exception being that of a user who suppresses use of a separate AppDomain for the tests.

[bording]

As the team kniows, I favor returning to the original model. That would, I believe, completely solve this problem along with a few others, the sole exception being that of a user who suppresses use of a separate AppDomain for the tests.

I'm not sure that's possible with how the VS Test Adapter model works. I believe that all the assemblies need to be copied to the bin folder.

[CharliePoole]

@bording If that's so, then it would have to be due to a change in VS since the time when we didn't copy the engine to the output folder. However, I think you may be confusing the adapter (which I believe now must be copied) with the engine.

Both the engine and the adapter previously referenced Mono.Cecil. The engine still does. I was under the impression that @OsirisTerje had removed the adapter dependency, but I haven't checked. I was addressing the engine only, since that's the repo this issue is in.

[rprouse]

Moving Mono.Cecil out to a different directory might be an option, but there is nothing like Assembly.LoadFrom or even AppDomain in versions of .NET Standard older than 2.0, so we would lose support for running tests in .NET Core 1.0 and 1.1.

[jnm2]

ILSpy has just merged a massive PR to replace Mono.Cecil with SR.Metadata.
https://github.com/icsharpcode/ILSpy/pull/1030

[bording]

Any more thoughts on how to address this? Having Mono.Cecil copied into the bin folder of my test projects is causing a ton of pain.

[jnm2]

I really want to solve this. Looking at it again.

[jnm2]

As a general principle, if we have any dependencies that a user might have, rather than bundling extra assemblies, we should make them NuGet references so that users can override the dependency version.

[CharliePoole]

Makes sense as a general principal. For anything specific, someone needs to look at why we could not upgrade it and figure out if anything has changed. IIRC, in the case of Mono.Cecil, it had to do with their dropping support of .NET 2.0.

[jnm2]

Very true.
What I'm trying right now is removing Mono.Cecil with the same tactics as nunit/nunit3-vs-adapter#454.

[jnm2]

Got sidetracked #460

[bording]

Mono.Cecil, it had to do with their dropping support of .NET 2.0.

Considering how ancient that version of the .NET Framework is, I think it's quite reasonable to consider dropping support for it!

[CharliePoole]

Sure, it just makes the issue more than simply Mono.Cecil. FWIW most active team members (incl. me) would love to not have to deal with .NET 2.0. We do it because it's a service to a (diminishing) group of users.

[mikkelbu]

Got sidetracked #460

You shouldn't get sidetracked so I've approved and merged the PR 😄

[jnm2]

Nearly finished, but I'm sidetracked again. I can't get the build script to run. Can't build a fresh clone on my machine, but AppVeyor is okay. This is gonna be interesting.

C:\Program Files\dotnet\sdk\2.1.401\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(198,5): error NETSDK10 05: Assets file 'C:\Users\Joseph\Source\Repos\nunit-console-debug\src\NUnitEngine\nunit-agent\obj\project.assets.json' doesn't have a t arget for '.NETFramework,Version=v3.5'. Ensure that restore has run and that you have included 'net35' in the TargetFrameworks for your project. [C:\Users\Joseph\Source\Repos\nunit-console-debug\src\NUnitEngine\nunit-agent\nunit-agent.csproj]
C:\Program Files\dotnet\sdk\2.1.401\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(198,5): error NETSDK10 05: Assets file 'C:\Users\Joseph\Source\Repos\nunit-console-debug\src\NUnitEngine\nunit-agent\obj\project.assets.json' doesn't have a t arget for '.NETFramework,Version=v3.5'. Ensure that restore has run and that you have included 'net35' in the TargetFrameworks for your project. [C:\Users\Joseph\Source\Repos\nunit-console-debug\src\NUnitEngine\nunit-agent\nunit-agent-x86.csproj]
C:\Program Files\dotnet\sdk\2.1.401\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(198,5): error NETSDK10 05: Assets file 'C:\Users\Joseph\Source\Repos\nunit-console-debug\src\NUnitConsole\nunit3-console\obj\project.assets.json' doesn't have a target for '.NETFramework,Version=v3.5'. Ensure that restore has run and that you have included 'net35' in the TargetFrameworks for
your project. [C:\Users\Joseph\Source\Repos\nunit-console-debug\src\NUnitConsole\nunit3-console\nunit3-console.csproj]

[jnm2]

Think I'm hitting this: https://github.com/Microsoft/msbuild/issues/3626

[bording]

@jnm2 If you are, did you see the workaround? dotnet/msbuild#3626 (comment)

[jnm2]

Yes, it's working for me for now. Thank you!

[jnm2]

All tests passing after replacing Mono.Cecil with reflection-only loading in the .NET Framework build. Starting on .NET Core next.