Harden some reflection invoke tests#113000
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-reflection |
There was a problem hiding this comment.
PR Overview
This PR hardens tests related to reflection invocation and stack frame analysis in anticipation of upcoming changes in reflection thunk implementations.
- Adds VerifyInnerException tests for constructor and method invocations.
- Adjusts stack frame tests by adding a dummy parameter to avoid reflection optimizations.
- Removes obsolete InvokeInterpreted and InvokeEmit tests to align with planned changes.
Reviewed Changes
| File | Description |
|---|---|
| src/libraries/System.Runtime/tests/System.Reflection.Tests/ConstructorCommonTests.cs | Added a test to check the inner exception for constructor invocations. |
| src/libraries/System.Runtime/tests/System.Reflection.Tests/MethodCommonTests.cs | Added a test to check the inner exception for method invocations. |
| src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs | Updated inline data and added a dummy parameter to avoid reflection optimizations. |
| src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs | Removed obsolete tests. |
| src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs | Removed obsolete tests. |
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
| [InlineData(0, false)] | ||
| [InlineData(1, false)] | ||
| // This is highly dependent on reflection implementation and is not consistent across runtimes. | ||
| // The extra 'bool _' parameter avoids a potential reflection optimization for common signatures which may interfere with stack frame count. |
There was a problem hiding this comment.
This will break again if this signature gets added to the list of optimized signatures. Should this be test be deleted, or ignore the potential extra frames instead?
There was a problem hiding this comment.
Correct - I would like to just remove this test but I was not sure on the long background\history here.
There was a problem hiding this comment.
I did remove that test. In general, the rest of the stack frame tests still verify the stack frame APIs work; this test verified constructors.
Also I will close the Mono mono/mono#15183 issue once this PR is in.
Contributes to #112994
Hardens two tests: