Skip to content

Commit

Permalink
tools: force common be required before any other modules
Browse files Browse the repository at this point in the history
PR-URL: #27650
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
ZYSzys authored and targos committed May 13, 2019
1 parent e74e661 commit 169ddc5
Show file tree
Hide file tree
Showing 52 changed files with 194 additions and 66 deletions.
1 change: 1 addition & 0 deletions test/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ rules:
node-core/number-isnan: error
## common module is mandatory in tests
node-core/required-modules: [error, common]
node-core/require-common-first: error
node-core/no-duplicate-requires: off

# Global scoped methods and vars
Expand Down
2 changes: 1 addition & 1 deletion test/async-hooks/hook-checks.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';
const assert = require('assert');
require('../common');
const assert = require('assert');

/**
* Checks the expected invocations against the invocations that actually
Expand Down
2 changes: 1 addition & 1 deletion test/async-hooks/verify-graph.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict';

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

function findInGraph(graph, type, n) {
let found = 0;
Expand Down
24 changes: 12 additions & 12 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ thread.
The `ArrayStream` module provides a simple `Stream` that pushes elements from
a given array.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
const ArrayStream = require('../common/arraystream');
const stream = new ArrayStream();
Expand All @@ -402,7 +402,7 @@ require a particular action to be taken after a given number of completed
tasks (for instance, shutting down an HTTP server after a specific number of
requests). The Countdown will fail the test if the remainder did not reach 0.

<!-- eslint-disable strict, node-core/required-modules -->
<!-- eslint-disable strict, node-core/require-common-first, node-core/required-modules -->
```js
const Countdown = require('../common/countdown');

Expand Down Expand Up @@ -574,7 +574,7 @@ one listed below. (`heap.validateSnapshotNodes(...)` is a shortcut for

Create a heap dump and an embedder graph copy and validate occurrences.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
validateSnapshotNodes('TLSWRAP', [
{
Expand All @@ -592,7 +592,7 @@ validateSnapshotNodes('TLSWRAP', [
The `hijackstdio` module provides utility functions for temporarily redirecting
`stdout` and `stderr` output.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
const { hijackStdout, restoreStdout } = require('../common/hijackstdio');

Expand Down Expand Up @@ -638,7 +638,7 @@ original state after calling [`hijackstdio.hijackStdOut()`][].
The http2.js module provides a handful of utilities for creating mock HTTP/2
frames for testing of HTTP/2 endpoints

<!-- eslint-disable no-unused-vars, node-core/required-modules -->
<!-- eslint-disable no-unused-vars, node-core/require-common-first, node-core/required-modules -->
```js
const http2 = require('../common/http2');
```
Expand All @@ -648,7 +648,7 @@ const http2 = require('../common/http2');
The `http2.Frame` is a base class that creates a `Buffer` containing a
serialized HTTP/2 frame header.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
// length is a 24-bit unsigned integer
// type is an 8-bit unsigned integer identifying the frame type
Expand All @@ -667,7 +667,7 @@ The serialized `Buffer` may be retrieved using the `frame.data` property.
The `http2.DataFrame` is a subclass of `http2.Frame` that serializes a `DATA`
frame.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
// id is the 32-bit stream identifier
// payload is a Buffer containing the DATA payload
Expand All @@ -684,7 +684,7 @@ socket.write(frame.data);
The `http2.HeadersFrame` is a subclass of `http2.Frame` that serializes a
`HEADERS` frame.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
// id is the 32-bit stream identifier
// payload is a Buffer containing the HEADERS payload (see either
Expand All @@ -702,7 +702,7 @@ socket.write(frame.data);
The `http2.SettingsFrame` is a subclass of `http2.Frame` that serializes an
empty `SETTINGS` frame.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
// ack is a boolean indicating whether or not to set the ACK flag.
const frame = new http2.SettingsFrame(ack);
Expand All @@ -715,7 +715,7 @@ socket.write(frame.data);
Set to a `Buffer` instance that contains a minimal set of serialized HTTP/2
request headers to be used as the payload of a `http2.HeadersFrame`.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
const frame = new http2.HeadersFrame(1, http2.kFakeRequestHeaders, 0, true);

Expand All @@ -727,7 +727,7 @@ socket.write(frame.data);
Set to a `Buffer` instance that contains a minimal set of serialized HTTP/2
response headers to be used as the payload a `http2.HeadersFrame`.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
const frame = new http2.HeadersFrame(1, http2.kFakeResponseHeaders, 0, true);

Expand All @@ -739,7 +739,7 @@ socket.write(frame.data);
Set to a `Buffer` containing the preamble bytes an HTTP/2 client must send
upon initial establishment of a connection.

<!-- eslint-disable no-undef, node-core/required-modules -->
<!-- eslint-disable no-undef, node-core/require-common-first, node-core/required-modules -->
```js
socket.write(http2.kClientMagic);
```
Expand Down
2 changes: 1 addition & 1 deletion test/common/arraystream.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

const { Stream } = require('stream');
Expand Down
2 changes: 1 addition & 1 deletion test/common/benchmark.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */

'use strict';

Expand Down
2 changes: 1 addition & 1 deletion test/common/countdown.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */

