test: 어학점수 관련 통합테스트 코드 추가 #165
Conversation
| // when | ||
| Long scoreId = scoreService.submitGpaScore(testUser.getEmail(), request); |
There was a problem hiding this comment.
이 부분은 Long이 아니라 long 을 쓰는게 좋겠습니다 🤔
Long 과 long 의 차이에 대해서는 크게 2가지가 있어요
- 할당되는 메모리 크기
- null 가능 여부
그런데 이런 지식적인 것에 나아가 제가 Long / long 을 엄격하게 구분하려고 하는 이유가 더 있는데요.
Long이 사용되면, 코드를 읽는 개발자는 '이 변수는 null 될 수도 있는거구나' 하고 생각하게 됩니다.
이런 오해를 없애기 위해서 null 되지 않을 곳에는 long을 써서 확실히 구분하면 좋겠습니다.
(사실 이 서비스코드의 리턴 타입도 long이 되어야 한다고 생각합니다.
생성에서 실패했다면 repository.save() 에서 예외가 발생할 것이니 id 를 null 로 반환할 이유는 더더욱 없을 것이라서요,
이렇게 long으로 대체해도 되는 Long이 저희 코드 곳곳에서 보여서 참.. 마음이 아픈데요..
점진적으로 잘 수정해나가봅시다!)
There was a problem hiding this comment.
Long이 사용되면, 코드를 읽는 개발자는 '이 변수는 null 될 수도 있는거구나' 하고 생각하게 됩니다.
이부분이 공감이 되네요! 좋은 거 같습니다! 반영했습니다. 😀
| assertAll( | ||
| () -> assertThat(savedScore.getId()).isEqualTo(scoreId), | ||
| () -> assertThat(savedScore.getGpa().getGpa()).isEqualTo(request.gpa()), | ||
| () -> assertThat(savedScore.getGpa().getGpaCriteria()).isEqualTo(request.gpaCriteria()), | ||
| () -> assertThat(savedScore.getIssueDate()).isEqualTo(request.issueDate()) | ||
| ); | ||
| } |
There was a problem hiding this comment.
이 assertAll 에도 PENDING 검증이 있으면 어떨까요?
() -> assertThat(savedScore.getVerifyStatus()).isEqualTo(VerifyStatus.PENDING)
| LanguageTestScoreRequest request = createLanguageTestScoreRequest(); | ||
|
|
||
| // when | ||
| Long scoreId = scoreService.submitLanguageTestScore(testUser.getEmail(), request); |
| 3.5, | ||
| 4.5, | ||
| LocalDate.now(), | ||
| "https://example.com/gpa-report.pdf" |
There was a problem hiding this comment.
해당 부분에서 전체 url이 아닌 resource까지의 path만이 저장되고 전달되는 것으로 알고 있습니다. 실제 구현은 /gpa-report.pdf 만 전달된다는 이야기인데요, 큰 문제가 있는 것은 아니나 실제와 동일하게 테스트를 설정해 두는 게 좋을 것 같습니다.
There was a problem hiding this comment.
좋은 거 같습니다! 반영했습니다 😀
관련 이슈
작업 내용
어학점수 관련 통합 테스트 코드 추가하였습니다.
특이 사항
리뷰 요구사항 (선택)