Skip to content

Commit

Permalink
Merge pull request #507 from scottkidder/no-pause-test
Browse files Browse the repository at this point in the history
Add new `no-pause-test` rule
  • Loading branch information
bmish committed Sep 17, 2019
2 parents 1b14f5f + 0d1829f commit 72a184a
Show file tree
Hide file tree
Showing 6 changed files with 195 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ The `--fix` option on the command line automatically fixes problems reported by

| | Rule ID | Description |
|:---|:--------|:------------|
| | [no-pause-test](./docs/rules/no-pause-test.md) | Disallow use of `pauseTest` helper in tests. |
| | [no-test-and-then](./docs/rules/no-test-and-then.md) | Disallow use of `andThen` test wait helper. |
| | [no-test-import-export](./docs/rules/no-test-import-export.md) | Disallow importing of "-test.js" in a test file and exporting from a test file. |

Expand Down
56 changes: 56 additions & 0 deletions docs/rules/no-pause-test.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Disallow use of `pauseTest` helper in tests (no-pause-test)

When `pauseTest()` is committed and run in CI it can cause runners to hang which is undesirable.


## Rule Details

This rule aims to prevent `pauseTest()` from being committed and run in CI.

Examples of **incorrect** code for this rule:

```js
import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';

import { pauseTest } from '@ember/test-helpers';

module('Acceptance | foo test', function(hooks) {
setupApplicationTest(hooks);

test('it hangs', async function() {
await this.pauseTest();
// or
await pauseTest();
});
});
```


## When Not To Use It

If you have tests that call `resumeTest()` following a `pauseTest()`

```js
import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';

import { pauseTest, resumeTest } from '@ember/test-helpers';

module('Acceptance | foo test', function(hooks) {
setupApplicationTest(hooks);

test('it runs', async function() {
const promise = pauseTest();

// Do some stuff

resumeTest(); // Done
});
});
```


## Further Reading

* [ember-test-helpers pauseTest documentation](https://github.com/emberjs/ember-test-helpers/blob/master/API.md#pausetest)
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ module.exports = {
'no-observers': require('./rules/no-observers'),
'no-old-shims': require('./rules/no-old-shims'),
'no-on-calls-in-components': require('./rules/no-on-calls-in-components'),
'no-pause-test': require('./rules/no-pause-test'),
'no-proxies': require('./rules/no-proxies'),
'no-restricted-resolver-tests': require('./rules/no-restricted-resolver-tests'),
'no-side-effects': require('./rules/no-side-effects'),
Expand Down
3 changes: 2 additions & 1 deletion lib/recommended-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module.exports = {
"ember/no-observers": "error",
"ember/no-old-shims": "error",
"ember/no-on-calls-in-components": "error",
"ember/no-pause-test": "off",
"ember/no-proxies": "off",
"ember/no-restricted-resolver-tests": "error",
"ember/no-side-effects": "error",
Expand All @@ -56,4 +57,4 @@ module.exports = {
"ember/use-brace-expansion": "error",
"ember/use-ember-data-rfc-395-imports": "off",
"ember/use-ember-get-and-set": "off"
}
}
47 changes: 47 additions & 0 deletions lib/rules/no-pause-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';

const emberUtils = require('../utils/ember');
const types = require('../utils/types');

const ERROR_MESSAGE = 'Do not use `pauseTest()`';

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Disallow use of `pauseTest` helper in tests.',
category: 'Testing',
recommended: false,
},
fixable: null,
},

ERROR_MESSAGE,

create(context) {
if (!emberUtils.isTestFile(context.getFilename())) {
return {};
}

return {
CallExpression(node) {
const { callee } = node;

if (
(types.isIdentifier(callee) && callee.name === 'pauseTest') ||
(types.isThisExpression(callee.object) &&
types.isIdentifier(callee.property) &&
callee.property.name === 'pauseTest')
) {
context.report({
node,
message: ERROR_MESSAGE,
});
}
},
};
},
};
88 changes: 88 additions & 0 deletions tests/lib/rules/no-pause-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
'use strict';

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require('../../../lib/rules/no-pause-test');
const RuleTester = require('eslint').RuleTester;

const { ERROR_MESSAGE } = rule;

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const TEST_FILE_NAME = 'some-test.js';

let ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2018,
sourceType: 'module',
},
});

ruleTester.run('no-pause-test', rule, {
valid: [
{
filename: TEST_FILE_NAME,
code: "run(() => { console.log('Hello World.'); });",
},
{
filename: TEST_FILE_NAME,
code: 'myCustomClass.pauseTest(myFunction);',
},
{
filename: TEST_FILE_NAME,
code: 'pauseTest.otherFunction(myFunction);',
},
{
filename: 'not-a-test-file.js',
code: 'async () => { await this.pauseTest(); }',
},
],
invalid: [
{
filename: TEST_FILE_NAME,
code: 'async () => { await this.pauseTest(); }',
output: null,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
filename: TEST_FILE_NAME,
code: 'async () => { await pauseTest(); }',
output: null,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
filename: TEST_FILE_NAME,
code: `
test('foo', async function(assert) {
await this.pauseTest();
});
`,
output: null,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
filename: TEST_FILE_NAME,
code: `
test('bar', async function(assert) {
await pauseTest();
});
`,
output: null,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
filename: TEST_FILE_NAME,
code: `
test('baz', function(assert) {
this.pauseTest();
});
`,
output: null,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
],
});

0 comments on commit 72a184a

Please sign in to comment.