nunit / nunit3-vs-adapter

NUnit 3.0 Visual Studio test adapter for use under VS 2012 or later
https://nunit.org
MIT License
203 stars 105 forks source link

Embed engine assemblies #583

Open jnm2 opened 5 years ago

jnm2 commented 5 years ago

Similar to https://github.com/nunit/nunit3-vs-adapter/pull/541, we should consider embedding all other assemblies that might conflict with user test project dependencies.

https://github.com/nunit/nunit3-vs-adapter/issues/573 has a short-term fix by catching up the version, but this only solves the problem for users who compile against the very latest engine (for now).

https://github.com/nunit/nunit3-vs-adapter/issues/573#issuecomment-441381856:

Currently it's looking like there's no way (in C#) to hook AssemblyResolve before VSTest encounters System.IO.FileNotFoundException while doing adapterAssembly.GetTypes():

'Could not load file or assembly 'nunit.engine.api, Version=3.0.0.0, Culture=neutral, PublicKeyToken=2638cd05610744eb' or one of its dependencies. The system cannot find the file specified.'

I'm pretty sure by now that these are our options:

  • Stop directly referencing any NUnit engine DLLs from the adapter; use reflection for everything

  • Or, ship a separate entry point assembly which does not reference any NUnit engine DLLs and which hooks AssemblyResolve and then loads our main adapter logic assembly which does reference NUnit engine DLLs.

  • Or, use something like https://github.com/Fody/Costura to rewrite the IL after building the adapter assembly to insert a module initializer, in hopes that a module initializer can hook AssemblyResolve before the TypeLoadException happens. (The best C# can do is a type initializer, and that's not good enough.)

nvborisenko commented 5 years ago

Can be closed.

jnm2 commented 5 years ago

@nvborisenko I think this is something we are still interested in.

nvborisenko commented 5 years ago

What if test adapter package will depend on engine nuget package. This lets nuget to resolve conflicts when user installs packages. I guess this way stops supporting Visual Studio v15.8 (not sure about changes in VS regarding how testhost is locating testadapters). As I see the latest version of VS uses adapters from test output folder, so adapter package can aware about copying dependencies to output folder.

OsirisTerje commented 5 years ago

We need to ensure backwards compatibility. So if we do it "smarter", it must still be able to retro-fit into older VS.

jnm2 commented 3 years ago

Or, use something like https://github.com/Fody/Costura to rewrite the IL after building the adapter assembly to insert a module initializer, in hopes that a module initializer can hook AssemblyResolve before the TypeLoadException happens. (The best C# can do is a type initializer, and that's not good enough.)

Since I've said this, I discovered that this cannot work. I was able to build examples on .NET Core and .NET Framework which show that Assembly.GetTypes() does not trigger the module initializer. I recall that Assembly.GetTypes() is exactly what VSTest is doing that is failing when the included types require dependencies to be available. We could be nice and ask VSTest to run RuntimeHelpers.RunModuleConstructor(adapterAssembly.ManifestModule.ModuleHandle); before calling Assembly.GetTypes, but in today's world it's not an option.

That leaves us with the remaining two options: