[release/8.0] Fix calling existing ctor with MethodInvoker; share tests with invokers #90968
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Tagging subscribers to this area: @dotnet/area-system-reflection Issue DetailsBackport of #90796 to release/8.0 /cc @jkotas @steveharter Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
|
@steveharter @jkotas can you please fill out the template, get an approval and a sign-off? |
|
@buyaa-n Could you please push it through the .NET 8.0 process since Steve is OOF? |
Sure, the template updated, added the |
|
Backports to RC2, before the snap, only require M2 approval. @jeffhandley |
|
I'm deferring to @ericstj on this one. It'll get reviewed within the next couple business days. |
There was a problem hiding this comment.
I discussed this with @steveharter last week and support this change. This is fixing a significant bug in a brand new 8.0 feature.
|
@carlossanlop please merge. Thanks |
carlossanlop
added
Servicing-approved
ghost
locked as resolved and limited conversation to collaborators
Backport of #90796 to release/8.0
/cc @jkotas @steveharter
Customer Impact
Refactored existing
MethodInfo\ConstructorInfotests so they can also run under the new invoker classesMethodInvoker\ConstructorInvokerwhich revealed a bug that fixed with this PR. The newMethodInvoker\ConstructorInvokerfeature added in .NET 8.0 preview 7. These fixes should be ported to .NET 8 since they only affect the new APIs added in Preview 7 and will avoid a breaking change in .NET 9.Testing
The refactored tests are new for
MethodInvoker\ConstructorInvokercase, now all those unit tests are running under theMethodInvoker\ConstructorInvokerfeature and all passing.Risk
Low - the fix is well tested with the new refactored unit tests.