-
-
Notifications
You must be signed in to change notification settings - Fork 798
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
#825 event handlers setup and verify #857
Conversation
Thanks for the PR, @lepijohnny. I'll try to review sometime during the next few days. |
np, take your time. I will add few more comments to PR since I noticed some issues but didn't really have time to fix PR. |
9e371a5
to
c6c3948
Compare
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.
Great work! I'm getting the impression that you took a lot of care in preparing this, all the way down to XML documentation comments. 👍
There are a few things that we might be able to tweak some further; and there's one or two things that I'm possibly not understanding correctly. Feel free to correct me if I got something wrong.
Oh btw., you should also add an entry to CHANGELOG.md
(under Unreleased → Added).
(Also, don't worry about the CodeFactor validation failure of this PR. I'll eventually deal with these "issues" by abandoning those .xdoc
files through inlining their contents into the main code files.)
c6c3948
to
a6ada6a
Compare
Thanks for the great work, @lepijohnny! 🚀 |
* WIP: initial feature commit * Thread-safe hasEventSetup check * Fix Resources * PR review comments processed * CHANGELOG.md updated * Fix codefactor violation.
Hi,
I was busy during the weekend with this feature, it adds the functionality to Setup and Verify events as all other methods. I tried to preserve backward compatibility and also I have added new test scenarios to verify this feature. Four new methods have been added (I am maybe thinking of merging this to
SetupEvent
andVerifyEvent
):SetupAdd
SetupRemove
VerifyAdd
VerifyRemove
Besides these methods
Verify()
,VerifyAll()
andVerifyNoOtherCalls
work as well.Kind regards,
Nikola