-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
test_runner: stringify AssertError expected and actual #47088
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
test_runner: stringify AssertError expected and actual #47088
Conversation
|
Review requested:
|
cjihrig
left a comment
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 think this change might work, but I think it's incomplete without a context object. I also worry about how the stringify change might interact with large objects.
I'm not 100% if it would be valid YAML, but if it is, maybe actual and expected should add another level of indentation instead of just trying to stringify.
6a60ab8 to
b6d1159
Compare
b6d1159 to
b540361
Compare
cjihrig
left a comment
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.
Sorry for the delay. LGTM
b540361 to
8840601
Compare
|
Landed in 102540a |
PR-URL: #47088 Fixes: #47075 Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #47088 Fixes: #47075 Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #47088 Fixes: #47075 Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #47088 Fixes: #47075 Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #47088 Fixes: #47075 Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #47088 Fixes: #47075 Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #47088 Fixes: #47075 Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#47088 Fixes: nodejs#47075 Reviewed-By: Colin Ihrig <[email protected]>
Fixes: #47075