-
Notifications
You must be signed in to change notification settings - Fork 269
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
Partially improve CollectionAssert message #4027
Partially improve CollectionAssert message #4027
Conversation
3fa7640
to
0ef3a45
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.
I would prefer to have actual and expected on new lines and not surrounded by quotes because for most complex data it could render weirdly.
@nohwnd I don't recall if your idea of expected/actual as exception data is already supported in TE or VSTest. If that's the case, it could be better than doing it explicitly (as done here).
@Youssef1313 Could we add a few tests?
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 have actual and expected on new lines and not surrounded by quotes because for most complex data it could render weirdly.
@nohwnd I don't recall if your idea of expected/actual as exception data is already supported in TE or VSTest. If that's the case, it could be better than doing it explicitly (as done here).
@Youssef1313 Could we add a few tests?
test/UnitTests/TestFramework.UnitTests/Assertions/CollectionAssertTests.cs
Outdated
Show resolved
Hide resolved
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.
One last readability nit and LGTM, thanks @Youssef1313
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.
Awesome
It still doesn't give the complete info as requested in #334