Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
4 of 63 tasks
guidari opened this issue May 3, 2024 · 6 comments
Open
4 of 63 tasks

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

guidari opened this issue May 3, 2024 · 6 comments
Labels
package: @carbon/react @carbon/react planning: umbrella Umbrella issues, surfaced in Projects views role: dev 🤖 type: infrastructure 🤖 Issues relating to devops, tech debt, etc.
Milestone

Comments

@guidari
Copy link
Contributor

guidari commented May 3, 2024

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

  1. type: infrastructure 🤖
    Gururajj77 guidari
    preetibansalui
  2. type: infrastructure 🤖
    Gururajj77 guidari
    preetibansalui

High priority components under 80% coverage

  1. tay1orjones
  2. 2nikhiltom Gururajj77
    annawen1 ariellalgilmore emyarod guidari preetibansalui riddhybansal tay1orjones
  3. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  4. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  5. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  6. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  7. ariellalgilmore
  8. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  9. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  10. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  11. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  12. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  13. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  14. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  15. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  16. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  17. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  18. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  19. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  20. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  21. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  22. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  23. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  24. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  25. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  26. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  27. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  28. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  29. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  30. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  31. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  32. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  33. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  34. ariellalgilmore
  35. ariellalgilmore
  36. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  37. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  38. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  39. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  40. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
    guidari
  41. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
    guidari
  42. guidari
  43. guidari
  44. ariellalgilmore
  45. ariellalgilmore
  46. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  47. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  48. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  49. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  50. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  51. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  52. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  53. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  54. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  55. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  56. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
    guidari
  57. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
    guidari
  58. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11
  59. afrohacks good first issue 👋 hacktoberfest package: @carbon/react role: dev 🤖 status: help wanted 👐 type: infrastructure 🤖 version: 11

Flaky tests to fix

@tay1orjones
Copy link
Member

tay1orjones commented May 8, 2024

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

@guidari guidari added the planning: umbrella Umbrella issues, surfaced in Projects views label May 21, 2024
@guidari guidari changed the title Draft Issue Test coverage for @carbon/react components May 21, 2024
@guidari guidari added this to the 2024 Q3 milestone May 23, 2024
@guidari
Copy link
Contributor Author

guidari commented Jun 4, 2024

Related issue: Research and implement a testing strategy for types provided through @carbon/react
#16361

@tay1orjones
Copy link
Member

tay1orjones commented Jun 20, 2024

... 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

@sstrubberg sstrubberg changed the title Test coverage for @carbon/react components Unit Test coverage for @carbon/react components Jun 25, 2024
@guidari guidari changed the title Unit Test coverage for @carbon/react components Unit Test coverage for @carbon/react components ☂️ Jun 26, 2024
@tay1orjones
Copy link
Member

Additionally we should look at any playwright test that is

  • evaluating as "slow" locally
  • using .locator() directly, as this can produce flakiness

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
Copy link
Contributor

elycheea commented Jul 2, 2024

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

@tay1orjones tay1orjones changed the title Unit Test coverage for @carbon/react components ☂️ Unit Test coverage for @carbon/react components ☂️ Aug 8, 2024
@tay1orjones tay1orjones added the package: @carbon/react @carbon/react label Aug 8, 2024
@tay1orjones tay1orjones added the type: infrastructure 🤖 Issues relating to devops, tech debt, etc. label Sep 4, 2024
@sstrubberg sstrubberg modified the milestones: 2024 Q3, 2024 Q4 Sep 12, 2024
@tay1orjones
Copy link
Member

tay1orjones commented Sep 12, 2024

Kickoff notes

Collecting coverage

  • Entire repo - yarn test --collectCoverage from the root
  • Collect coverage for a specific component (will update the html report)
    • yarn test packages/react/src/components/Tooltip/__tests__/DefinitionTooltip-test.js --collectCoverage
  • Get coverage for a specific component in your terminal
    • yarn test packages/react/src/components/Tooltip/__tests__/DefinitionTooltip-test.js --coverage --collectCoverageFrom=packages/react/src/components/Tooltip/DefinitionTooltip.tsx

Reading the coverage report

  • The report is placed in /coverage - open in finder and navigate to the .html file and open it
  • Red = lines not covered
  • Yellow - conditional branch not covered
  • Green left margin e.g. 85x is how many times the line was hit across all the tests. This number can largely be ignored

Other notes

  • Default of 3 for effort on these issues

Tips for test writing

  • 100% coverage is the goal, but despite the number the intention is for every prop in the component api to be covered by a logical test that ensures the prop functionality behaves as expected.
  • ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: @carbon/react @carbon/react planning: umbrella Umbrella issues, surfaced in Projects views role: dev 🤖 type: infrastructure 🤖 Issues relating to devops, tech debt, etc.
Projects
Status: 🏗 In Progress
Status: Now 💫
Development

No branches or pull requests

4 participants