From 0674789f3639f57b04f0d34fb213d17963ac4a97 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 2 Jan 2017 20:27:58 -0800 Subject: [PATCH] doc: edit writing-tests.md * Remove passive voice * Remove unneeded modifiers * Minor comma change PR-URL: https://github.com/nodejs/node/pull/10585 Reviewed-By: Gibson Fahnestock Reviewed-By: Italo A. Casas --- doc/guides/writing-tests.md | 61 +++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index d1328c70ea7f51..f27c076590e513 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -12,15 +12,15 @@ with code `0` on success. A test will fail if: - It never exits. In this case, the test runner will terminate the test because it sets a maximum time limit. -Tests can be added for multiple reasons: +Add tests when: -- When adding new functionality. -- When fixing regressions and bugs. -- When expanding test coverage. +- Adding new functionality. +- Fixing regressions and bugs. +- Expanding test coverage. ## Test structure -Let's analyze this very basic test from the Node.js test suite: +Let's analyze this basic test from the Node.js test suite: ```javascript 1 'use strict'; @@ -59,11 +59,12 @@ the nature of the test requires that the test run without it. The second line loads the `common` module. The `common` module is a helper module that provides useful tools for the tests. -Even if no functions or other properties exported by `common` are used in a -test, the `common` module should still be included. This is because the `common` -module includes code that will cause tests to fail if variables are leaked into -the global space. In situations where no functions or other properties exported -by `common` are used, it can be included without assigning it to an identifier: +Even if a test uses no functions or other properties exported by `common`, +the test should still include the `common` module before any other modules. This +is because the `common` module includes code that will cause a test to fail if +the test leaks variables into the global space. In situations where a test uses +no functions or other properties exported by `common`, include it without +assigning it to an identifier: ```javascript require('../common'); @@ -86,28 +87,28 @@ const assert = require('assert'); const http = require('http'); ``` -These modules are required for the test to run. Except for special cases, these -modules should only include core modules. -The `assert` module is used by most of the tests to check that the assumptions -for the test are met. -Note that require statements are sorted, in +The test checks functionality in the `http` module. + +Most tests use the `assert` module to confirm expectations of the test. + +The require statements are sorted in [ASCII](http://man7.org/linux/man-pages/man7/ascii.7.html) order (digits, upper case, `_`, lower case). ### **Lines 10-21** -This is the body of the test. This test is quite simple, it just tests that an +This is the body of the test. This test is simple, it just tests that an HTTP server accepts `non-ASCII` characters in the headers of an incoming request. Interesting things to notice: -- If the test doesn't depend on a specific port number then always use 0 instead - of an arbitrary value, as it allows tests to be run in parallel safely, as the - operating system will assign a random port. If the test requires a specific - port, for example if the test checks that assigning a specific port works as - expected, then it is ok to assign a specific port number. +- If the test doesn't depend on a specific port number, then always use 0 + instead of an arbitrary value, as it allows tests to run in parallel safely, + as the operating system will assign a random port. If the test requires a + specific port, for example if the test checks that assigning a specific port + works as expected, then it is ok to assign a specific port number. - The use of `common.mustCall` to check that some callbacks/listeners are called. -- The HTTP server is closed once all the checks have run. This way, the test can +- The HTTP server closes once all the checks have run. This way, the test can exit gracefully. Remember that for a test to succeed, it must exit with a status code of 0. @@ -115,20 +116,20 @@ request. Interesting things to notice: ### Timers -The use of timers is discouraged, unless timers are being tested. There are -multiple reasons for this. Mainly, they are a source of flakiness. For a thorough +Avoid timers unless the test is specifically testing timers. There are multiple +reasons for this. Mainly, they are a source of flakiness. For a thorough explanation go [here](https://github.com/nodejs/testing/issues/27). -In the event a timer is needed, it's recommended using the -`common.platformTimeout()` method, that allows setting specific timeouts +In the event a test needs a timer, consider using the +`common.platformTimeout()` method. It allows setting specific timeouts depending on the platform. For example: ```javascript const timer = setTimeout(fail, common.platformTimeout(4000)); ``` -will create a 4-seconds timeout, except for some platforms where the delay will -be multiplied for some factor. +will create a 4-second timeout on most platforms but a longer timeout on slower +platforms. ### The *common* API @@ -193,9 +194,9 @@ var server = http.createServer(common.mustCall(function(req, res) { ### Flags Some tests will require running Node.js with specific command line flags set. To -accomplish this, a `// Flags: ` comment should be added in the preamble of the +accomplish this, add a `// Flags: ` comment in the preamble of the test followed by the flags. For example, to allow a test to require some of the -`internal/*` modules, the `--expose-internals` flag should be added. +`internal/*` modules, add the `--expose-internals` flag. A test that would require `internal/freelist` could start like this: ```javascript