Skip to content

Commit

Permalink
avoid global timers by way of ESLint; closes #3342 (#3374)
Browse files Browse the repository at this point in the history
- Remove unused variables to avoid problems with #237 (and their associated inline ESLint directives)
- Use existing ESLint rules (`no-restricted-globals`, `no-restricted-modules` and `no-restricted-syntax`) to disallow global timer access.
- Removed "poison pill" tests in `test/unit/runnable.spec.js`.

For those rules, our config applies only to `bin/*` and `lib/**/*.js`.

- `no-restricted-globals`: Disallow bare references (including calls) to timers like `setTimeout`.
- `no-restricted-modules`: Disallow use of Node.js' `timers` module to get around the above restriction.
- `no-restricted-syntax`: 
  - Disallow calls of `global.setTimeout` and other timers
  - Disallow `new global.Date()`
  - Disallow any expressions involving `global.Date.*`, `global.setTimeout.*` and other timers

- add more files to markdown linting
- reorg tests, remove `fs` test from jsapi tests
  • Loading branch information
boneskull committed Dec 18, 2018
1 parent 468165c commit dac4e92
Show file tree
Hide file tree
Showing 13 changed files with 407 additions and 326 deletions.
36 changes: 36 additions & 0 deletions .eslintrc.yml
Expand Up @@ -28,12 +28,48 @@ overrides:
ecmaVersion: 6
env:
browser: no

- files:
- test/**/*.{js,mjs}
env:
mocha: yes
globals:
expect: no
- files:
- bin/*
- lib/**/*.js
rules:
no-restricted-globals:
- error
- name: setTimeout
message: &GH-237 See https://github.com/mochajs/mocha/issues/237
- name: clearTimeout
message: *GH-237
- name: setInterval
message: *GH-237
- name: clearInterval
message: *GH-237
- name: setImmediate
message: *GH-237
- name: clearImmediate
message: *GH-237
- name: Date
message: *GH-237
no-restricted-modules:
- error
- timers
no-restricted-syntax:
- error
# disallow `global.setTimeout()`, `global.setInterval()`, etc.
- selector: 'CallExpression[callee.object.name=global][callee.property.name=/(set|clear)(Timeout|Immediate|Interval)/]'
message: *GH-237
# disallow `new global.Date()`
- selector: 'NewExpression[callee.object.name=global][callee.property.name=Date]'
message: *GH-237
# disallow property access of `global.<timer>.*`
- selector: '*[object.object.name=global][object.property.name=/(Date|(set|clear)(Timeout|Immediate|Interval))/]:expression'
message: *GH-237

- files:
- test/**/*.mjs
parserOptions:
Expand Down
13 changes: 0 additions & 13 deletions lib/reporters/base.js
Expand Up @@ -18,19 +18,6 @@ var supportsColor = process.browser ? null : require('supports-color');

exports = module.exports = Base;

/**
* Save timer references to avoid Sinon interfering.
* See: https://github.com/mochajs/mocha/issues/237
*/

/* eslint-disable no-unused-vars, no-native-reassign */
var Date = global.Date;
var setTimeout = global.setTimeout;
var setInterval = global.setInterval;
var clearTimeout = global.clearTimeout;
var clearInterval = global.clearInterval;
/* eslint-enable no-unused-vars, no-native-reassign */

/**
* Check if both stdio streams are associated with a tty.
*/
Expand Down
6 changes: 0 additions & 6 deletions lib/reporters/html.js
Expand Up @@ -18,13 +18,7 @@ var escape = utils.escape;
* Save timer references to avoid Sinon interfering (see GH-237).
*/

/* eslint-disable no-unused-vars, no-native-reassign */
var Date = global.Date;
var setTimeout = global.setTimeout;
var setInterval = global.setInterval;
var clearTimeout = global.clearTimeout;
var clearInterval = global.clearInterval;
/* eslint-enable no-unused-vars, no-native-reassign */

/**
* Expose `HTML`.
Expand Down
7 changes: 0 additions & 7 deletions lib/reporters/xunit.js
Expand Up @@ -17,14 +17,7 @@ var path = require('path');
/**
* Save timer references to avoid Sinon interfering (see GH-237).
*/

/* eslint-disable no-unused-vars, no-native-reassign */
var Date = global.Date;
var setTimeout = global.setTimeout;
var setInterval = global.setInterval;
var clearTimeout = global.clearTimeout;
var clearInterval = global.clearInterval;
/* eslint-enable no-unused-vars, no-native-reassign */

/**
* Expose `XUnit`.
Expand Down
6 changes: 0 additions & 6 deletions lib/runnable.js
Expand Up @@ -8,15 +8,9 @@ var utils = require('./utils');
/**
* Save timer references to avoid Sinon interfering (see GH-237).
*/

/* eslint-disable no-unused-vars, no-native-reassign */
var Date = global.Date;
var setTimeout = global.setTimeout;
var setInterval = global.setInterval;
var clearTimeout = global.clearTimeout;
var clearInterval = global.clearInterval;
/* eslint-enable no-unused-vars, no-native-reassign */

var toString = Object.prototype.toString;

module.exports = Runnable;
Expand Down
2 changes: 2 additions & 0 deletions lib/stats-collector.js
Expand Up @@ -21,6 +21,8 @@
* @property {number} duration - number of msecs that testing took.
*/

var Date = global.Date;

/**
* Provides stats such as test duration, number of tests passed / failed etc., by listening for events emitted by `runner`.
*
Expand Down
3 changes: 2 additions & 1 deletion package-scripts.js
Expand Up @@ -36,7 +36,8 @@ module.exports = {
description: 'Run ESLint linter'
},
markdown: {
script: 'markdownlint "*.md" "docs/**/*.md" ".github/*.md"',
script:
'markdownlint "*.md" "docs/**/*.md" ".github/*.md" "lib/**/*.md" "test/**/*.md" "example/**/*.md"',
description: 'Run markdownlint linter'
}
},
Expand Down
17 changes: 17 additions & 0 deletions test/integration/color.spec.js
@@ -0,0 +1,17 @@
'use strict';

var childProcess = require('child_process');
var path = require('path');

describe('mocha binary', function() {
it('should not output colors to pipe', function(done) {
var command = [path.join('bin', 'mocha'), '--grep', 'missing-test'];
childProcess.execFile(process.execPath, command, function(err, stdout) {
if (err) return done(err);

expect(stdout, 'not to contain', '[90m');

done();
});
});
});
File renamed without changes.
1 change: 0 additions & 1 deletion test/jsapi/index.js
Expand Up @@ -18,7 +18,6 @@ mocha.addFile('test/unit/hook-sync.spec.js');
mocha.addFile('test/unit/hook-sync-nested.spec.js');
mocha.addFile('test/unit/hook-async.spec.js');
mocha.addFile('test/unit/duration.spec.js');
mocha.addFile('test/node-unit/fs.spec.js');
mocha.addFile('test/unit/globals.spec.js');
mocha.addFile('test/unit/timeout.spec.js');

Expand Down
23 changes: 0 additions & 23 deletions test/node-unit/color.spec.js

This file was deleted.

22 changes: 0 additions & 22 deletions test/node-unit/fs.spec.js

This file was deleted.

0 comments on commit dac4e92

Please sign in to comment.