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

Monitor not available in .NET Standard 2.0 #1557

Closed
michael-hawker opened this issue May 11, 2021 · 8 comments · Fixed by #1879
Closed

Monitor not available in .NET Standard 2.0 #1557

michael-hawker opened this issue May 11, 2021 · 8 comments · Fixed by #1879

Comments

@michael-hawker
Copy link

Description

Wanted to make use of Event assertions to count times that events fired, but seems like Monitor isn't available for .NET Standard 2.0?

We're trying to do tests for UI components in our UWP library. This restriction doesn't seem to be called out in the docs. Is there a reason this isn't available for .NET Standard 2.0?

Expected behavior:

Able to use Event assertions as documented in all target frameworks/standards.

Versions

  • Which version of Fluent Assertions are you using? 5.10.3
  • Which .NET runtime and version are you targeting? .NET Standard 2.0 (UWP .NET Native)

FYI @RosarioPulella

@dennisdoomen
Copy link
Member

That specific API requires Reflection to dynamically generate event handlers, something that is not available on .NET Standard 2.0.

@Sergio0694
Copy link

Sergio0694 commented May 11, 2021

Please correct me if I'm wrong - it seems like that EventMonitor type was skipped on .NET Standard 2.0 specifically because the library is internally using DynamicMethod to do dynamic IL generation, which isn't built-in there, right? If that's the only blocking issue to support those APIs in this scenario, there's actually the official System.Reflection.Emit.Lightweight package that essentially backports DynamicMethod to .NET Standard 2.0 as well. Unless there's also other missing APIs that can't be worked around at all (either via polyfilling or by adding out of the box NuGet packages), it should in theory be possible to reference this package to enable EventMonitor for developers on .NET Standard 2.0 too? If supporting .NET Standard 2.0 is not considered worth the effort at this time I can understand, but figured I'd mention the package in case it was just missed while developing the APIs 🙂

@michael-hawker
Copy link
Author

Thanks @Sergio0694 I think they'd run in Debug mode, but not in Release Mode still which is how we're running ours in the CI. So, I think we'd still hit the issue?

@dennisdoomen thanks for the background, we just spent 15 minutes being confused trying to read the docs and not understanding why it was missing until we poked around the code. Would be handy to call that out in the docs, and keep the API surface the same for the implementations, but just throw a NotImplementedException on .NET Standard 2.0.

@Sergio0694 source generators in the future could help with these scenarios, eh?

@dennisdoomen
Copy link
Member

Please correct me if I'm wrong - it seems like that EventMonitor type was skipped on .NET Standard 2.0 specifically because the library is internally using DynamicMethod to do dynamic IL generation, which isn't built-in there, right? If that's the only blocking issue to support those APIs in this scenario, there's actually the official System.Reflection.Emit.Lightweight package that essentially backports DynamicMethod to .NET Standard 2.0 as well. Unless there's also other missing APIs that can't be worked around at all (either via polyfilling or by adding out of the box NuGet packages), it should in theory be possible to reference this package to enable EventMonitor for developers on .NET Standard 2.0 too? If supporting .NET Standard 2.0 is not considered worth the effort at this time I can understand, but figured I'd mention the package in case it was just missed while developing the APIs

I slightly recall we tried that, but I'm not sure anymore. But if anybody is up to it, feel free to try. We are very open to contributors.

Would be handy to call that out in the docs, and keep the API surface the same for the implementations, but just throw a NotImplementedException on .NET Standard 2.0.

I'm not very fond of that. But I agree about the documentation.

@jnyrup
Copy link
Member

jnyrup commented May 12, 2021

For reference, the original issue was #859 where we removed support for .netstandard as System.Reflection.Emit.Lightweight was unlisted from nuget because the package was lying about being available to all TFMs that can consume netstandard2.0

After we merged #861 the nuget was relisted with the note

However as mentioned earlier in this thread these packages claim to be netstandard1.1 compatible but that is a lie. They will only work on .NET Core and .NET Framework. So if you are consuming them from a netstandard library expect that library to fail if it is ran on any other .NET Standard implementation.

I don't know if the case has changed for the much newer 4.6.0 or 4.7.0.

ITaluone added a commit to ITaluone/fluentassertions that referenced this issue Apr 5, 2022
ITaluone added a commit to ITaluone/fluentassertions that referenced this issue Apr 5, 2022
@jnyrup
Copy link
Member

jnyrup commented Apr 6, 2022

Tried to see if referencing System.Reflection.Emit.LightWeight 4.7.0 from our netstandard2.0 target could change anything.

Made a test in UWP.Specs (UAP 10.0.19041.0) calling the EventMonitor assertions and got

System.PlatformNotSupportedException: Dynamic code generation is not supported on this platform.

ReflectionEmitThrower.ThrowPlatformNotSupportedException() + 0x54
DynamicMethod.ctor(String, Type, Type[], Module) + 0x26
EventHandlerFactory.GenerateHandler(Type, EventRecorder) + 0x1b2
EventRecorder.Attach(WeakReference, EventInfo) + 0x111
EventMonitor`1.AttachEventHandler(EventInfo, Func`1) + 0x1cf
EventMonitor`1.Attach(Type, Func`1) + 0x256
EventMonitor`1.ctor(Object, Func`1) + 0x1a6
AssertionExtensions.Monitor[T](T, Func`1) + 0x17f
EventAssertionSpecs.When_a_monitored_class_event_has_fired_it_should_be_possible_to_reset_the_event_monitor() + 0x7a
InvokeRetV(Object, IntPtr, InvokeUtils.ArgSetupState&, Boolean) + 0xbf
Call(IntPtr, Object, IntPtr, InvokeUtils.ArgSetupState&, Boolean) + 0xdf
InvokeUtils.CallDynamicInvokeMethod(Object, IntPtr, Object, IntPtr, IntPtr, Object, Object[], BinderBundle, Boolean, Boolean, Boolean) + 0x5aa

Looking inside of system.reflection.emit.lightweight\4.7.0\bin\netstandard2.0\System.Reflection.Emit.Lightweight.dll

public DynamicMethod(...)
{
      throw new PlatformNotSupportedException(System.SR.PlatformNotSupported_RefEmitLightweight);
}

@Sergio0694
Copy link

Sergio0694 commented Apr 6, 2022

UWP apps are 100% AOT and dynamic code generation is not supported, that is correct. The only reason why compiled LINQ expressions work, for instance, is that they have a special interpreter, but that is not available for just manually emitted IL.

@jnyrup
Copy link
Member

jnyrup commented Apr 7, 2022

@Sergio0694 Thanks for the clarification.

We support net47 and netcoreapp2.1, so according to the docs the implementations that might benefit from System.Reflection.Emit.Lightweight are mono, xamarin.xyz and Unity.

We'll update the documentation instead of adding a dependency only to throw a better exception before DynamicMethod throws.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants