Skip to content

Commit

Permalink
feat(no-expect-in-setup-teardown): add no-expect-in-setup-teardown rule
Browse files Browse the repository at this point in the history
  • Loading branch information
alecxe authored and Nicolas Fernandez committed Dec 2, 2016
1 parent be6454b commit a6768c5
Show file tree
Hide file tree
Showing 5 changed files with 230 additions and 15 deletions.
28 changes: 15 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,20 @@ configuration files.

### Rules

Rule | Recommended | Options
---- | ----------- | -------
[named-spy][] | 0 |
[no-focused-tests][] | 2 |
[no-disabled-tests][] | 1 |
[no-suite-dupes][] | 1, `'block'` | `['block', 'branch']`
[no-spec-dupes][] | 1, `'block'` | `['block', 'branch']`
[missing-expect][] | 0, `'expect()'` | expectation function names
[no-suite-callback-args][] | 2 |
[valid-expect][] | 1 |
[no-assign-spyon][] | 0 |
[no-unsafe-spy][] | 1 |
[no-global-setup][] | 2 |
Rule | Recommended | Options
---- | ----------- | -------
[named-spy][] | 0 |
[no-focused-tests][] | 2 |
[no-disabled-tests][] | 1 |
[no-suite-dupes][] | 1, `'block'` | `['block', 'branch']`
[no-spec-dupes][] | 1, `'block'` | `['block', 'branch']`
[missing-expect][] | 0, `'expect()'` | expectation function names
[no-suite-callback-args][] | 2 |
[valid-expect][] | 1 |
[no-assign-spyon][] | 0 |
[no-unsafe-spy][] | 1 |
[no-global-setup][] | 2 |
[no-expect-in-setup-teardown][] | 0, `'expect()'` | expectation function names
For example, using the recommended configuration, the `no-focused-tests` rule
is enabled and will cause ESLint to throw an error (with an exit code of `1`)
Expand Down Expand Up @@ -110,6 +111,7 @@ See [configuring rules][] for more information.
[no-assign-spyon]: docs/rules/no-assign-spyon.md
[no-unsafe-spy]: docs/rules/no-unsafe-spy.md
[no-global-setup]: docs/rules/no-global-setup.md
[no-expect-in-setup-teardown]: docs/rules/no-expect-in-setup-teardown.md
[configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules
Expand Down
48 changes: 48 additions & 0 deletions docs/rules/no-expect-in-setup-teardown.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Discourage making expectations in setup and teardown functions (no-expect-in-setup-teardown)

Making expectations inside test setup or teardown functions is sometimes a sign of a bad test design.

It might negatively impact readability as a reader might think you have the actual test logic splattered between setup, teardown and the test functions themselves.

Sometimes there is actually a need to test the result of a setup/teardown function call. In cases like these, it is recommended to create a separate test to test the setup/teardown process.

## Rule details

This rule triggers a warning if there is an `expect()` call inside `beforeEach()`, `afterEach()`, `beforeAll()` and `afterAll()`.

An array of expect function names may be passed to the configuration of this rule. By default only `expect()` is used.

The following patterns are considered warnings:

```js
beforeEach(function() { expect(true).toBe(true); });
afterEach(function() { expect(true).toBe(true); });
beforeAll(function() { expect(true).toBe(true); });
afterAll(function() { expect(true).toBe(true); });
```

The following patterns are not warnings:

```js
beforeEach(function() { someOtherFunction(); });
afterEach(function() {});
beforeAll(function() { someOtherFunction(); });
afterAll(function() {});
```

### AngularJS `$httpBackend` rule configuration example

```yaml
rules:
no-expect-in-setup-teardown:
- 2
- expect()
- $httpBackend.expect()
- $httpBackend.expectDELETE()
- $httpBackend.expectGET()
- $httpBackend.expectJSONP()
- $httpBackend.expectHEAD()
- $httpBackend.expectPATCH()
- $httpBackend.expectPOST()
- $httpBackend.expectPUT()
```
6 changes: 4 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ module.exports = {
'valid-expect': require('./lib/rules/valid-expect'),
'no-assign-spyon': require('./lib/rules/no-assign-spyon'),
'no-unsafe-spy': require('./lib/rules/no-unsafe-spy'),
'no-global-setup': require('./lib/rules/no-global-setup')
'no-global-setup': require('./lib/rules/no-global-setup'),
'no-expect-in-setup-teardown': require('./lib/rules/no-expect-in-setup-teardown')
},
configs: {
recommended: {
Expand All @@ -27,7 +28,8 @@ module.exports = {
'jasmine/valid-expect': 1,
'jasmine/no-assign-spyon': 0,
'jasmine/no-unsafe-spy': 1,
'jasmine/no-global-setup': 2
'jasmine/no-global-setup': 2,
'jasmine/no-expect-in-setup-teardown': 0
}
}
}
Expand Down
50 changes: 50 additions & 0 deletions lib/rules/no-expect-in-setup-teardown.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use strict'

/**
* @fileoverview Discourage having expect in setup and teardown functions
* @author Alexander Afanasyev
*/

module.exports = function (context) {
var allowed = context.options.length ? context.options : ['expect()']
var setupRegexp = /^(before|after)(Each|All)$/

var insideSetup = false
var setupFunctionName

function buildName (node) {
if (node.type === 'CallExpression') {
return buildName(node.callee) + '()'
}
if (node.type === 'MemberExpression') {
return buildName(node.object) + '.' + node.property.name
}
if (node.type === 'Identifier') {
return node.name
}
}

return {
CallExpression: function (node) {
if (setupRegexp.test(node.callee.name)) {
insideSetup = true
setupFunctionName = node.callee.name
return
}

if (insideSetup) {
var functionName = buildName(node)
if (allowed.indexOf(functionName) > -1) {
context.report(node, 'Unexpected "' + functionName + '" call in "' + setupFunctionName + '()"')
}
}
},

'CallExpression:exit': function (node) {
if (setupRegexp.test(node.callee.name)) {
insideSetup = false
setupFunctionName = undefined
}
}
}
}
113 changes: 113 additions & 0 deletions test/rules/no-expect-in-setup-teardown.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
'use strict'

var rule = require('../../lib/rules/no-expect-in-setup-teardown')
var RuleTester = require('eslint').RuleTester

var eslintTester = new RuleTester()

eslintTester.run('no-expect-in-setup-teardown', rule, {
valid: [
'beforeEach(function() {});',
'afterEach(function() {});',
'beforeAll(function() {});',
'afterAll(function() {});',
'it("", function() { expect(true).toBe(true); })',
'beforeEach(function() { someOtherFunction(); });',
'afterEach(function() { someOtherFunction(); });',
'beforeAll(function() { someOtherFunction(); });',
'afterAll(function() { someOtherFunction(); });',
'expect(true).toBe(true);',
{
code: 'it("", function() {$httpBackend.expectGET();})',
options: [
'$httpBackend.expectGET()'
]
},
{
code: 'it("", function() {return $httpBackend.expectGET();})',
options: [
'$httpBackend.expectGET()'
]
},
{
code: 'it("", function() {a.deeply.nested().expect.expression();})',
options: [
'a.deeply.nested().expect.expression()'
]
},
{
code: 'it("", function() { expect(true).toBe(true); })',
options: [
'a.deeply.nested().expect.expression()'
]
}
],

invalid: [
{
code: 'beforeEach(function() { expect(true).toBe(true); });',
errors: [
{
message: 'Unexpected "expect()" call in "beforeEach()"'
}
]
},
{
code: 'afterEach(function() { expect(true).toBe(true); });',
errors: [
{
message: 'Unexpected "expect()" call in "afterEach()"'
}
]
},
{
code: 'beforeAll(function() { expect(true).toBe(true); });',
errors: [
{
message: 'Unexpected "expect()" call in "beforeAll()"'
}
]
},
{
code: 'afterAll(function() { expect(true).toBe(true); });',
errors: [
{
message: 'Unexpected "expect()" call in "afterAll()"'
}
]
},
{
code: 'beforeEach(function() {$httpBackend.expectGET();})',
options: [
'$httpBackend.expectGET()'
],
errors: [
{
message: 'Unexpected "$httpBackend.expectGET()" call in "beforeEach()"'
}
]
},
{
code: 'beforeAll(function() {return $httpBackend.expectGET();})',
options: [
'$httpBackend.expectGET()'
],
errors: [
{
message: 'Unexpected "$httpBackend.expectGET()" call in "beforeAll()"'
}
]
},
{
code: 'afterEach(function() {a.deeply.nested().expect.expression()})',
options: [
'a.deeply.nested().expect.expression()'
],
errors: [
{
message: 'Unexpected "a.deeply.nested().expect.expression()" call in "afterEach()"'
}
]
}
]
})

0 comments on commit a6768c5

Please sign in to comment.