From 47235f0c4409916f4be3409749c784de98770531 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 4 Feb 2017 02:14:32 +0800 Subject: [PATCH 1/6] doc: update test writing guide Add guide about choice of assertions and encourage using ES.Next features. --- doc/guides/writing-tests.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index f27c076590e513..dc78216b1c8552 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -209,6 +209,36 @@ const assert = require('assert'); const freelist = require('internal/freelist'); ``` +### Assertions + +When writing assertions for object comparison, prefer the strict versions, +namely: + +* `assert.strictEqual()` over `assert.equal()` +* `assert.deepStrictEqual()` over `assert.deepEqual()` + +When using `assert.throws()`, if possible, provide the full error message: + +```js +assert.throws( + () => { + throw new Error('Wrong value'); + }, + /^Error: Wrong value$/ // Instead of something like /Wrong value/ +); +``` + +### ES.Next features + +At the moment we only use a selected subset of ES.Next features in +JavaScript code under the `lib` directory for performance considerations, +but when writing tests, it is encouraged to use ES.Next features that have +already landed in the ECMAScript specification. For example: + +* `let` and `const` over `var` +* Template literals over string concatenation +* Arrow functions over normal functions, if appropriate + ## Naming Test Files Test files are named using kebab casing. The first component of the name is From 4f6f2359c510a4baa6edaffe3ec7313d2a4c6295 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 4 Feb 2017 02:15:26 +0800 Subject: [PATCH 2/6] doc: update testing guide regarding WPT upstream --- doc/guides/writing-tests.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index dc78216b1c8552..daf032870ff7d2 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -250,3 +250,29 @@ For example, a test for the `beforeExit` event on the `process` object might be named `test-process-before-exit.js`. If the test specifically checked that arrow functions worked correctly with the `beforeExit` event, then it might be named `test-process-before-exit-arrow-functions.js`. + +## Imported Tests + +### Web Platform Tests + +Some of the tests for the WHATWG URL implementation (named +`test-whatwg-url-*.js`) are imported from the +[Web Platform Tests Project](https://github.com/w3c/web-platform-tests/tree/master/url). +If you see a test wrapped in code like this + +```js +/* eslint-disable */ +/* WPT Refs: + https://github.com/w3c/web-platform-tests/blob/8791bed/url/urlsearchparams-stringifier.html + License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html +*/ + +// Test code + +/* eslint-enable */ +``` + +and want to improve it, please send a PR to the upstream project +first, and when it's merged, send another PR to this project +to update the code accordingly (be sure to update the hash in the +URL following `WPT Refs:`). From 1d9c9f0e16dc4b89914a5f49799fda04387a13ac Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 4 Feb 2017 21:54:43 +0800 Subject: [PATCH 3/6] fix style --- doc/guides/writing-tests.md | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index daf032870ff7d2..d204df0a93711c 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -211,8 +211,7 @@ const freelist = require('internal/freelist'); ### Assertions -When writing assertions for object comparison, prefer the strict versions, -namely: +When writing assertions for comparison, prefer the strict versions: * `assert.strictEqual()` over `assert.equal()` * `assert.deepStrictEqual()` over `assert.deepEqual()` @@ -230,14 +229,14 @@ assert.throws( ### ES.Next features -At the moment we only use a selected subset of ES.Next features in -JavaScript code under the `lib` directory for performance considerations, -but when writing tests, it is encouraged to use ES.Next features that have -already landed in the ECMAScript specification. For example: +For performance considerations, we only use a selected subset of ES.Next +features in JavaScript code in the `lib` directory. However, when writing +tests, it is encouraged to use ES.Next features that have already landed +in the ECMAScript specification. For example: * `let` and `const` over `var` * Template literals over string concatenation -* Arrow functions over normal functions, if appropriate +* Arrow functions when appropriate ## Naming Test Files @@ -258,7 +257,7 @@ functions worked correctly with the `beforeExit` event, then it might be named Some of the tests for the WHATWG URL implementation (named `test-whatwg-url-*.js`) are imported from the [Web Platform Tests Project](https://github.com/w3c/web-platform-tests/tree/master/url). -If you see a test wrapped in code like this +These imported tests will be wrapped like this: ```js /* eslint-disable */ @@ -272,7 +271,7 @@ If you see a test wrapped in code like this /* eslint-enable */ ``` -and want to improve it, please send a PR to the upstream project -first, and when it's merged, send another PR to this project -to update the code accordingly (be sure to update the hash in the -URL following `WPT Refs:`). +If you want to improve tests that have been imported this way, please send +a PR to the upstream project first. When your proposed change is merged in +the upstream project, send another PR to update the code accordingly. +Be sure to update the hash in the hash in the URL following `WPT Refs:`. From f236299648372be4dde72dcebe1dbac2be8e18b8 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 6 Feb 2017 11:35:52 +0800 Subject: [PATCH 4/6] fix styles --- doc/guides/writing-tests.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index d204df0a93711c..57ce084727b092 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -211,7 +211,7 @@ const freelist = require('internal/freelist'); ### Assertions -When writing assertions for comparison, prefer the strict versions: +When writing an assertions for comparison, prefer the strict versions: * `assert.strictEqual()` over `assert.equal()` * `assert.deepStrictEqual()` over `assert.deepEqual()` @@ -273,5 +273,5 @@ These imported tests will be wrapped like this: If you want to improve tests that have been imported this way, please send a PR to the upstream project first. When your proposed change is merged in -the upstream project, send another PR to update the code accordingly. -Be sure to update the hash in the hash in the URL following `WPT Refs:`. +the upstream project, send another PR here to update Node.js accordingly. +Be sure to update the hash in the URL following `WPT Refs:`. From 7ce8f5dde264ba75bd4e05b10b374fb12ad5df80 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 6 Feb 2017 13:16:50 +0800 Subject: [PATCH 5/6] fix typo --- doc/guides/writing-tests.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index 57ce084727b092..e3fe2fb102ba7d 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -211,7 +211,7 @@ const freelist = require('internal/freelist'); ### Assertions -When writing an assertions for comparison, prefer the strict versions: +When writing an assertion for comparison, prefer the strict versions: * `assert.strictEqual()` over `assert.equal()` * `assert.deepStrictEqual()` over `assert.deepEqual()` From 61fb2e8e5b02e9b65069f433914332126e438e08 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 6 Feb 2017 13:54:14 +0800 Subject: [PATCH 6/6] drop 'for comparison' --- doc/guides/writing-tests.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index e3fe2fb102ba7d..36cf04fc546825 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -211,7 +211,7 @@ const freelist = require('internal/freelist'); ### Assertions -When writing an assertion for comparison, prefer the strict versions: +When writing assertions, prefer the strict versions: * `assert.strictEqual()` over `assert.equal()` * `assert.deepStrictEqual()` over `assert.deepEqual()`