Skip to main content

Standardise Components Tests

Proposed change

After taking a look at our tests, it is possible to understand we don't have a unique way of structuring our test files.

The goal of this proposal is to point some possible guidelines for the way we create and structure the test files for our components.

This will only apply to component tests, all the other tests (like transformers, for example) can be discussed later.

renderComponent() placement

How it looks now

Usually, we have a function in our test files called renderComponent which calls renderWithProviders to render the component.

In some files, we are using an arrow function, in other a regular named function.

Also, we are not consistent about the placement it (sometimes is the 1st thing in the file, other times is the last).

Alternative

  • Use a regular function instead of an arrow function.
  • Move renderComponent function to the bottom of the file.

Why? Since the goal of the file is to have tests, it would be nice to open the file and see the tests instead of helper functions on top. It's the same thought of not having helper functions on the top of the files in the components.

renderComponent() with component parameters

How it looks now

We don't have a unique way of passing the component parameters to the renderComponent method. Sometimes we use an object, other times regular function parameters.

function renderComponent(address: string)
function renderComponent({ isLoading, hasError, title })

Alternative 1

Since now we are using TypeScript we can try to take advantage of the type system.

With this approach, we will use the same Props interface as the component, turning mandatory to use the same data types.

import SaltedCaramelIceCream, { Props } from './SaltedCaramelIceCream';

...

describe(...
it(..., () => {
renderComponent({ withTopping: false });
...
})
)
...

function renderComponent(props: Props) {
return render(<SaltedCaramelIceCream {...props} />)
}

renderComponent() with context parameters

How it looks now

We don't have a unique way of passing context parameters to the renderComponent method. Sometimes we use an object, other times regular function parameters.

function renderComponent(isLoading: boolean, error: ApiError, cards: Card[])
function renderComponent({ isLoading, error, cards }: CardsContextState)

Alternative 1

Since now we are using TypeScript we can try to take advantage of the type system.

With this approach, we will use the context state interfaces and pass the objects to the renderComponent function.

For this case, we can also create some factories to simplify the data creation.

import SaltedCaramelIceCream, { Props } from './SaltedCaramelIceCream';

...

describe(...
it(..., () => {
renderComponent(props, { isLoading: false, error: null, cards: [] });

// -- or if we create a factory for this cases, we could use
renderComponent(props, makeCardsContextState(false, null, []));
// --
...
})
)
...

function renderComponent(props: Props, cardsContextValue: CardsContextState) {
const providersOptions = MockProvidersOptions();
providersOptions.withContext(CardsContext, cardsContextValue);

return renderWithProviders(<SaltedCaramelIceCream {...props} />, providersOptions)
}

Motivation

  • Like we did in components, defining some rules and best practices for testing will increase the readability and compliance throughout our codebase.
  • If all of us follow the same rules it will lower the effort of understanding what is being tested and increase (in the future) our speed when facing new bugs.
  • If we define rules of how to organize our test files, structural changes in the future will be easier to address.

Proposed transition strategy

Since this only applies to components tests, there will only be one library to migrate (libs/components).

In that case, we should start by transition that library as proof of concept to this strategy and to take some notes about the process.

After that, all our apps will need to be migrated. For that step, we can split the apps by the team members as we have done in previous migrations.