Skip to content

Commit

Permalink
Merge pull request #102 from cypress-io/issue-100
Browse files Browse the repository at this point in the history
Antipatterns / Best practices
  • Loading branch information
brian-mann committed Sep 24, 2017
2 parents d432d27 + 3823027 commit ec5fea4
Showing 1 changed file with 173 additions and 0 deletions.
173 changes: 173 additions & 0 deletions source/guides/references/best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ WIP.
WIP.
## Running code in `after` or `afterEach` hooks
WIP.
## Overusing `.then()`
WIP.
Expand Down Expand Up @@ -73,6 +77,175 @@ WIP.
WIP. -->

## Having tests rely on the state of previous tests

{% note danger %}
{% fa fa-warning %} **Anti-Pattern:** Coupling multiple tests together.
{% endnote %}

{% note success %}
{% fa fa-check-circle %} **Best Practice:** Tests should always be able to be run independently from one another **and still pass**.
{% endnote %}

You only need to do one thing to know whether you've coupled your tests incorrectly, or if one test is relying on the state of a previous one.

Simply put an `.only` on the test and refresh the browser.

If this test can run **by itself** and pass - congratulations you have written a good test.

If this is not the case, then you should refactor and change your approach.

How to solve this:

- Move repeated code in previous tests to `before` or `beforeEach` hooks.
- Combine multiple tests into one larger test.

Let's imagine the following test that is filling out the form.


```javascript
// an example of what NOT TO DO
describe('my form', function () {
it('visits the form', function () {
cy.visit('/users/new')
})

it('requires first name', function () {
cy.get('#first').type('Johnny')
})

it('requires last name', function () {
cy.get('#last').type('Appleseed')
})

it('can submit a valid form', function () {
cy.get('form').submit()
})
})
```

What's wrong with the above tests? They are all coupled together!

If you were to put an `.only` on any of the last three tests, they would fail. Each test requires the previous to run in a specific order in order to pass.

Here's 2 ways we can fix this:

***1. Combine into one test***

```javascript
// a bit better
describe('my form', function () {
it('can submit a valid form', function () {
cy.visit('/users/new')

cy.log('filling out first name') // if you really need this
cy.get('#first').type('Johnny')

cy.log('filling out last name') // if you really need this
cy.get('#last').type('Appleseed')

cy.log('submitting form') // if you really need this
cy.get('form').submit()
})
})
```

Now we can put an `.only` on this test and it will run successfully irrespective of any other test. The ideal Cypress workflow is writing and iterating on a single test at a time.

***2. Run shared code before each test***

```javascript
describe('my form', function () {
beforeEach(function () {
cy.visit('/users/new')
cy.get('#first').type('Johnny')
cy.get('#last').type('Appleseed')
})

it('displays form validation', function () {
cy.get('#first').clear() // clear out first name
cy.get('form').submit()
cy.get('#errors').should('contain', 'First name is required')
})

it('can submit a valid form', function () {
cy.get('form').submit()
})
})
```

This above example is ideal because now we are resetting the state between each test and ensuring nothing in previous tests leaks into subsequent ones.

We're also paving the way to make it easy to write multiple tests against the "default" state of the form. That way each test stays lean but each can be run independently and pass.

## Creating "tiny" tests with a single assertion

{% note danger %}
{% fa fa-warning %} **Anti-Pattern:** Acting like you're writing unit tests.
{% endnote %}

{% note success %}
{% fa fa-check-circle %} **Best Practice:** Add multiple assertions and don't worry about it
{% endnote %}

We've seen many users writing this kind of code:

```javascript
describe('my form', function () {
before(function () {
cy.visit('/users/new')
cy.get('#first').type('johnny')
})

it('has validation attr', function () {
cy.get('#first').should('have.attr', 'data-validation', 'required')
})

it('has active class', function () {
cy.get('#first').should('have.class', 'active')
})

it('has formatted first name', function () {
cy.get('#first').should('have.value', 'Johnny') // capitalized first letter
})
})
```

While technically this runs fine - this is really excessive, and not performant.

Why you did this pattern in unit tests:

- When assertions failed you relied on the test's title to know what failed
- You were told that adding multiple assertions was bad and accepted this as truth
- There was no performance penalty splitting up multiple tests because they run really fast

Why you shouldn't do this in Cypress:

- Writing integration tests is not the same as unit tests
- You will always know (and can visually see) which assertion failed in a large test
- Cypress runs a series of async lifecycle events that reset state between tests
- Resetting tests is much slower than simply adding more assertions

It is common for tests in Cypress to issue 30+ commands. Because nearly every command has a default assertion (and can therefore fail), even by limiting your assertions you're not saving yourself anything because **any single command could implicitly fail**.

How you should rewrite those tests:

```javascript
describe('my form', function () {
before(function () {
cy.visit('/users/new')
})

it('validates and formats first name', function () {
cy.get('#first')
.type('johnny')
.should('have.attr', 'data-validation', 'required')
.and('have.class', 'active')
.and('have.value', 'Johnny')
})
})
```

## Unnecessary Waiting

{% note danger %}
Expand Down

0 comments on commit ec5fea4

Please sign in to comment.