international-labour-organization / designsystem

A design system for the International Labour Organization
Apache License 2.0
1 stars 3 forks source link

React: add minimal test coverage for all components #934

Open justintemps opened 2 months ago

justintemps commented 2 months ago

Implement minimal test coverage for all components in the React component library to ensure basic functionality and prevent regressions.

Our React component library is currently lacking in test coverage. We need to establish a baseline of minimal test coverage for all the components in the library.

Objectives

Acceptance Criteria

bashlk commented 2 months ago

@justintemps Before we start on this, I think it will be good to measure the test coverage so that we have a good idea of where we stand now. We can then clearly see which components are currently not being tested and add a basic "render check" to the components that are not currently tested to see if it renders with the expected elements. Later we can also use the test coverage report to find the components that are under tested and improve them.

Since the react package uses CRA to run jest, we should be able to get the coverage report by adding a single option to the test command. Ideally we should regularly measure and report the test coverage using the CI but we can also run it manually at certain intervals to track our progress.

bashlk commented 2 months ago

Also we should probably move away from using CRA for the tests. It's really strange that it is being used for the tests alone. Do you know why the react package ended up using rollup for the builds and CRA for the tests?

Shashika6 commented 2 months ago

Did an initial investigation on react test for components. The plan will be to releas tests individually for each component as there is a quick release flow and we also have an advantage to prioritise components. (Like what we did in RTL we can have a label for this)

Components that require tests to be written.

Components with tests already written.

justintemps commented 2 months ago

Also we should probably move away from using CRA for the tests. It's really strange that it is being used for the tests alone. Do you know why the react package ended up using rollup for the builds and CRA for the tests?

@bashlk I'm not sure why we're using CRA for the tests, but I'm not married to it. If there's a better way of rendering React components for testing, I'm all for it.

justintemps commented 2 months ago

Hey @Shashika6 this looks good to me.

bashlk commented 2 months ago

Since we are looking into this, it might be better to switch the React project to using Vite and Vitest so that we don't have to refactor even more code. It seems like Storybook is already set up to use Vite.

What do you think @justintemps?

justintemps commented 1 month ago

This is currently blocked by #985