dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.48k stars 4.77k forks source link

Move Built-in marshalling tests to use DNNE and live under the System.RuntimeInteropServices test tree #84451

Open jkoritzinsky opened 1 year ago

jkoritzinsky commented 1 year ago

To simplify the testing strategy for LibraryImportGenerator, we utilized the DNNE package to enable writing the "unmanaged" side of the tests in C#. For our tests under src/tests/Interop that are only testing our built-in marshalling logic, we should consider moving these tests to utilize DNNE as well and live under the libraries test tree. This change will help us reduce how many tests we own under src/tests.

We will keep tests under src/tests/Interop that test interactions with native code that we can't safely validate with a managed implementation (because "both sides" of the managed code could be wrong), such as calling convention tests or primitive/native intrinsic type marshalling tests. We should also exclude moving tests that integrate tightly with platform features or use a custom app launcher to have the test start from native code, like the COM and Objective-C tests.

MichalStrehovsky commented 1 year ago

The use of DNNE currently blocks running the LibraryImportGenerator tests with NativeAOT:

https://github.com/dotnet/runtime/blob/de5a04b2d7d184c5a9121b56cd51c713f72def76/src/libraries/tests.proj#L456-L457

The tests are also excluded on mobile/bionic, etc. probably for similar reasons. If we want to put more testing on that plan, we need to resolve this too.

jkoritzinsky commented 1 year ago

The tests in the src/tests tree that have native components are also excluded on mobile (the native build there was never hooked up to work for mobile targets), so I don't think those targets should be a requirement before doing this. Bionic and NativeAOT are good prereqs before doing this work though.

AaronRobinsonMSFT commented 1 year ago

If we want to put more testing on that plan, we need to resolve this too.

DNNE was designed to be compatible with a NativeAOT workflow. I would imagine that in a NativeAOT scenario, we could simply not use DNNE and NativeAOT the test assembly to get the native exports generated. Is that fair or is there another issue?

jkoritzinsky commented 1 year ago

I think if we turn on DirectPInvoke and set up the build such that the symbols from the UnmanagedCallersOnly methods in NativeExports are generated, things will work. Just haven't had a chance to try it. And if DirectPInvoke doesn't work, we can use a custom DllImportResolver to redirect the PInvokes to the main DLL.