'use strict';

Expand Down
2 changes: 1 addition & 1 deletion test/common/dns.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

const assert = require('assert');
Expand Down
2 changes: 1 addition & 1 deletion test/common/duplexpair.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';
const { Duplex } = require('stream');
const assert = require('assert');
Expand Down
2 changes: 1 addition & 1 deletion test/common/fixtures.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

const path = require('path');
Expand Down
2 changes: 1 addition & 1 deletion test/common/heap.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';
const assert = require('assert');
const util = require('util');
Expand Down
2 changes: 1 addition & 1 deletion test/common/hijackstdio.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

// Hijack stdout and stderr
Expand Down
2 changes: 1 addition & 1 deletion test/common/http2.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

// An HTTP/2 testing tool used to create mock frames for direct testing
Expand Down
3 changes: 2 additions & 1 deletion test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

/* eslint-disable node-core/required-modules, node-core/crypto-check */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
/* eslint-disable node-core/crypto-check */
'use strict';
const process = global.process; // Some tests tamper with the process global.
const path = require('path');
Expand Down
2 changes: 1 addition & 1 deletion test/common/index.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */

import { createRequireFromPath } from 'module';
import { fileURLToPath as toPath } from 'url';
Expand Down
2 changes: 1 addition & 1 deletion test/common/internet.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

// Utilities for internet-related tests
Expand Down
2 changes: 1 addition & 1 deletion test/common/report.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';
const assert = require('assert');
const fs = require('fs');
Expand Down
3 changes: 2 additions & 1 deletion test/common/tls.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable node-core/required-modules, node-core/crypto-check */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
/* eslint-disable node-core/crypto-check */

'use strict';
const crypto = require('crypto');
Expand Down
2 changes: 1 addition & 1 deletion test/common/tmpdir.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

const fs = require('fs');
Expand Down
2 changes: 1 addition & 1 deletion test/common/wpt.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
'use strict';

const assert = require('assert');
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-example-loader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/example-loader.mjs
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
import assert from 'assert';
import ok from '../fixtures/es-modules/test-esm-ok.mjs';

Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-loader-dependency.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-with-dep.mjs
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
import '../fixtures/es-modules/test-esm-ok.mjs';

// We just test that this module doesn't fail loading
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
import './not-found.js';
2 changes: 1 addition & 1 deletion test/es-module/test-esm-preserve-symlinks-not-found.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
import './not-found.mjs';
2 changes: 1 addition & 1 deletion test/es-module/test-esm-resolve-hook.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/js-loader.mjs
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */
import { namedExport } from '../fixtures/es-module-loaders/js-as-esm.js';
import assert from 'assert';
import ok from '../fixtures/es-modules/test-esm-ok.mjs';
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-type-flag.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --experimental-modules
import cjs from '../fixtures/baz.js';
import '../common/index.mjs';
import cjs from '../fixtures/baz.js';
import { message } from '../fixtures/es-modules/message.mjs';
import assert from 'assert';

Expand Down
3 changes: 1 addition & 2 deletions test/js-native-api/test_general/testInstanceOf.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';
const fs = require('fs');

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

// Addon is referenced through the eval expression in testFile
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict';

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

if (process.env.NODE_PENDING_DEPRECATION)
common.skip('test does not work when NODE_PENDING_DEPRECATION is set');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Flags: --no-warnings
'use strict';

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

if (new Error().stack.includes('node_modules'))
common.skip('test does not work when inside `node_modules` directory');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-includes.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';
const assert = require('assert');
const common = require('../common');
const assert = require('assert');

const b = Buffer.from('abcdef');
const buf_a = Buffer.from('a');
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-child-process-spawn-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
// caused the third argument (`options`) to be ignored.
// See https://github.com/nodejs/node/issues/24912.

const assert = require('assert');
const { spawn } = require('child_process');

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

const assert = require('assert');
const { spawn } = require('child_process');

tmpdir.refresh();

const command = common.isWindows ? 'cd' : 'pwd';
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-child-process-spawnsync-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
// caused the third argument (`options`) to be ignored.
// See https://github.com/nodejs/node/issues/24912.

const assert = require('assert');
const { spawnSync } = require('child_process');

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

const assert = require('assert');
const { spawnSync } = require('child_process');

const command = common.isWindows ? 'cd' : 'pwd';
const options = { cwd: tmpdir.path };

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-dgram-deprecation-error.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict';

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

Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-eslint-require-common-first.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

common.skipIfEslintMissing();

const RuleTester = require('../../tools/node_modules/eslint').RuleTester;
const rule = require('../../tools/eslint-rules/require-common-first');

new RuleTester().run('require-common-first', rule, {
valid: [
{
code: 'require("common")\n' +
'require("assert")'
}
],
invalid: [
{
code: 'require("assert")\n' +
'require("common")',
errors: [{ message: 'Mandatory module "common" must be loaded ' +
'before any other modules.' }]
}
]
});
2 changes: 1 addition & 1 deletion test/parallel/test-global-console-exists.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable node-core/required-modules */
/* eslint-disable node-core/require-common-first, node-core/required-modules */

'use strict';

Expand Down
Loading

0 comments on commit 169ddc5

Please sign in to comment.