New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add flag to support async methods by null guard check #1123
base: master
Are you sure you want to change the base?
Conversation
How is the user expected to use it? I usually get the assertion via AutoData and verify the subject. Can I set the property before verifying the subject? Or should I create the assertion manually like you do in the tests? Is the new behaviour additive? I.e. can I successfully test a non async method if I enable the new behaviour? If so, it's would only be about customizing the assertion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I suggest to consider implementing this (new) behavior in a new class? That's how we've been doing this in AutoFixture for many years, and has been working well.
@ploeh even described this in an article: SOLID is Append-only
The advantage will be that the old behavior remains intact. If a user wants to use the new behavior, he or she can use the new class (hypothetically, AsyncGuardClauseAssertion or similar).
Well, yes, IIRC either Clean Code or Refectoring points out that Boolean flags are code smells, and I completely agree with that sentiment. I'm not so convinced, however, that this is a proper issue that ought to be resolved, but let's discuss that over at #1106 |
I came across the issue (of GuardClauseAssertion not supporting methods marked with async) during implementation of a test suite this week and found this thread. It appear there's a PR available that provides the expected behaviour so what's the status? Are we still discussing the final design (i.e. a separate class as opposed to a property on the existing class)? What's required to move this request (and PR) forward? It's been 3 years and the world is moving to Task-based APIs for good reason. Not having this feature available in AutoFixture requires jumping through hoops to exclude Task-based methods being passed to the null guard check. If a separate class is required per previous approach in AutoFixture (as it mentioned by #1123 (review)), then we should move in that direction to get the feature implemented, verified and released. |
It's been almost two months since I asked about the required effort to bring support for async methods in null guard check and the lack of this support is starting to become quite tedious in our test suites; asked the following question previously:
So, what's the status and what are you proposing? |
Closes #1106
Added configuration node to support
async
methods, which are popular nowadays.Made this feature disabled by default to not cause breaking changes. Potentially we could make this behavior default in v5.
@moodmosaic Please take a look 馃槈