Skip to content

Commit

Permalink
doc: edit writing-tests.md
Browse files Browse the repository at this point in the history
* Remove passive voice
* Remove unneeded modifiers
* Minor comma change

PR-URL: nodejs#10585
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
  • Loading branch information
Trott authored and italoacasas committed Jan 24, 2017
1 parent 8ca4990 commit da9b692
Showing 1 changed file with 31 additions and 30 deletions.
61 changes: 31 additions & 30 deletions doc/guides/writing-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit da9b692

Please sign in to comment.