michael-s-molina commented on pull request #13422: URL: https://github.com/apache/superset/pull/13422#issuecomment-789667575
Hey @yardz! Nice refactoring! About the tests: Try to access the elements by `role` instead of `test-id` which is not accessible to the users. You can use one of the [predefined roles](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Table_Role) `rowgroup`, `row`, `columnheader`, and `cell`. With testing library, you're encouraged to test the user's interaction with the screen. Relying on the React component data like state, props or attributes leads to brittle tests that depend on implementation detail and don't provide as much value to users. You can review some of your expects of type `toHaveAttribute`. Think about the scenario where we change the implementation of the component (remove attributes or classes) but keep the same functionality to the user. Your tests should not fail after the change. Instead of test for attributes: ``` render(<TableCollection {...} />); expect(screen.getByTestId('loading')).toHaveAttribute( 'data-loading', 'loading', ); ``` Try to change the focus more to how the **user** will perceive the different component states. ``` render(<TableCollection loading {...} />); const status = screen.getByRole('status'); expect(status).toBeInTheDocument(); expect(status).toHaveTextContent('Loading'); ``` You're are finding the tests very weak because this component is a simple layer on top of `react-table` without too many additional behaviors. The majority of tests have already been made by the `react-table` team. This is common in this kind of component. You will find more complex test scenarios in components that have many custom behaviors or components created from scratch. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
