From 9ab63d61f8f8d65b0e0643c83dd7936c4cf0a309 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Wed, 12 Jul 2017 17:46:17 -0400 Subject: [PATCH] doc: add guidance on testing new errors PR-URL: https://github.com/nodejs/node/pull/14207 Reviewed-By: Refael Ackermann Reviewed-By: Vse Mozhet Byt Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- doc/guides/using-internal-errors.md | 47 +++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/doc/guides/using-internal-errors.md b/doc/guides/using-internal-errors.md index ea011aaa83..3549a0da6b 100644 --- a/doc/guides/using-internal-errors.md +++ b/doc/guides/using-internal-errors.md @@ -69,6 +69,53 @@ for the error code should be added to the `doc/api/errors.md` file. This will give users a place to go to easily look up the meaning of individual error codes. +## Testing new errors + +When adding a new error, corresponding test(s) for the error message +formatting may also be required. If the messasge for the error is a +constant string then no test is required for the error message formatting +as we can trust the error helper implementation. An example of this kind of +error would be: + +```js +E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound'); +``` + +If the error message is not a constant string then tests to validate +the formatting of the message based on the parameters used when +creating the error should be added to +`test/parallel/test-internal-errors.js`. These tests should validate +all of the different ways parameters can be used to generate the final +message string. A simple example is: + +```js +// Test ERR_TLS_CERT_ALTNAME_INVALID +assert.strictEqual( + errors.message('ERR_TLS_CERT_ALTNAME_INVALID', ['altname']), + 'Hostname/IP does not match certificate\'s altnames: altname'); +``` + +In addition, there should also be tests which validate the use of the +error based on where it is used in the codebase. For these tests, except in +special cases, they should only validate that the expected code is received +and NOT validate the message. This will reduce the amount of test change +required when the message for an error changes. + +For example: + +```js +assert.throws(() => { + socket.bind(); +}, common.expectsError({ + code: 'ERR_SOCKET_ALREADY_BOUND', + type: Error +})); +``` + +Avoid changing the format of the message after the error has been created. +If it does make sense to do this for some reason, then additional tests +validating the formatting of the error message for those cases will +likely be required. ## API