Narrow down scope of unit tests


Apr 17, 2023

PROPOSED

arisp8

Technical Stories:

Context and Problem Statement

Our integration tests accept a query document and produce an output. They are useful for testing that all the underlying dependencies are interacting nicely and are producing the outcome we want. If any dependency was missing or if functions are called in unexpected ways, these tests are likely to identify such issues.

Our unit tests on the other hand are not entirely consistent and sometimes look a lot like integration tests that use mock values. The wide scope of these tests means that we need to maintain a lot of mocks across different tests. Adding a new resolver or field that's used frequently might mean that multiple tests need to be updated to include new mocks.

This can be a time-consuming procedure for our team, and there's no clear benefit from these functional tests as a lot of what we're testing would also be covered by the integration tests.

Decision Drivers

  • Establishing a consistent approach to testing our backend server.
  • Making unit tests cleaner by keeping a narrow scope.
  • Improving dev productivity.

Considered Options

  • Establishing a 3-part testing approach: unit tests (test functions), "integration" tests (test multiple functions and how they interact) and "e2e" tests (test how the server responds to http calls)
  • Using a 2-part testing approoach: Using narrow unit tests to test resolver functions, queries and mutations. Using wider integration tests to test entire queries, ensure that functions are called correctly, and that the server is able to produce the response that we expect.

Decision Outcome

The proposed option is Option 2. The reasons we reject Option 1 are:

  • Integration tests with mocked values would have significant overlap with the integration tests that connect to the actual data sources.
  • We would still need to maintain unnecessary mock values just to get certain unrelated function calls to run.

Option 2 is preferred compared to our existing approach because it will make our tests more narrow, meaning that we won't need to update unit tests when we make unrelated changes. Unless we make changes to foundational types or schemas, having to update multiple unrelated unit tests should be an uncommon occurrence and should be flagged.

Positive Consequences

  • We'll have a consistent approach to testing.
  • Our unit tests will be able to deeply test that our functions work as expected.
  • Devs will save time from updating unrelated tests (and code reviews will be easier)

Negative Consequences

  • There's a chance that we'll lose a bit of coverage if we don't carefully migrate different checks to the integration tests.

An example of the old approach VS new approach

This example will better illustrate what we're moving towards.

Old approach

// set up mocks
const { data, errors } = await server.executeAsFlashpacker({
    query: FxRatesDocument,
});
// expectations are here

New approach

// set up mocks
const context = await server.mockContext({ userEmail: 'test-user@flashpack.com' });
const data = await fxRatesQuery({}, {}, context);
// expectations are here

Visualised

You can see a nice visualisation of what we're aiming for here.