From 23edfe00b3bfb2d464c25dd4bd4c24a6f56849ad Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
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 <gibfahn@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
---
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 c76d600e6e..c587eb3d8e 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 http = require('http');
const assert = require('assert');
```
-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