carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.88k stars 1.82k forks source link

Unit Test coverage for `@carbon/react` components ☂️ #16319

Open guidari opened 7 months ago

guidari commented 7 months ago

The goal of this workstream is to increase confidence in each component, not just to increase coverage. It’s better to have 80% coverage with high-quality tests than 100% coverage with low-quality tests.

If you want to tackle any research, just create an issue to keep your notes from the research. Would be nice to look for repos that are using that on their PRs, to take as an example.

[!IMPORTANT]
Read the kickoff notes below for implementation instructions and tips

### Add code coverage CI check
- [ ] https://github.com/carbon-design-system/carbon/issues/17123
- [ ] https://github.com/carbon-design-system/carbon/issues/17124
- [ ] https://github.com/carbon-design-system/carbon/issues/17985
### High priority components under 80% coverage
- [ ] https://github.com/carbon-design-system/carbon/issues/17414
- [ ] https://github.com/carbon-design-system/carbon/issues/17415
- [ ] https://github.com/carbon-design-system/carbon/issues/17470
- [ ] https://github.com/carbon-design-system/carbon/issues/17471
- [ ] https://github.com/carbon-design-system/carbon/issues/17472
- [ ] https://github.com/carbon-design-system/carbon/issues/17473
- [ ] https://github.com/carbon-design-system/carbon/issues/17429
- [ ] https://github.com/carbon-design-system/carbon/issues/17474
- [ ] https://github.com/carbon-design-system/carbon/issues/17475
- [ ] https://github.com/carbon-design-system/carbon/issues/17476
- [ ] https://github.com/carbon-design-system/carbon/issues/17477
- [ ] https://github.com/carbon-design-system/carbon/issues/17478
- [ ] https://github.com/carbon-design-system/carbon/issues/17479
- [ ] https://github.com/carbon-design-system/carbon/issues/17480
- [ ] https://github.com/carbon-design-system/carbon/issues/17481
- [ ] https://github.com/carbon-design-system/carbon/issues/17482
- [ ] https://github.com/carbon-design-system/carbon/issues/17483
- [ ] https://github.com/carbon-design-system/carbon/issues/17484
- [ ] https://github.com/carbon-design-system/carbon/issues/17485
- [ ] https://github.com/carbon-design-system/carbon/issues/17486
- [ ] https://github.com/carbon-design-system/carbon/issues/17487
- [ ] https://github.com/carbon-design-system/carbon/issues/17488
- [ ] https://github.com/carbon-design-system/carbon/issues/17489
- [ ] https://github.com/carbon-design-system/carbon/issues/17490
- [ ] https://github.com/carbon-design-system/carbon/issues/17491
- [ ] https://github.com/carbon-design-system/carbon/issues/17492
- [ ] https://github.com/carbon-design-system/carbon/issues/17493
- [ ] https://github.com/carbon-design-system/carbon/issues/17494
- [ ] https://github.com/carbon-design-system/carbon/issues/17495
- [ ] https://github.com/carbon-design-system/carbon/issues/17496
- [ ] https://github.com/carbon-design-system/carbon/issues/17497
- [ ] https://github.com/carbon-design-system/carbon/issues/17498
- [ ] https://github.com/carbon-design-system/carbon/issues/17499
- [ ] https://github.com/carbon-design-system/carbon/issues/17441
- [ ] https://github.com/carbon-design-system/carbon/issues/17442
- [ ] https://github.com/carbon-design-system/carbon/issues/17500
- [ ] https://github.com/carbon-design-system/carbon/issues/17501
- [ ] https://github.com/carbon-design-system/carbon/issues/17502
- [ ] https://github.com/carbon-design-system/carbon/issues/17503
- [ ] https://github.com/carbon-design-system/carbon/issues/17504
- [ ] https://github.com/carbon-design-system/carbon/issues/17505
- [ ] https://github.com/carbon-design-system/carbon/issues/17437
- [ ] https://github.com/carbon-design-system/carbon/issues/17438
- [ ] https://github.com/carbon-design-system/carbon/issues/17432
- [ ] https://github.com/carbon-design-system/carbon/issues/17431
- [ ] https://github.com/carbon-design-system/carbon/issues/17506
- [ ] https://github.com/carbon-design-system/carbon/issues/17507
- [ ] https://github.com/carbon-design-system/carbon/issues/17508
- [ ] https://github.com/carbon-design-system/carbon/issues/17509
- [ ] https://github.com/carbon-design-system/carbon/issues/17510
- [ ] https://github.com/carbon-design-system/carbon/issues/17511
- [ ] https://github.com/carbon-design-system/carbon/issues/17512
- [ ] https://github.com/carbon-design-system/carbon/issues/17513
- [ ] https://github.com/carbon-design-system/carbon/issues/17514
- [ ] https://github.com/carbon-design-system/carbon/issues/17515
- [ ] https://github.com/carbon-design-system/carbon/issues/17516
- [ ] https://github.com/carbon-design-system/carbon/issues/17517
- [ ] https://github.com/carbon-design-system/carbon/issues/17518
- [ ] https://github.com/carbon-design-system/carbon/issues/17519
### Flaky tests to fix
- [ ] `Menu-test.avt.e2e.js:25:3 › @avt Menu › @avt-keyboard-nav Menu`
- [ ] `Slider › behaves as expected - Two Handle Slider Component API › Error handling, expected behavior from event handlers › gracefully tolerates empty event passed to _onDrag`
tay1orjones commented 6 months ago

I'd love to see this include adding something like a codecov ci check on our PRs.

guidari commented 6 months ago

Related issue: Research and implement a testing strategy for types provided through @carbon/react https://github.com/carbon-design-system/carbon/issues/16361

tay1orjones commented 5 months ago

... increase confidence in each component, not just to increase coverage

One example of this that I just ran into that we should consider:

Tests covering callback props should always assert both how many times it's called and what it's called with

expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith(arg1, arg2, ...);

Numerous times we've fixed bugs relating to callbacks being called multiple times or not at all. We've also had plenty of times where the contents of the callback args change unintentionally. Let's make sure we always test both.

There's probably not a way to automatically enforce this, but it's a style/practice we should adopt everywhere imo

tay1orjones commented 5 months ago

Additionally we should look at any playwright test that is

We recently found a case were the tile avt tests were actually failing locally, but passing in CI. It seems the reason was that the tests were timing out and not producing an error in ci. Locally they would fail. We should evaluate to ensure all the tests we have pass individually locally.

elycheea commented 5 months ago

@tay1orjones The timing of your comment was impeccable. @ariellalgilmore and I were investigating some flakiness on our end as you wrote this. 😹

tay1orjones commented 2 months ago

Kickoff notes

Collecting coverage

Reading the coverage report

Other notes

Tips for test writing

ariellalgilmore commented 2 months ago

adding https://codewithhugo.com/jest-exclude-coverage/ as a resource. In Jest 100% coverage might not be possible because it's not a "real" browser setting. For example, when testing for overflow the tests will never pick it up. The linked guide explains how to ignore or exclude files when necessary