Conversation
✅ Deploy Preview for we-write ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| }; | ||
| it('모든 장르 버튼이 렌더링됨', () => { | ||
| setup(); | ||
| ['전체', '판타지', '로맨스', '스릴러/미스터리', '무협'].forEach((genre) => { |
There was a problem hiding this comment.
확장을 고려해서 genres 상수를 따로 만들어서 import해오는 방법도 있을 것 같습니다.
최근에 비슷한 문제를 봐서 생각이나서 말씀드렸습니다~!!
| } | ||
|
|
||
| jest.mock('@/hooks/api/library/useInfiniteStories', () => ({ | ||
| useInfiniteStories: jest.fn(), |
There was a problem hiding this comment.
여기서는 __esModule 옵션이 없는게 export default로 설정돼서 그런건가요?
There was a problem hiding this comment.
저도 정확히 몰라서 gpt한테 물어보니, const로 export 하는 경우에는 __esModules가 필요없다고 하네요 ㅎㅎ
| const input = screen.getByLabelText('스토리 제목으로 검색'); | ||
|
|
||
| fireEvent.change(input, { target: { value: '천마재림' } }); | ||
| // typescript Non-null 처리 |
There was a problem hiding this comment.
불필요한 주석이면 삭제하는게 좋을 것 같습니다.
근데 어떤 부분이 Non-null처리를 하는 건가요? 궁금하네요
There was a problem hiding this comment.
TypeScript에서는 closest('form')가 null을 반환할 수도 있다고 경고한다고 합니다. 그래서 closest('form')뒤에 "!"를 붙여 Non-null처리를 한것입니다!
주석은 지워도 될거 같아서 지워놓겠습니다~!
SwimmingRiver
left a comment
SwimmingRiver
left a comment
There was a problem hiding this comment.
고생하셨습니다~~
저는 아직도 테스트가 어려운 거 같은데 잘 하신거 같아요!
궁금한거 몇개랑 참고사항 코멘트 남겼습니다.
juyesu
left a comment
juyesu
left a comment
There was a problem hiding this comment.
처음 작성해주신 테스트 코드 PR이시군요! 고생하셧습니다!
리뷰 남겨드린 부분 한번씩 체크 부탁드릴게요!!
| import { useInfiniteStories } from '@/hooks/api/library/useInfiniteStories'; | ||
| import useCurrentViewPort from '@/hooks/useCurrentViewPort'; |
There was a problem hiding this comment.
파일 중간에 import 문을 선언해주신 이유가 있을까요?
일반적으로 ES Module 문법에서 import 를 작성하는 것과 차이가 있다면 어떤 부분이었을 지 궁금합니다
There was a problem hiding this comment.
jest.mock()이 적용된 모듈의 mock 버전을 가져오기 위해 import를 중간에 배치한다고 합니다. 일반 ESM은 파일 최상단 import만 허용하지만, Jest는 mock 적용을 위해 변환 시 순서를 재조정하기 떄문에 위 방법이 가능하다고 합니다!
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| mockUseCurrentViewPort.mockReturnValue({ viewportWidth: 1200 }); |
There was a problem hiding this comment.
viewportWidth: 1200 는 Tailwindcss에서 반응형 breakpoint로 설정한 값일까요?
만약 그렇다면 tailwind에서는 1024, 1280, 1536px 단위로 레이아웃이 변경되는데 1200이 의도한 값이 맞을까요?
코드를 읽을 때 매직 넘버의 의미를 파악할 수 있도록 상수화해주시면 좋을 것 같습니다!
There was a problem hiding this comment.
VIEWPORT_BREAK_POINT.LG 와 VIEWPORT_BREAK_POINT.XL 사이 값인데
1200 보다는 아래 코드가 의도 파악이 보일거 같아
beforeEach(() => {
mockUseCurrentViewPort.mockReturnValue({
viewportWidth: (VIEWPORT_BREAK_POINT.LG + VIEWPORT_BREAK_POINT.XL) / 2,
});
});
이렇게 수정해놓도록 하겠습니다.
| }); | ||
|
|
||
| render( | ||
| <LibraryListContainer {...defaultProps} searchType={'제목' as const} /> |
There was a problem hiding this comment.
LibraryListContainer 컴포넌트를 호출할 때
searchType props로 전달하는 값이 searchType={'제목' as const}과 searchType="제목" 두 가지 방법으로 혼재되어 있는 것 같습니다.
일부러 렌더 시에 props를 다르게 전달한 이유가 있었을까요?
There was a problem hiding this comment.
이부분은 통일하는게 나을거 같아 searchType="제목"로 수정해놓겠습니다.
| it('선택된 장르는 active 스타일이 적용됨', () => { | ||
| setup(['판타지']); | ||
| const fantasyBadge = screen.getByRole('button', { name: '판타지' }); | ||
| expect(fantasyBadge).toHaveClass('bg-black', 'text-white'); |
There was a problem hiding this comment.
toHaveClass('bg-black', 'text-white')와 같이 tailwind 클래스를 바탕으로 요소를 찾게 되면
테스트가 스타일(코드)에 종속적이게 되는 문제가 발생할 수 있을 것 같아요
예를 들어 스타일 변경 사항이 발생하면 테스트 코드도 함께 수정을 진행해야 해서 유지보수성이 떨어질 수 있을 것 같습니다.
There was a problem hiding this comment.
위 문제사항에 대해 인지해서 아래와 같이 수정해보았습니다.
// GenreBadge.tsx
<button
type="button"
aria-pressed={selectedGenres.includes(genre)}
>
{genre}
</button>
// GenreBadge.test.tsx
it('선택된 장르는 aria-pressed=true', () => {
setup(['판타지']);
const fantasyBadge = screen.getByRole('button', { name: '판타지' });
expect(fantasyBadge).toHaveAttribute('aria-pressed', 'true');
});
3 participants
변경 사항
library 페이지에서 사용하는 컴포넌트에 대한 테스트 코드를 추가하였습니다.
테스트를 진행한 컴포넌트 다음과 같습니다.
사용자 관점에서 발생할 수 있는 이벤트를 위주로 테스트 코드를 작성하였으며, 필요한 데이터와 함수는 Mock으로 대체하여 테스트를 진행하였습니다.