Alter binding source processing to no longer look for BindingAttribute#1044
Alter binding source processing to no longer look for BindingAttribute#1044Code-Grump wants to merge 5 commits intomainfrom
Conversation
This enhances backwards-compatilbity in scenarios where implementers may replace the binding source object and do not copy the new method attributes collection across
| return false; | ||
|
|
||
| var bindingSourceType = CreateBindingSourceType(type, filteredAttributes); | ||
| var candiateMethods = type.GetMethods( |
There was a problem hiding this comment.
I’m curious about the performance impact — there could be many methods involved. In time and memory.
Creating a list for every type at least adds memory pressure, and reflection is relatively slow.
P.S. There’s a typo in “candidate”.
There was a problem hiding this comment.
This definitely adds a cost for us - the system is written to produce a lightweight model layer between the actual reflection and the processing of binding methods, and without restructuring it more significantly, there's going to be allocations we could avoid.
However, I'd simply make the case that it's happening on a scale that's hard to perceive for most use-cases. This is the model that NUnit has used for a long time and that xunit has always used. If we'd like to make it as efficient as theirs, we'd need to do something more breaking in our APIs to refactor the component to yield types more efficiently.
There was a problem hiding this comment.
Good one about xUnit and NUnit. Im don't know if they use some kind of cache.
2 participants
🤔 What's changed?
This changes the binding source processing to no longer look for the presence of a
Bindingattribute on the class. Now, simply using any of the binding attributes (step bindings, hooks, etc) is sufficient to trigger a class being treated as binding type.⚡️ What's your motivation?
This change is in-keeping with most test frameworks to no longer require marker attributes at the class level, simplifying user code somewhat.
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
The changes made to the processor have been done in a way that should maintain backwards-compatibility at the contract. I would appreciate this being scrutinised for potential problems.
📋 Checklist:
This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.