Conversation
emibgo2
reviewed
Jan 31, 2025
| }); | ||
|
|
||
| async findOne(id: number, customer: ICustomer): Promise<PetEntity> { | ||
| const pet = await this.petRepository.getOne(id); |
Contributor
There was a problem hiding this comment.
service에서는 findOne이고 repository에서는 getOne인데 두개 동일하게 맞춰주세요!
get~은 존재하지않으면 exception이고 find는 그렇지 않습니다.
| }); | ||
| } | ||
|
|
||
| findOneById(petId: number): Promise<PetEntity> { |
Contributor
There was a problem hiding this comment.
getOneById라는 이름이 더 적합할거 같아요!
| @CurrentCustomer() customer: CustomerEntity, | ||
| ): Promise<PetChecklistDto[]> { | ||
| return await this.petService.findCheckList(category, type, null, customer); | ||
| return await this.petService.findCheckList(category, type, null); |
Contributor
There was a problem hiding this comment.
이부분도 get과 find가 혼용되어서 사용되고 있습니다
| @ApiOkResponse({ type: PetDto, description: '반려동물 정보 조회 성공' }) | ||
| @Get('/my') | ||
| async getAll(@CurrentCustomer() customer: CustomerEntity): Promise<Pet[]> { | ||
| async getAll( |
| @@ -93,7 +106,7 @@ export class PetController { | |||
| async getOne( | |||
Contributor
There was a problem hiding this comment.
좀더 다양한 테스트 케이스를 작성해보는건 어떨까요??
예를들어 ChecklistType에따라서 petChecklistChoices나 petChecklistAnswer 둘중 null로 반환되는 경우 등이 있을것 같아요
emibgo2
reviewed
Feb 18, 2025
Contributor
emibgo2
left a comment
emibgo2
left a comment
There was a problem hiding this comment.
잘하신것 같아요!
마음은 Approve 이지만 컨플릭을 해결해주세요 :(
그리고 추가적으로 테스트 코드 작성할때 it 말고 test로 테스트 코드를 표현하는게 좋아보여요!
it은 영어로 작성할때는 코드가 더 잘 읽히지만 한글인 경우엔
test로 하는게 더 직관적인것 같아서 다른 test 코드들이 test 함수로 테스트 코드를 작성했습니다.
두가지 부분만 해결해주시고 알려 주시면 바로 approve 하겠습니다!
| expect(pet.customer.customerName).toBe(customer.customerName); | ||
| expect(pet.customer.authProvider).toBe(customer.authProvider); | ||
| }); | ||
| it('PetDto의 breedId가 존재하지 않을 때 BadRequestException을 발생시킨다.', async () => { |
| expect(pet.petName).toBe(createdPet.petName); | ||
| }); | ||
|
|
||
| it('특정 customer의 pet이 아닐 경우 ForbiddenException를 발생시킨다', async () => { |
| expect(checklist).toBeDefined(); | ||
| expect(Array.isArray(checklist)).toBe(true); | ||
| }); | ||
| it('체크리스트 타입이 Choice일 때 petChecklistAnswer는 null이다.', async () => { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠️ 코드 리팩토링
pet 관련 테스트 코드를 작성했습니다.
변경 사항
새로 작성한 Mock Repository 클래스 및 테스트 파일 외의 주요 수정 사항은 아래와 같습니다.
참고) https://stackoverflow.com/questions/53963007/error-while-running-nestjs-in-production-mode-cannot-find-module
테스트 방법
yarn test test/unit/pet/application/pet.service.test.ts
yarn test test/unit/pet/presentation/pet.controller.test.ts
yarn test test/unit/pet/pet.domain.test.ts
참고 사항
IntelliJ의 organize imports 기능을 적용했더니 거의 모든 파일이 커밋되어버려 (SHA: 264100e) revert로 되돌리기를 했습니다..! 확인 부탁드립니다 ㅜㅜ
jest.fn(), jest.spyOn() 등을 활용하는 게 좋을지 몰라서 우선 보류하고 Customer 파트 테스트코드 형식을 참고해서 작성했습니다.