test: more useful assert messaging for child-process-execSync - #15972
test: more useful assert messaging for child-process-execSync#15972ericljpemberton wants to merge 1 commit into
Conversation
Sorry, something went wrong.
|
|
||
| assert.strictEqual(ret.length, msgBuf.length); | ||
| assert.deepStrictEqual(ret, msgBuf, 'execSync result buffer should match'); | ||
| assert.deepStrictEqual(ret, msgBuf); |
There was a problem hiding this comment.
The returned error message is not going to be great here and therefore it might be better to use a error message that includes both buffers. But this should not be blocking.
Sorry, something went wrong.
There was a problem hiding this comment.
@BridgeAR When I run assert.deepStrictEqual in the default behavior using foobar (as in the test) and foobaz the default assertion thrown is this:
AssertionError: <Buffer 66 6f 6f 62 61 7a 0a> deepStrictEqual <Buffer 66 6f 6f 62 61 72 0a>
Are you saying that I should change it to look something like the following?:
AssertionError: ret <Buffer 66 6f 6f 62 61 7a 0a> and msgBuf <Buffer 66 6f 6f 62 61 72 0a> should be equal
Sorry, something went wrong.
There was a problem hiding this comment.
Na, just ignore my comment ;-)
Sorry, something went wrong.
PR-URL: nodejs#15972 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
Landed in 55bb9c4 Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 ! |
Sorry, something went wrong.
PR-URL: #15972 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15972 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15972 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15972 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15972 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This is a commit for Node Interactive. I changed some basic assertion messages to hopefully provide better insights into errors.
Checklist
Affected core subsystem(s)
tests