test: Z lib const descriptive failure messages #15977
Conversation
| [ | ||
| 'Expected Z_OK to be 0;', | ||
| `got ${zlib.constants.Z_OK}` | ||
| ].join(' ')); |
There was a problem hiding this comment.
I'm not a fan of this formatting, but I had to adhere to linter rules; specifically, max line length of 80. I would prefer format like this:
assert.strictEqual(zlib.codes.Z_OK, 0,
`Z_OK should be immutable. Expected to get 0, got ${zlib.codes.Z_OK}`);
What would folks think about overriding the max-line-length linter rule to 120 characters for this file?
There was a problem hiding this comment.
That is what is done in test/parallel/test-require-extensions-same-filename-as-dir-trailing-slash.js by adding /* eslint-disable max-len */.
There was a problem hiding this comment.
Ah, nice. That's what I had in mind to do. Guess I didn't want to rock the linting rules boat on my first contribution to Node, eh? Ha! Will override here.
There was a problem hiding this comment.
This would also work
assert.strictEqual(
zlib.constants.Z_OK,
0,
`Expected Z_OK to be 0; got ${zlib.constants.Z_OK}`
);Deactiviating the max-len is not a good idea.
Trott
added
the
code-and-learn
BridgeAR
changed the title
|
CI is ✔️ |
PR-URL: nodejs#15977 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
|
Landed in 91724cb Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 ! |
PR-URL: #15977 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: nodejs/node#15977 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
More informative failure messages for ZLib constants tests. Using template literals to output the actual value in addition to expected.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test