Fix for Issue #315 #318
Fix for Issue #315 #318
Conversation
|
This is not intended for a Merge, I'm putting up what I got done as of today because I will be gone next week, I would like to complete it but I don't want fixing this to be blocked on me. I left the original code commented out so it is easier to see what I'm changing. With the fix as it currently is I am getting three other Scenario test failures but I haven't been able to investigate yet.
and System.Private.ServiceModel\tests\Scenarios\Client\ExpectedExceptions\ExpectedExceptionTests.cs
|
| @@ -77,13 +77,17 @@ public object[] GetCustomAttributes(Type attributeType, bool inherit) | |||
| switch (this.ProviderType) | |||
| { | |||
| case AttributeProviderType.Type: | |||
| return this.Type.GetTypeInfo().GetCustomAttributes(attributeType, inherit).ToArray(); | |||
| //return this.Type.GetTypeInfo().GetCustomAttributes(attributeType, inherit).ToArray(); | |||
| return this.Type.GetTypeInfo().GetCustomAttributes().ToArray(); | |||
There was a problem hiding this comment.
Thanks for going after the root cause @StephenBonikowsky . This PR will need a couple tweaks before merging.
We should not modify CustomAttributeProvider like this because that changes its general behavior for our specific failure case. I believe there is an alternate form of GetCustomAttributes which does not take an 'attributeType' parameter, and we should be calling that one instead. In other words, I would revert these changes to this version of GetCustomAttributes.
StephenBonikowsky
force-pushed
the
Issue315
branch
from
c84a2f4 to
cf4aea9
Compare
|
LGTM. IT is probably worth a buddy-build to test behavior in NET Native, but I recommend you go ahead and merge now. If it fails in NET Native we can open a new issue. |
Fixes #315