Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: add linting rule for before/beforeEach just like the async test rule #151

Merged
merged 5 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ You can add rules:
"cypress/assertion-before-screenshot": "warn",
"cypress/no-force": "warn",
"cypress/no-async-tests": "error",
"cypress/no-async-before": "error",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this issue also occur in after/afterEach lifecycle hooks? Maybe no-async-hooks might be a better rule, if this is the case.

Copy link
Contributor Author

@Arcturus5404 Arcturus5404 Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so, but I am not sure they will cause a problem. It might has the same potential to break your testcase. Should the new hook rule also include the test rule? one to rule them all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do know that in before and beforeEach the async code might actually break the test, because of the dependency. But there might not be a problem as such in the after because of race conditions

"cypress/no-pause": "error"
}
}
Expand Down
52 changes: 52 additions & 0 deletions docs/rules/no-async-before.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Prevent using async/await in Cypress test cases (no-async-tests)

Cypress commands that return a promise may cause side effects in before/beforeEach hooks, possibly causing unexpected behavior.

## Rule Details

This rule disallows using `async` `before` and `beforeEach` functions.

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

```js
describe('my feature', () => {
before('my test case', async () => {
await cy.get('.myClass')
// other operations
})
})
```

```js
describe('my feature', () => {
before('my test case', async () => {
cy
.get('.myClass')
.click()

await someAsyncFunction()
})
})
```

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

```js
describe('my feature', () => {
before('my test case', () => {
cy.get('.myClass')
// other operations
})
})

```

## When Not To Use It

If there are genuine use-cases for using `async/await` in your before then you may not want to include this rule (or at least demote it to a warning).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If there are genuine use-cases for using `async/await` in your before then you may not want to include this rule (or at least demote it to a warning).
If there are genuine use-cases for using `async/await` in your `before/beforeEach` function callback, then you may not want to include this rule (or at least demote it to a warning).


## Further Reading

- [Commands Are Asynchronous](https://docs.cypress.io/guides/core-concepts/introduction-to-cypress.html#Commands-Are-Asynchronous)
- [Commands Are Promises](https://docs.cypress.io/guides/core-concepts/introduction-to-cypress.html#Commands-Are-Promises)
- [Commands Are Not Promises](https://docs.cypress.io/guides/core-concepts/introduction-to-cypress.html#Commands-Are-Not-Promises)
47 changes: 47 additions & 0 deletions lib/rules/no-async-before.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict'

module.exports = {
meta: {
docs: {
description: 'Prevent using async/await in Cypress before methods',
category: 'Possible Errors',
recommended: true,
},
messages: {
unexpected: 'Avoid using async functions with Cypress before / beforeEach functions',
},
},

create (context) {
function isBeforeBlock (callExpressionNode) {
const { type, name } = callExpressionNode.callee

return type === 'Identifier'
&& name === 'before' || name === 'beforeEach'
}

function isBeforeAsync (node) {
return node.arguments
&& node.arguments.length >= 2
&& node.arguments[1].async === true
}

return {
Identifier (node) {
if (node.name === 'cy' || node.name === 'Cypress') {
const ancestors = context.getAncestors()
const asyncTestBlocks = ancestors
.filter((n) => n.type === 'CallExpression')
.filter(isBeforeBlock)
.filter(isBeforeAsync)

if (asyncTestBlocks.length >= 1) {
asyncTestBlocks.forEach((node) => {
context.report({ node, messageId: 'unexpected' })
})
}
}
},
}
},
}
25 changes: 25 additions & 0 deletions tests/lib/rules/no-async-before.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict'

const rule = require('../../../lib/rules/no-async-before')
const RuleTester = require('eslint').RuleTester

const ruleTester = new RuleTester()

const errors = [{ messageId: 'unexpected' }]
// async functions are an ES2017 feature
const parserOptions = { ecmaVersion: 8 }

ruleTester.run('no-async-before', rule, {
valid: [
{ code: 'before(\'a before case\', () => { cy.get(\'.someClass\'); })', parserOptions },
{ code: 'before(\'a before case\', async () => { await somethingAsync(); })', parserOptions },
{ code: 'async function nonTestFn () { return await somethingAsync(); }', parserOptions },
{ code: 'const nonTestArrowFn = async () => { await somethingAsync(); }', parserOptions },
],
invalid: [
{ code: 'before(\'a test case\', async () => { cy.get(\'.someClass\'); })', parserOptions, errors },
{ code: 'beforeEach(\'a test case\', async () => { cy.get(\'.someClass\'); })', parserOptions, errors },
{ code: 'before(\'a test case\', async function () { cy.get(\'.someClass\'); })', parserOptions, errors },
{ code: 'beforeEach(\'a test case\', async function () { cy.get(\'.someClass\'); })', parserOptions, errors },
],
})