Testing your Application: explain why interdependent tests are bad

[bajtos]

In https://github.com/strongloop/loopback4-example-getting-started/pull/2, the proposed acceptance tests are relying on the first test to fill the database with data that's used by subsequent tests:

  it('creates a todo', async () => {
    item = (await client
      .post('/todo')
      .send(payload)
      .expect(
        Object.assign({}, payload, {
          id: 1
        })
      )).body;
  });

  //...

  it('successfully deletes todos', async () => {
    await client.del(`/todo/${item.id}`).send();
    await client
      .get(`/todo/${item.id}`)
      .send()
      .expect(404);
  });

(see https://github.com/strongloop/loopback4-example-getting-started/pull/2#discussion_r147389550)

This is an anti-pattern to avoid:

• if the first test fails, all subsequent tests fail too (because data was not filled in), but with an unhelpful error message is confusing.
• it is not possible to run individual tests on their own, e.g. via it.only() or mocha -g "test name".

We should extend Data Handling in Testing your Application to mention this pattern and explain why it's a bad thing to do.

[bajtos]

Few notes from my chat with @kjdelisle about acceptance tests - we should capture it in docs too.

Would it be okay if it was just one full CRUD sweep?

I disagree! We should have one acceptance test for each CRUD endpoint. The test should call Repository and/or test-data-builder API to create whatever is the expected database state

So for example, if you are testing

.get(`/todo/${item.id}`)

then the test should go like this

beforeEach(givenEmptyDatabase);

it('...', async () => {
  const item = await givenProduct({title: 'Plan for today'});
  const response = await client.get(`/todo/${item.id}`).expect(200);
  expect(response.body).to.have.property('title', 'Plan for today');
});

Alternatively, you may want a more strict assertion, i.e. verify all properties in the body, something like expect(response.body).to.eql(item.toJSON()), but I am not sure if it's necessary and/or a good idea.

The deepEquals and containsDeep methods work fairly well So the key takeaway here is make builder functions that prep the database using the repo directly. And then I don't have to fudge the tests together.

[stale[bot]]

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

[stale[bot]]

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.