Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Should().NotThrowAfter assertion for actions #942
Add Should().NotThrowAfter assertion for actions #942
Changes from 2 commits
0e8d5f8
b7a41f8
2da5e3b
d83dd87
b46d678
b6a22e1
5d80597
cdd339e
fe90171
a8bdaf9
a9aa045
cc58f1d
670d278
f1525e5
f22fcdb
bb1e1a0
36a850c
e3004c2
cfc76ca
40258c0
7da866c
ccfeeca
49ec86f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
🔧 I would prefer to use
TimeSpan
forwaitTime
andpollInterval
.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.
🔧 Maybe
NotThrowAnymoreAfter
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.
🤔 Do we also need to be able to specify an exception type here?
🤔 Do we also need this assertion for
Func<Task>
for consistency with the other exception APIs?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.
Are you sure? I think that this would be a tad too long. It might also be misleading. Doesn't "anymore" suggest that the Action must have been throwing exceptions at some point
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.
Sounds good. Would you keep an overload with the current signature or would you expect the user to convert from that representation to the TimeSpan?
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.
True. Very true. Alright. Let's stick with
NotThrowAfter
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.
They can use the
TimeSpan
extension methods like12.Seconds()
.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.
I have not encountered a use case for this so far. This would mean that the Action could be still throwing exceptions but not the specified one ... :confused. We could add it for consistency's sake. You could also wait until someone requests it. I don't know your policy regarding these kind of questions 😏
Probably.