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

MSW 모킹 라이브러리 도입 및 Jest 테스트 라이브러리 Vitest로 변경 작업 #137

Merged
merged 11 commits into from Jan 8, 2024

Conversation

davidktlee
Copy link
Collaborator

Motivation 🧐

  • 백엔드와의 동시 작업을 위한 MSW 라이브러리를 도입했습니다.
  • MSW를 도입하며 호환성 문제가 발생한 Jest 테스트 라이브러리 대신 Vitest테스트 라이브러리를 도입했습니다.

Key Changes 🔑

  • MSW 관련 파일 생성 및 vite.config.js 환경설정 완료.
  • Jest 테스트 시 사용하던 svgr에 대한 __mocks 파일 삭제 및 vite.config.js에 설정 완료.
  • 캐러셀에서 랜덤 이미지 테스트를 위한 설정으로 next.config.js설정 변경.

To Reviewers 🙏

  • /mocks/server.ts 파일은 Next App Router 문제express의 모킹 인터셉트를 위한 파일입니다. handler 파일은 동일하게 작성해도 되며, 사용 시 새로운 터미널에서 npm run mock 명령어로 서버를 하나 생성 후 작동시키면 됩니다.
  • MSWJest의 호환성 이슈로 MSW (Mock Service Worker) 도입 #130 의 리뷰에서 정욱님과 소통하였으며 Vitest로의 변경을 합의하였습니다.
  • 자세한 MSW 도입과 에러에 대한 부분은 프로젝트의 wiki 페이지에 작성 해 놓도록 하겠습니다.

Copy link

cr-gpt bot commented Jan 5, 2024

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

Copy link
Collaborator

@hatchling13 hatchling13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR 목적은 MSW 도입 및 Vitest로의 변경인데 여러 컴포넌트들이 함께 추가되어있는 모습입니다. 서로 다른 부분을 작업하고 있고, 작업자가 많지 않은 상황이기에 크게 문제는 없습니다만 이후부터는 PR 내용에 맞지 않는 커밋들은 별도의 PR로 분리할 필요가 있다고 생각합니다. 라이브러리 작업을 별도의 브랜치에서 진행해주셨으면 어땠을까 생각되네용~

그 외 각각의 개별적인 부분에 대한 리뷰는 확인해주시면 되겠습니다.

frontend/vite.config.js Outdated Show resolved Hide resolved
frontend/src/mocks/handlers.ts Show resolved Hide resolved
frontend/src/app/layout.tsx Outdated Show resolved Hide resolved
Copy link

cr-gpt bot commented Jan 5, 2024

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

@davidktlee
Copy link
Collaborator Author

davidktlee commented Jan 5, 2024

MSW 컴포넌트에서의 조건문 if (process.env.NODE_ENV === 'production') 이 되어야 하는데 반대로 되어 있어서 수정했습니다.
라이브러리 작업을 별도로 작업 하긴 했는데 pr을 합쳐서 올린 부분은 저도 수정의 필요성을 느꼈습니다.
다음 pr때는 확실한 관심사 분리를 해 올리겠습니다.

아 그리고 구글 로그인 부분에 로고가 안 뜨던데 그 부분 확인 부탁드리겠습니다!
카카오 로고는 카카오 디자인측에서 따로 제공되는 svg 파일이 없어 png 파일로 처리하였습니다.

@hatchling13
Copy link
Collaborator

SVGR 세팅을 해놨는데 코드가 Next의 Image를 쓰는 코드에서 안 바뀌어있더라구요. import할때 앞 글자 대문자로 해줘서 그대로 넣으면 다시 보이긴 합니다. PR 머지된 후 고쳐놓겠습니다.

Copy link
Member

@stoneHee99 stoneHee99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@hatchling13 hatchling13 merged commit 0b5158f into Kernel360:develop Jan 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants