Skip to content

Commit

Permalink
test: refactor tests for easier isolated execution
Browse files Browse the repository at this point in the history
Refactor tools/test.py to allow nested parallel/sequential/pummel tests.
Refactor tools/test.py to blacklist suites rather than whitelist.
Makefile now allows CI_JS_SUITES and CI_NATIVE_SUITES to be overridden.
Introduce special DEFAULT_JS_SUITES rather than maintaining list of suites.
Delete stale test-http-client-reconnect-bug.js test.
  • Loading branch information
Benjamin Coe committed Sep 16, 2017
1 parent 082c434 commit 94791e1
Show file tree
Hide file tree
Showing 466 changed files with 597 additions and 563 deletions.
40 changes: 36 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ expect throughout each step of the process.
* [Commit message guidelines](#commit-message-guidelines)
* [Step 5: Rebase](#step-5-rebase)
* [Step 6: Test](#step-6-test)
* [Step 6.5: Test Coverage](#step-65-test-coverage)
* [Step 7: Push](#step-7-push)
* [Step 8: Opening the Pull Request](#step-8-opening-the-pull-request)
* [Step 9: Discuss and Update](#step-9-discuss-and-update)
Expand Down Expand Up @@ -371,10 +372,16 @@ Bug fixes and features should always come with tests. A
provided to make the process easier. Looking at other tests to see how they
should be structured can also help.

The `test` directory within the `nodejs/node` repository is complex and it is
often not clear where a new test file should go. When in doubt, add new tests
to the `test/parallel/` directory and the right location will be sorted out
later.
Tests should be placed in a directory corresponding to the subsystem that
they exercise, e.g., `es-module` for ESM loader tests, `child-process` for
tests related to the `child-process` module. Tests can be further organized
into the following subtypes:

* `subsystem/parallel`: tests that can be run in parallel with each other.
Most new tests should be placed in this location.
* `subsystem/pummel`: tests that should be run while the system is under
heavy load.
* `subsystem/sequential`: tests that should be run sequentially.

Before submitting your changes in a Pull Request, always run the full Node.js
test suite. To run the tests (including code linting) on Unix / macOS:
Expand Down Expand Up @@ -420,6 +427,31 @@ $ ./node ./test/parallel/test-stream2-transform.js
Remember to recompile with `make -j4` in between test runs if you change code in
the `lib` or `src` directories.

#### Step 6.5: Test coverage

A great way to find nooks and crannies of the codebase that could use more
testing is to use coverage reports. To generate a coverage report simply
run:

```console
$ ./configure --coverage
$ make coverage
```

Afterwards a detailed HTML coverage report can be found in
`coverage/index.html`.

It is also possible to generate coverage for a specific subsystem. For instance,
the following generates coverage for the `child-process` submodule:

```console
$ ./configure --coverage
$ CI_JS_SUITES=child-process* CI_NATIVE_SUITES= make coverage
```

recent coverage reports can be found at
[coverage.nodejs.org](https://coverage.nodejs.org/).

#### Step 7: Push

Once you are sure your commits are ready to go, with passing tests and linting,
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ test-all: test-build test/gc/build/Release/binding.node
test-all-valgrind: test-build
$(PYTHON) tools/test.py --mode=debug,release --valgrind

CI_NATIVE_SUITES := addons addons-napi
CI_JS_SUITES := abort async-hooks doctool inspector known_issues message parallel pseudo-tty sequential
CI_NATIVE_SUITES ?= addons addons-napi
CI_JS_SUITES ?= DEFAULT_JS_SUITES

# Build and test addons without building anything else
test-ci-native: LOGLEVEL := info
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';
// Flags: --expose_internals
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const cp = require('child_process');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');

function pwd(callback) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const { ChildProcess } = require('child_process');
assert.strictEqual(typeof ChildProcess, 'function');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --expose_internals
'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const internalCp = require('internal/child_process');
const oldSpawnSync = internalCp.spawnSync;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');

let returns = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');

const spawn = require('child_process').spawn;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
require('../../common');
const assert = require('assert');
const fixtures = require('../common/fixtures');
const fixtures = require('../../common/fixtures');

const spawn = require('child_process').spawn;
const childPath = fixtures.path('parent-process-nonpersistent.js');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const fork = require('child_process').fork;
const net = require('net');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const os = require('os');
const util = require('util');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');

const spawn = require('child_process').spawn;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const exec = require('child_process').exec;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const cp = require('child_process');
const stdoutData = 'foo';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const exec = require('child_process').exec;
let success_count = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const child_process = require('child_process');

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';
// Flags: --expose_internals
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const cp = require('child_process');

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const cp = require('child_process');

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';
// Refs: https://github.com/nodejs/node/issues/7342
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const exec = require('child_process').exec;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const cp = require('child_process');

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const execFile = require('child_process').execFile;
const uv = process.binding('uv');
const fixtures = require('../common/fixtures');
const fixtures = require('../../common/fixtures');

const fixture = fixtures.path('exit.js');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const spawn = require('child_process').spawn;
const fixtures = require('../common/fixtures');
const fixtures = require('../../common/fixtures');

const exitScript = fixtures.path('exit.js');
const exitChild = spawn(process.argv[0], [exitScript, 23]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
const common = require('../../common');
const cp = require('child_process');
const assert = require('assert');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const { fork, spawn } = require('child_process');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const fork = require('child_process').fork;
const fixtures = require('../common/fixtures');
const fixtures = require('../../common/fixtures');

const cp = fork(fixtures.path('child-process-message-and-exit.js'));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* child process.
*/

const common = require('../common');
const common = require('../../common');
if (common.isWindows)
common.skip('Sending dgram sockets to child processes is not supported');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
require('../../common');
const assert = require('assert');
const child_process = require('child_process');
const spawn = child_process.spawn;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const fork = require('child_process').fork;
const net = require('net');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const fork = require('child_process').fork;
const net = require('net');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';
// This test verifies that the shell option is not supported by fork().
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const cp = require('child_process');
const expected = common.isWindows ? '%foo%' : '$foo';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
require('../../common');
const assert = require('assert');
const fork = require('child_process').fork;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const fork = require('child_process').fork;

if (process.argv[2] === 'child') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// (asynchronously) to use the closed channel to it's creator caused a segfault.
'use strict';

const common = require('../common');
const common = require('../../common');
const assert = require('assert');

const cluster = require('cluster');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
'use strict';
const common = require('../common');
const common = require('../../common');

// Ensures that child_process.fork can accept string
// variant of stdio parameter in options object and
// throws a TypeError when given an unexpected string

const assert = require('assert');
const fork = require('child_process').fork;
const fixtures = require('../common/fixtures');
const fixtures = require('../../common/fixtures');

const childScript = fixtures.path('child-process-spawn-node');
const errorRegexp = /^TypeError: Incorrect value of stdio option:/;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const cp = require('child_process');
const net = require('net');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
const common = require('../common');
const common = require('../../common');
const assert = require('assert');
const fork = require('child_process').fork;
const args = ['foo', 'bar'];
const fixtures = require('../common/fixtures');
const fixtures = require('../../common/fixtures');

const n = fork(fixtures.path('child-process-spawn-node.js'), args);

Expand Down
Loading

0 comments on commit 94791e1

Please sign in to comment.