Skip to content

Feat/3 implement Reservation #24

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

Closed
wants to merge 16 commits into from
Closed

Feat/3 implement Reservation #24

wants to merge 16 commits into from

Conversation

yunsuu
Copy link
Collaborator

@yunsuu yunsuu commented Jul 10, 2023

What is this PR?

Key Changes

  • nothing now

Test Checklist

@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Patch coverage has no change and project coverage change: -50.00 ⚠️

Comparison is base (5a62890) 50.00% compared to head (85ca06c) 0.00%.

❗ Current head 85ca06c differs from pull request most recent head 4c034b3. Consider uploading reports for the commit 4c034b3 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #24       +/-   ##
============================================
- Coverage     50.00%   0.00%   -50.00%     
============================================
  Files             2       4        +2     
  Lines             8      22       +14     
============================================
- Hits              4       0        -4     
- Misses            4      22       +18     
Impacted Files Coverage Δ
...cketingservice/controller/ReservationController.kt 0.00% <0.00%> (ø)
.../com/group4/ticketingservice/entity/Reservation.kt 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@junha-ahn
Copy link
Member

junha-ahn commented Jul 10, 2023

All checks have failed

해당 commit이 git actions 체크에서 실패했네요

Todo

  • commit naming rule에 맞게 커밋 메세지 변경 (feat: reservation and jpa init 등)
  • ./gradlew ktlintFormat 명령 실행
  • TicketingserviceApplicationTests > contextLoads() FAILED 해당 테스트도 성공할 수 있게 변경해주세요

커밋 이름 변경

기존 커밋 이름 변경 후 git push -f

  • 해당 project는 main branch를 향해 squash merge만 지원하고 있어서, 내용 수정 commit이 여러개여도 괜찮습니다(어차피 main branch에는 하나의 커밋으로만 합쳐집니다)

왜 이런 커밋이 올라갔는지 모르겠지만, 한번 gradlew build하시면 이러한 체크들이 인텔리제이에서도 세팅되어서, 앞으로 로컬환경에서 자동 체크 될겁니다 (아마도 #21 )

@junha-ahn junha-ahn added the question Further information is requested label Jul 10, 2023
@junha-ahn junha-ahn linked an issue Jul 10, 2023 that may be closed by this pull request
@junha-ahn junha-ahn changed the title reservation feature and JPA init without test code feat: Reservation Jul 10, 2023
@junha-ahn junha-ahn changed the title feat: Reservation feat: add Reservation Jul 10, 2023
@junha-ahn junha-ahn changed the title feat: add Reservation feat: implement Reservation Jul 10, 2023
@junha-ahn junha-ahn requested review from bohblue2 and hihahayoung and removed request for bohblue2 and hihahayoung July 10, 2023 12:55
@junha-ahn
Copy link
Member

개발팀 첫 🥇 기능 구현 PR 축하드립니다 👍 👍 👍

매우 좋습니다 :)

@junha-ahn
Copy link
Member

junha-ahn commented Jul 10, 2023

향후 git issue로 만들 것 (개선사항)

  • docker-compose - mysql - 을 사용해서 로컬 개발 환경 통일화 (해당 PR에 대한 코드 리뷰 아닙니다)
    • 해당 PR은 개발자가 직접 local에 실행한 mysql에 연결한 상태로 approve
  • controller - service - repository 패턴 적용과 함께 Test 구현

@junha-ahn junha-ahn self-requested a review July 12, 2023 10:42
Copy link
Member

@junha-ahn junha-ahn left a comment

Choose a reason for hiding this comment

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

check plz

@junha-ahn junha-ahn removed the question Further information is requested label Jul 12, 2023
@junha-ahn junha-ahn changed the title feat: implement Reservation Feat/3 implement Reservation Jul 12, 2023
@junha-ahn
Copy link
Member

@bohblue2 코드 리뷰 부탁드립니다 (프로젝트 구조 등)

@junha-ahn
Copy link
Member

@yunsuu 테스트 시나리오 작성 및 테스트 코드 작성 부탁드립니다.

참고

@junha-ahn junha-ahn self-requested a review July 16, 2023 13:01
Copy link
Collaborator

@bohblue2 bohblue2 left a comment

Choose a reason for hiding this comment

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

작은 수정사항이 있습니다. 수정해주세요.

}

@PostMapping("/reservation")
fun reservePerformance(@RequestBody reservation: Reservation): Reservation {
Copy link
Member

Choose a reason for hiding this comment

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

향후에는 Reservation DTO를 선언해서 사용해주세요

  • 다만 현재 PR에서는, 돌아가는데 문제가 없기 때문에 수정하지 않아도 됩니다. (읽고 resolve 눌러주세요)

데이터 전송 객체(DTO)란 프로세스 사이에서 데이터를 전송하는 객체를 말합니다. DTO 기법을 사용하면 중요한 정보를 노출시키지 않고 두 시스템간.. - from

@@ -1 +1,8 @@

#spring.datasource.url=jdbc:mysql://localhost:3306/test
Copy link
Member

Choose a reason for hiding this comment

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

주석 삭제 부탁드립니다

fun read(){
val reservation = Reservation(1)
val savedReservation = reservationRepository.save(reservation)
if(savedReservation != null){
Copy link
Member

Choose a reason for hiding this comment

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

테스트 코드에는 조건문이 없어야 합니다!

val savedReservation = reservationRepository.save(reservation)
val foundReservation = reservationRepository.findByUserId(1)

Assertions.assertNotNull(foundReservation)
Asserts.assertEquals(savedReservation.id, foundReservation.id)

으로 수정하는게 좋을 것 같습니다.

또한 코틀린 문법에 따르면 object?.key 도 사용가능할듯합니다

Asserts.assertEquals(savedReservation.id, foundReservation?.id)

Copy link
Member

Choose a reason for hiding this comment

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

1. 레이어별 테스트

해당 테스트는 Repository - Database간의 상호작용을 테스트 하였습니다.

Controller의 테스트가 필요해보입니다. 관련개념글

컨트롤러 테스트 코드 Example

2. 테스트 환경 격리

해당 테스트 Local 환경에 실제로 구동중인 MySQL에 의존하고 있습니다.
따라서 Git actions를 통해 test 할때는 동일한 환경이 아니기 때문에 실패하게 됩니다. (MySQL이 준비되어 있지 않기 때문에)

격리된 테스트 (Isolated Test) 데이터베이스와 같은 공유 자원을 사용하는 테스트는 실행 순서에 따라 성공, 실패 여부가 결정되는 비결정적인(non-deterministic) 테스트가 될 수 있다. 이런 비결정적 테스트는 실패하면 버그가 원인인지, 비결정적 동작이 원인인지 알기 힘들다. 따라서 테스트는 실행 순서에 상관 없이 독립적으로, 결정적(deterministic)으로 실행되어야한다. 테스트 격리는 공유 자원을 사용하는 여러 테스트끼리 격리하여 서로 영향을 주고 받지 못하게 하는 기법이다. 특히 데이터베이스를 사용하는 객체를 테스트할 때 테스트 격리가 필요하다. - from

해결 방법은 몇가지가 있습니다.

  1. in-memory Database H2를 사용하여 테스트
  2. testcontainer 환경에서 (현재 main branch에 준비되어 있습니다) 해당 코드를 테스트 하기
  3. 해당 Database 테스트는 본질적으로 Unit Test가 아니기에 해당 파일 삭제하기

먼저 1번의 경우 좋은 방법이 아닙니다. 왜냐하면 h2는 mysql 을 mock up(가짜화)하는데 성공해 유닛 테스트를 구현할 수 있게는 해주지만, 본질적으로 mysql이 아니기 때문에 실제와 같은 환경을 제공해주지는 않기 때문입니다.
따라서, 2번이 더 좋은 선택이라고 볼 수 있습니다 - 관련 글

@junha-ahn
Copy link
Member

통합 테스트 관련 오류 이슈 #48

@junha-ahn junha-ahn closed this Aug 8, 2023
@junha-ahn junha-ahn deleted the feat/3-reservation branch August 8, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

예매 기능 구현
3 participants