This repository was archived by the owner on May 13, 2019. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 19
[RFC] doc: add How to write a node test
guide
#30
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
# How to write a Node.js test | ||
|
||
## What is a test? | ||
|
||
A test must be a node script that exercises a specific functionality provided by node and | ||
checks that it behaves as expected. It should return 0 on success, otherwise it will | ||
fail. A test will fail if: | ||
|
||
- It exits by calling `process.exit(code)` where `code != 0` | ||
- It exits due to an uncaught exception. | ||
- It never exits. In this case, the test runner will terminate the test because it sets a maximum time limit. | ||
|
||
They can be added for multiple reasons: | ||
|
||
- When adding new functionality. | ||
- When fixing regression and bugs. | ||
- Expanding test coverage. | ||
|
||
|
||
## Test structure | ||
|
||
Let's analyze this very basic test from the Node.js test suite: | ||
|
||
```javascript | ||
1 'use strict'; | ||
2 const common = require('../common'); | ||
3 const http = require('http'); | ||
4 const assert = require('assert'); | ||
5 | ||
6 const server = http.createServer(common.mustCall((req, res) => { | ||
7 res.end('ok'); | ||
8 })); | ||
9 server.listen(common.PORT, () => { | ||
10 http.get({ | ||
11 port: common.PORT, | ||
12 headers: {'Test': 'Düsseldorf'} | ||
13 }, common.mustCall((res) => { | ||
14 assert.equal(res.statusCode, 200); | ||
15 server.close(); | ||
16 })); | ||
17 }); | ||
``` | ||
|
||
**Lines 1-2** | ||
|
||
``` | ||
'use strict'; | ||
const common = require('../common'); | ||
``` | ||
|
||
These two lines are mandatory and should be included on every test. | ||
The `common` module is a helper module that provides useful tools for the tests. | ||
If for some reason, no functionality from `common` is used, it should still be included like this: | ||
|
||
``` | ||
require('../common'); | ||
```` | ||
|
||
Why? It checks for leaks of globals. | ||
|
||
**Lines 3-4** | ||
|
||
``` | ||
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. | ||
|
||
**Lines 6-17** | ||
|
||
This is the body of the test. This test is quite simple, it just tests that an HTTP server accepts `obs-text` characters in the headers of an incoming request. Interesting things to notice: | ||
|
||
- The use of `common.PORT` as the listening port. Always use `common.PORT` instead of using an arbitrary value, as it allows to run tests in parallel safely, as they are not trying to reuse the same port another test is already using. | ||
- 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 exit gracefully. Remember that for a test to succeed, it must exit with a status code of 0 | ||
|
||
## General recommendations | ||
|
||
### Timers | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps something about |
||
The use of timers is discouraged, unless we're testing timers. The reasons for this are multiple. 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 depending on the platform. For example: | ||
|
||
``` | ||
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. | ||
|
||
### The *common* API | ||
|
||
Make use of the helpers from the `common` module as much as possible. | ||
|
||
One interesting case is `common.mustCall`. The use of `common.mustCall` may avoid the use of extra variables and the corresponding assertions. Let's explain this with a real test from the test suite. | ||
|
||
```javascript | ||
'use strict'; | ||
var common = require('../common'); | ||
var assert = require('assert'); | ||
var http = require('http'); | ||
|
||
var request = 0; | ||
var response = 0; | ||
process.on('exit', function() { | ||
assert.equal(request, 1, 'http server "request" callback was not called'); | ||
assert.equal(response, 1, 'http request "response" callback was not called'); | ||
}); | ||
|
||
var server = http.createServer(function(req, res) { | ||
request++; | ||
res.end(); | ||
}).listen(common.PORT, function() { | ||
var options = { | ||
agent: null, | ||
port: this.address().port | ||
}; | ||
http.get(options, function(res) { | ||
response++; | ||
res.resume(); | ||
server.close(); | ||
}); | ||
}); | ||
``` | ||
|
||
This test could be greatly simplified by using `common.mustCall` like this: | ||
|
||
```javascript | ||
'use strict'; | ||
var common = require('../common'); | ||
var assert = require('assert'); | ||
var http = require('http'); | ||
|
||
var server = http.createServer(common.mustCall(function(req, res) { | ||
res.end(); | ||
})).listen(common.PORT, function() { | ||
var options = { | ||
agent: null, | ||
port: this.address().port | ||
}; | ||
http.get(options, common.mustCall(function(res) { | ||
res.resume(); | ||
server.close(); | ||
})); | ||
}); | ||
|
||
``` | ||
|
||
### 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 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. A test that would require `internal/freelist` could start like this: | ||
|
||
```javascript | ||
'use strict'; | ||
|
||
// Flags: --expose-internals | ||
|
||
require('../common'); | ||
const assert = require('assert'); | ||
const freelist = require('internal/freelist'); | ||
``` |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should indicate that
require
statements should always useconst
but note that there are existing older tests in the core suite that have yet to be updated.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My opinion only, but
const
vs.var
is a broader topic project-wide that doesn't need to necessarily be mentioned here. It can be, surely. But if there is a strong desire to make this doc as lean as possible, it can probably be left out just as well. I wouldn't hold up on publishing this over the issue either way.