[new] - Create app components, home page and add unit testing #1
Conversation
coverage.txt
Outdated
| ----------------------|----------|----------|----------|----------|-------------------| | ||
| File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s | | ||
| ----------------------|----------|----------|----------|----------|-------------------| | ||
| All files | 92.59 | 100 | 77.78 | 92.59 | | |
| }); | ||
|
|
||
| it('renders all children', () => { | ||
| const layout = screen.getByTestId('layout'); |
There was a problem hiding this comment.
Since the Layout has a styled component rendering a <main> HTML element, you should use getByRole('main') instead and leave test-id attributes for other elements difficult to reach
src/components/Layout/index.jsx
Outdated
|
|
||
| function Layout({ children }) { | ||
| return ( | ||
| <Styled.Container className="container" data-testid="layout"> |
There was a problem hiding this comment.
Use data-test-id for more complex components that are harder to reach. The Styled.Container would be rendered as a main HTML and role - check
| render(<NavBar />); | ||
| }); | ||
|
|
||
| it('renders NavBar and its elements', () => { |
There was a problem hiding this comment.
👌🏽 nice, it would be even better if you test user interaction for the navbar to open/close
src/components/SideMenu/index.jsx
Outdated
| @@ -0,0 +1,30 @@ | |||
| import React from 'react'; | |||
| import Styled from './styled'; | |||
There was a problem hiding this comment.
Use destructuring to clean your JSX code
| import Styled from './styled'; | |
| import { CloseBtn, CloseIcon, ContainerMenu, Links, LinkText, SideMenu } from './styled'; |
| }); | ||
|
|
||
| it('renders SideMenu and its elements', () => { | ||
| const sideMenu = screen.getByTestId('sideMenu'); |
There was a problem hiding this comment.
Same as other tests, look for the internal elements and their details. Make your tests meaningful for the readers. Your tests should explain the usage of your production code
| it('renders NavBar and its elements', () => { | ||
| const navBar = screen.getByTestId('navBar'); | ||
| expect(navBar.children.length).toEqual(1); | ||
| const navContent = screen.getByTestId('navContent'); |
There was a problem hiding this comment.
Same as above, leave the usage of data-test-id as the last resource. Instead, you could use getByRole() to assert you have those 4 elements at specific:
- A menu icon (maybe with a
data-test-id- depending on the final render of a Dehaze Material UI component) - A side menu
- An input, and
- A checkbox
| expect(screen.getByText(items[1].snippet.title)).toBeTruthy(); | ||
| expect(screen.getByText(items[1].snippet.description)).toBeTruthy(); |
There was a problem hiding this comment.
I agreed with the usage of data-test-id here but the expectation statements could be more descriptive using extra vars with the correct names like:
| expect(screen.getByText(items[1].snippet.title)).toBeTruthy(); | |
| expect(screen.getByText(items[1].snippet.description)).toBeTruthy(); | |
| const videoData = items[1].snippet; | |
| expect(screen.getByText(videoData.title)).toBeTruthy(); | |
| expect(screen.getByText(videoData.description)).toBeTruthy(); |
| const row = screen.getByTestId('row'); | ||
| expect(row.children.length).toEqual(24); |
There was a problem hiding this comment.
how about expecting the 24 elements are actually VideoCards? and that there's only one per Styled.Col
src/pages/Home/Home.page.jsx
Outdated
| <Link to="/login">let me in →</Link> | ||
| )} | ||
| <section> | ||
| <div> |
There was a problem hiding this comment.
I do not see the need for a div here since you are wrapping everything with a section
2 participants
No description provided.