doc: edit writing-tests.md

Trott · MylesBorins · commit 1975f8216824 · 2017-03-08T17:10:22.000-08:00
* Remove passive voice * Remove unneeded modifiers * Minor comma change PR-URL: #10585 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com>
diff --git 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,49 +87,49 @@ 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.
 
 ## General recommendations
 
 ### 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
