Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.2.0 causes NUnit.Engine.TestEngineActivator.CreateInstance() to return null #635

Closed
dave-yotta opened this issue Aug 26, 2021 · 5 comments

Comments

@dave-yotta
Copy link

dave-yotta commented Aug 26, 2021

#! "netcoreapp3.1"
#r "nuget: NUnit.Engine, 3.10.0"
using NUnit.Engine;
var engine = TestEngineActivator.CreateInstance();

since 1.2.0 TestEngineActivator.CreateInstance is returning null.
Something to do with assembly changes?

@seesharper
Copy link
Collaborator

@hrumhurum We have an issue here. Might be related to the fact that the code does Assembly.LoadFrom

https://github.com/nunit/nunit-console/blob/master/src/NUnitEngine/nunit.engine.api/TestEngineActivator.cs

@hrumhurum
Copy link
Contributor

hrumhurum commented Aug 26, 2021

Assembly.LoadFrom is not routed to AssemblyLoadContext.CurrentContextualReflectionContext, while Assembly.Load is routed correctly. This looks like an omission in .NET runtime.

Due to that flaw, the assemblies loaded with Assembly.LoadFrom are always loaded to the default AssemblyLoadContext.

There are three possible workarounds:

  1. Do not use assembly isolation at all at dotnet-script level, or
  2. Keep a handcrafted list of affected assemblies and homogenize them with the default assembly load context
  3. Contact NUnit and propose to use AssemblyLoadContext.CurrentContextualReflectionContext for assembly loading when it is not null

What shall we choose?

@seesharper
Copy link
Collaborator

seesharper commented Aug 26, 2021

Me and @filipw discussed this and we landed on making this an opt-in feature for now.

Number 2 and 3 are not the best options as we would most probably run into other libraries with the same "defect" and the list will just grow

@filipw What would be a good name for such an option? -ilc | --isolated-load-context ?

@dave-yotta
Copy link
Author

Quick reaction there guys :)

@filipw
Copy link
Member

filipw commented Aug 27, 2021

1.2.1 is out which makes the isolation opt in based on an --isolated-load-context switch

I think this can be closed

@filipw filipw closed this as completed Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants