-
Notifications
You must be signed in to change notification settings - Fork 1
[Test] 시간 기반 테스트 수정 #232
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
[Test] 시간 기반 테스트 수정 #232
Conversation
개요생성자에서 변경 사항
예상 코드 리뷰 소요 시간🎯 2 (단순) | ⏱️ ~10분
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @leegwichan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 CustomizeTableEntity의 usedAt 필드 처리 방식을 개선하고, 특히 Mac과 Windows 환경 간의 시간 정밀도 차이로 인해 발생하던 불안정한 테스트를 수정하는 것을 목표로 합니다. usedAt 필드가 생성 시점에 현재 시간으로 자동 설정되는 대신, 외부에서 주입된 값을 사용하도록 변경하여 유연성을 높였습니다. 또한, 테스트에서 시간 비교 로직을 나노초 단위에서 초 단위로 변경하여 플랫폼 독립적인 테스트 결과를 보장합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이 PR은 서로 다른 운영 체제에서 LocalDateTime의 정밀도 차이로 인해 발생하던 시간 기반 테스트의 불안정성 문제를 해결합니다. CustomizeTableEntity 생성자에서 usedAt 필드를 도메인 객체의 값을 사용하도록 수정한 것은 올바른 버그 수정입니다. 또한, 테스트 코드에서 minusNanos(1L)를 minusSeconds(1L)로 변경하여 시간 비교가 정밀도 문제에 영향을 받지 않도록 개선한 점도 좋습니다. 다만, 테스트의 안정성을 더욱 높이기 위해 몇 가지 제안 사항을 코멘트로 남겼습니다. 전반적으로 좋은 수정이라고 생각합니다.
| Member member = new Member("default@gmail.com"); | ||
| CustomizeTable table = new CustomizeTable(member, "tableName", "agenda", "찬성", "반대", | ||
| true, true, LocalDateTime.now().minusNanos(1L)); | ||
| true, true, LocalDateTime.now().minusSeconds(1L)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Member member = new Member("default@gmail.com"); | ||
| CustomizeTable table = new CustomizeTable(member, "tableName", "agenda", "찬성", "반대", | ||
| true, true, LocalDateTime.now().minusNanos(1L)); | ||
| true, true, LocalDateTime.now().minusSeconds(1L)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test Results135 files 135 suites 14s ⏱️ Results for commit cfa2c2c. |
📝 Test Coverage Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/debatetimer/entity/customize/CustomizeTableEntityTest.java (1)
50-50: 일관성을 위해minusSeconds(1L)사용을 권장합니다.이 테스트는
usedAt을 검증하지 않지만, 33번과 71번 라인과의 일관성을 위해minusSeconds(1L)로 변경하는 것이 코드베이스 전체의 통일성을 높일 수 있습니다.🔎 일관성을 위한 제안
CustomizeTable table = new CustomizeTable(member, "tableName", "agenda", "찬성", "반대", - true, true, LocalDateTime.now().minusNanos(1L)); + true, true, LocalDateTime.now().minusSeconds(1L));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/debatetimer/entity/customize/CustomizeTableEntity.java(1 hunks)src/test/java/com/debatetimer/entity/customize/CustomizeTableEntityTest.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push
🔇 Additional comments (2)
src/test/java/com/debatetimer/entity/customize/CustomizeTableEntityTest.java (1)
33-33: 크로스 플랫폼 타임스탬프 정밀도 문제 해결을 승인합니다.
minusNanos(1L)에서minusSeconds(1L)로 변경하여 H2의 마이크로초(6자리) 정밀도 절단으로 인한 플랫폼 간 테스트 불일치 문제를 해결했습니다. 1초 차이는isAfter단언문이 모든 플랫폼에서 일관되게 통과하기에 충분합니다.Also applies to: 71-71
src/main/java/com/debatetimer/entity/customize/CustomizeTableEntity.java (1)
63-63: 이 변경은 테스트를 위해 의도적으로 설계되었으며 프로덕션 환경에서는 안전합니다.
CustomizeTable인스턴스는 요청 처리 시점에LocalDateTime.now()로 생성되며, 같은 트랜잭션 내에서 즉시 엔티티로 영속화됩니다.CustomizeTable생성과 엔티티 저장 사이의 시간 차이는 무시할 수 있는 수준입니다. 이 변경으로 테스트에서 타임스탬프를 제어할 수 있게 되었으며,updateTable()과updateUsedAt()메서드의 동작을 검증하는 테스트가 정상적으로 작동합니다.Likely an incorrect or invalid review comment.
coli-geonwoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/noti
커찬, 흥미로운 이슈를 해결하셨네요 :) approve 합니다~
| this.warningBell = customizeTable.isWarningBell(); | ||
| this.finishBell = customizeTable.isFinishBell(); | ||
| this.usedAt = LocalDateTime.now(); | ||
| this.usedAt = customizeTable.getUsedAt(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 한번 영속화되었던 거니까 아무래도 벤더와 무관하게 나노초 범위를 똑같이 통일시켜준거군요 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트도 1초 뒤로 설정해서, DB와 무관하게 테스트를 통과하도록 수정했습니다~
unifolio0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/noti
approve합니다
🚩 연관 이슈
closed #
🗣️ 리뷰 요구사항 (선택)
아래 글을 읽어주세요.
해당 테스트가 Mac에서 통과하는 이유 (by Gemini)
윈도우에서 해당 테스트가 통과하지 않는 이유 (by Gemini)
그래서 1초 차이로 테스트에 반영하도록 수정했습니다. 추가적인 리뷰 부탁드립니다.
Summary by CodeRabbit
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.