-
-
Notifications
You must be signed in to change notification settings - Fork 9
Refactor Assert methods #255
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
Conversation
Important Auto Review SkippedReview was skipped due to path filters Files ignored due to path filters (10)
You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
This PR has a ton of changes so it might be easier to review on a commit by commit basis. The POC project was updated to ensure that all Assert methods work as expected. |
ok, so from my point of view, you've been adjusting the following and please correct me if I'm wrong:
All up, I feel it's ok, indeed, not easy to review if not going in each commit. I'll still need a bit of time to review everything. |
That is a good summary. The primary change was adding the Along the way I fixed some tech debt (such as using I'll update the description with a more comprehensive list of changes. |
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.
Finally took the time to review and it looks good.
@CoryCharlton if no other change to be done, I can go and merge, just let me know. |
I have no other changes at this time. |
Description
HandleFail
is
operator so equality overloads cannot impact themAreEqual
,AreNotEqual
and obsolete methodsDoesNotReturn
attribute to methods that always throw exceptions (ie:HandleFail
)AreSame
andAreNotSame
to useReferenceEquals
so equality overloads cannot impact themArray
overload ofAreEqual
since it is effectively the same as theObject
overload and gave a false impression that the collection was being checkingContains
,DoesNotContains
,EndsWith
, andStartsWith
to use the built-in methods in theString
classMotivation and Context
The existing code needed some love. My motivation was that by making all these changes not only will unit testing be more enjoyable but hopefully it will be easier to keep this code up to date.
How Has This Been Tested?
Screenshots
Types of changes
Checklist: