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

Lbustamante/2 testing #2

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

LuisBustamanteM
Copy link
Owner

@LuisBustamanteM LuisBustamanteM commented Oct 11, 2021

2 - Intro to testing

Key Features:

  • Added snapshot testing on all components.
  • Added error handling to CardGrid component when failing to get data from a fetch.
  • Replaced Mock CardGrid data with a request & state handling with useState & useEffect hooks.
  • Added utils directory to store reusable functions.

Question answers:

  • Using create-react-app, what do we need to set up for testing?
    • create-react-app already comes with Jest and react testing library built in.
  • What components are worth to test in your development?
    • All of them, by the time our app grows we need to make sure everything is working as intended.
  • Can you apply TDD once you already created components?
    • Yes, but it's a better approach to start TDD from the initial development phase.

Code Coverage

File % Stmts % Branch % Funcs % Lines Uncovered Line #s
All files 89.47 42.86 77.78 89.47
src 66.67 100 100 66.67
globalStyle.js 100 100 100 100
index.js 0 100 100 0 7
src/components/App 100 100 100 100
App.component.jsx 100 100 100 100
index.js 0 0 0 0
src/components/BurgerMenu 100 100 100 100
BurgerMenu.component.js 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/components/CardGrid 70 66.67 40 70
CardGrid.component.jsx 62.5 66.67 40 62.5 13,14,21
index.js 0 0 0 0
style.js 100 100 100 100
src/components/Layout 100 100 100 100
Layout.component.jsx 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/components/LeftNav 100 100 100 100
LeftNav.component.jsx 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/components/Link 100 50 100 100
Link.react.js 100 50 100 100 23
src/components/Navbar 100 100 100 100
Navbar.component.jsx 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/components/Private 0 0 0 0
Private.component.jsx 0 0 0 0 7,9,10
index.js 0 0 0 0
src/components/RightNav 100 100 100 100
RightNav.component.js 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/components/SearchBar 87.5 100 50 87.5
SearchBar.component.js 75 100 50 75 16
index.js 0 0 0 0
style.js 100 100 100 100
src/components/Toggle 100 100 100 100
Toggle.component.js 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/components/UserInfo 100 100 100 100
UserInfo.component.js 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/components/VideoCard 100 100 100 100
VideoCard.component.js 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/pages/Home 100 100 100 100
Home.page.jsx 100 100 100 100
index.js 0 0 0 0
style.js 100 100 100 100
src/utils 100 100 100 100
fetchApi.js 100 100 100 100

Test Suites: 13 passed, 13 total
Tests: 15 passed, 15 total
Snapshots: 15 passed, 15 total
Time: 4.231s, estimated 5s
Ran all test suites matching /./i.

@cylcrow
Copy link

cylcrow commented Oct 21, 2021

Hey @LuisBustamanteM, I hope you're doing great.

I have a couple comments.
You did implement tests, but all of them are just snapshot tests.

Let's recall that snapshot testing is more useful when we want to make sure that static content is not changed once we change data or props we pass to them.
If we only use snapshot testing, we're not testing all functionality of our components, and when we try to test logic, its's likely that our snapshot tests will fail.

Let's rethink.
On which components can we remove snapshot testing and add a more meaningful way to test component's functionality?

  • Meaningful test cases were implemented for components and sub-routines logic.
  • All the test cases were successful. (I marked this OK, but there's on test failing for the API because I have not access to the real API when testing and we already know to to use nock to mock API calls)

Bonus Points

  • Test coverage is above 60%. (recall what I mentioned above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants