[25.07.18 / TASK-225] Test - 주간 뉴스레터 배치 단위 테스트 추가 #37
Conversation
|
""" Walkthrough테스트 환경 개선과 사용자 중심의 주간 트렌드 및 뉴스레터 관련 데이터 픽스처가 추가되었습니다. 기존 픽스처가 사용자별 데이터를 반영하도록 수정되었으며, 비활성 사용자 케이스도 포함됩니다. 또한, 주간 뉴스레터 배치 작업의 주요 로직에 대한 단위 테스트가 새롭게 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant WeeklyNewsletterBatch
participant SESClient
participant DB
TestRunner->>WeeklyNewsletterBatch: run()
WeeklyNewsletterBatch->>DB: delete_old_mail_logs()
WeeklyNewsletterBatch->>DB: get_target_user_chunks()
WeeklyNewsletterBatch->>DB: get_weekly_trend()
WeeklyNewsletterBatch->>WeeklyNewsletterBatch: build_newsletter_objects()
WeeklyNewsletterBatch->>SESClient: send_email()
SESClient-->>WeeklyNewsletterBatch: email_sent
WeeklyNewsletterBatch->>DB: update_weekly_trend()
WeeklyNewsletterBatch->>DB: update_user_weekly_trend()
WeeklyNewsletterBatch-->>TestRunner: 결과 반환
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (15)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
insight/tests/conftest.py (1)
228-234: 도달할 수 없는 코드 제거 필요
trending_summary가 이미None으로 설정되었으므로 (225번 줄), 이 조건문은 절대 실행되지 않습니다.- # 사용자 인사이트는 제목을 조금 다르게 설정 - if insight_dict["trending_summary"]: - insight_dict["trending_summary"][0]["title"] = "Django 모델 최적화하기" - insight_dict["trending_summary"][0]["summary"] = ( - "Django ORM을 효율적으로 사용하는 방법" - )
🧹 Nitpick comments (3)
insight/tests/conftest.py (2)
1-18: pytest-django 플러그인 사용 권장conftest.py에서 직접 Django 설정을 로드하는 대신
pytest-django플러그인 사용을 고려해보세요. 이 플러그인은 Django 테스트 환경을 자동으로 구성하고 더 나은 테스트 격리를 제공합니다.
pytest.ini또는pyproject.toml에 다음 설정 추가:[tool.pytest.ini_options] DJANGO_SETTINGS_MODULE = "backoffice.settings.local"그리고 이 Django 설정 코드를 제거할 수 있습니다.
198-198: 주석이 코드 동작과 일치하지 않음주석에는 "주간 글 작성 사용자"라고 되어 있지만, 실제로는
user_weekly_reminder를None으로 설정하고 있습니다. 이는 주간 글을 작성한 사용자가 아니라 리마인더가 없는 사용자를 나타내는 것 같습니다.- insight_dict["user_weekly_reminder"] = None # 주간 글 작성 사용자 + insight_dict["user_weekly_reminder"] = None # 리마인더가 없는 사용자insight/tests/tasks/test_weekly_newsletter_batch.py (1)
68-70: 복잡한 mock 체이닝을 헬퍼 메서드로 개선 가능Django ORM의 체이닝을 모킹하는 패턴이 여러 테스트에서 반복됩니다. 가독성과 유지보수성을 위해 헬퍼 메서드 사용을 고려해보세요.
def mock_queryset(mock_filter, return_value): """Django QuerySet 체이닝을 모킹하는 헬퍼""" mock_filter.return_value.values.return_value.distinct.return_value = return_value return mock_filter
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
insight/tests/conftest.py(4 hunks)insight/tests/tasks/test_weekly_newsletter_batch.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
insight/tests/conftest.py (4)
insight/models.py (8)
TrendAnalysis(23-27)TrendingItem(10-19)UserWeeklyTrend(94-134)WeeklyTrend(58-91)WeeklyTrendInsight(31-33)WeeklyUserReminder(45-49)WeeklyUserStats(37-41)WeeklyUserTrendInsight(53-55)insight/schemas.py (1)
Newsletter(17-19)modules/mail/schemas.py (1)
EmailMessage(11-19)common/models.py (1)
to_json_dict(39-41)
🔇 Additional comments (3)
insight/tests/conftest.py (1)
106-124: 적절한 테스트 픽스처 구현주간 사용자 통계와 리마인더를 위한 픽스처가 명확하고 적절하게 구현되었습니다.
insight/tests/tasks/test_weekly_newsletter_batch.py (2)
43-60: 명확한 테스트 구조Given-When-Then 패턴을 사용한 테스트 구조가 매우 명확하고 이해하기 쉽습니다. AAA 패턴이 일관되게 적용되어 있어 좋습니다.
330-435: 포괄적인 엣지 케이스 테스트다양한 실패 시나리오와 엣지 케이스를 잘 다루고 있습니다:
- 대상 사용자 없음
- 주간 트렌드 데이터 없음
- 낮은 성공률 시나리오
테스트 커버리지가 우수합니다.
| @patch("insight.tasks.weekly_newsletter_batch.logger") | ||
| def test_delete_old_maillogs_success(self, mock_logger, newsletter_batch): | ||
| """7일 이전 메일 로그 삭제 성공 테스트""" | ||
| with patch.object(NotiMailLog.objects, "filter") as mock_filter: | ||
| mock_delete = MagicMock() | ||
| mock_filter.return_value.delete = mock_delete | ||
| mock_delete.return_value = (2, {"noti.NotiMailLog": 2}) | ||
|
|
||
| newsletter_batch._delete_old_maillogs() | ||
|
|
||
| mock_filter.assert_called_once_with( | ||
| created_at__lt=newsletter_batch.before_a_week, is_success=True | ||
| ) | ||
| mock_delete.assert_called_once() |
There was a problem hiding this comment.
실제 메일 로그 model 도 mocking 하는 것은 별로일까요?
사실 이건 newsletter_batch._delete_old_maillogs 호출 했냐 테스트 밖에 안되는데...!
There was a problem hiding this comment.
삭제 결과 개수 또는 로그도 검증하면 더 명확한 테스트가 되지 않을까 싶습니다!
다은님은 어떻게 생각하시나요??
There was a problem hiding this comment.
해당 메서드에서 별다른 처리 없이 DB delete만 하고 있어서 애매해진 것 같네요. 전 올바른 where 조건으로 호출하고 있는지 검증하는데 의미가 있다고 생각하였습니다. 요거는 통합테스트를 추가해두겠습니다!!
| @patch("insight.tasks.weekly_newsletter_batch.logger") | ||
| def test_delete_old_maillogs_success(self, mock_logger, newsletter_batch): | ||
| """7일 이전 메일 로그 삭제 성공 테스트""" | ||
| with patch.object(NotiMailLog.objects, "filter") as mock_filter: | ||
| mock_delete = MagicMock() | ||
| mock_filter.return_value.delete = mock_delete | ||
| mock_delete.return_value = (2, {"noti.NotiMailLog": 2}) | ||
|
|
||
| newsletter_batch._delete_old_maillogs() | ||
|
|
||
| mock_filter.assert_called_once_with( | ||
| created_at__lt=newsletter_batch.before_a_week, is_success=True | ||
| ) | ||
| mock_delete.assert_called_once() |
There was a problem hiding this comment.
삭제 결과 개수 또는 로그도 검증하면 더 명확한 테스트가 되지 않을까 싶습니다!
다은님은 어떻게 생각하시나요??
| ] | ||
|
|
||
| with patch.object(User.objects, "filter") as mock_filter: | ||
| mock_filter.return_value.values.return_value.distinct.return_value = mock_users |
There was a problem hiding this comment.
실제 fixture가 아닌 filter를 mocking 하신 이유가 있을까요? 궁금해서 여쭤봅니다!
There was a problem hiding this comment.
user fixture를 반환하도록 Django ORM 자체를 모킹했다고 보시면 될 듯합니다! (체이닝 첫번째 메서드인 filter를 모킹)
There was a problem hiding this comment.
딱 하나 아쉬운 테스트 세트 중 template 렌더링, otuput 에 대한게 아쉽습니다!
아예 파일을 따로 추가하는 것도 좋을 것 같아서요!
- 토큰 만료되어서 insight_userweeklytrend 가 없는 분들 템플릿
- 통계 주간 글이 없는 분들 템플릿
- 둘 다 있는 분들 템플릿
index.html / insight_preview.html / user_weekly_trend.html / weekly_trend.html 템플릿 대상!
| @patch("insight.tasks.weekly_newsletter_batch.logger") | ||
| def test_delete_old_maillogs_success(self, mock_logger, newsletter_batch): | ||
| """7일 이전 메일 로그 삭제 성공 테스트""" | ||
| with patch.object(NotiMailLog.objects, "filter") as mock_filter: | ||
| mock_delete = MagicMock() | ||
| mock_filter.return_value.delete = mock_delete | ||
| mock_delete.return_value = (2, {"noti.NotiMailLog": 2}) | ||
|
|
||
| newsletter_batch._delete_old_maillogs() | ||
|
|
||
| mock_filter.assert_called_once_with( | ||
| created_at__lt=newsletter_batch.before_a_week, is_success=True | ||
| ) | ||
| mock_delete.assert_called_once() |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
insight/tests/conftest.py (1)
27-35: 메모리 릭 가능성이 있는 구현입니다.과거 리뷰에서 지적된 것처럼 yield에서 에러 발생 시
del이 실행되지 않을 수 있습니다.patch.dict를 사용하거나 try-finally 블록으로 안전하게 처리하는 것을 권장합니다.-@pytest.fixture -def mock_setup_django(): - """setup_django 모듈 모킹""" - sys.modules["setup_django"] = MagicMock() - try: - yield sys.modules["setup_django"] - finally: - # 테스트 후 정리 - del sys.modules["setup_django"] +@pytest.fixture +def mock_setup_django(): + """setup_django 모듈 모킹""" + mock_module = MagicMock() + try: + sys.modules["setup_django"] = mock_module + yield mock_module + finally: + # 테스트 후 정리 + sys.modules.pop("setup_django", None)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
insight/tasks/weekly_newsletter_batch.py(1 hunks)insight/tests/conftest.py(7 hunks)insight/tests/tasks/test_weekly_newsletter_batch.py(1 hunks)insight/tests/tasks/test_weekly_newsletter_template.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- insight/tests/tasks/test_weekly_newsletter_batch.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
insight/tasks/weekly_newsletter_batch.py (1)
noti/models.py (1)
NotiMailLog(49-78)
🔇 Additional comments (12)
insight/tasks/weekly_newsletter_batch.py (1)
74-74: 독스트링 개선이 적절합니다.구체적인 "7일" 대신 "이전 뉴스레터"로 일반화한 것이 메서드의 실제 역할을 더 잘 설명합니다.
insight/tests/tasks/test_weekly_newsletter_template.py (9)
9-27: 픽스처 구조가 잘 설계되었습니다.SESClient 모킹과 WeeklyNewsletterBatch 인스턴스 생성이 적절합니다. 의존성 주입 패턴이 잘 적용되어 있습니다.
31-66: 포괄적인 성공 케이스 테스트입니다.데이터베이스 쿼리, 상태 업데이트, 템플릿 렌더링 결과를 모두 검증하는 잘 구성된 테스트입니다.
67-81: 데이터 없음 케이스에 대한 적절한 예외 처리 테스트입니다.예외 메시지까지 정확히 검증하는 견고한 테스트입니다.
83-107: 템플릿 렌더링 실패 시나리오를 잘 테스트했습니다.필수 요소가 없는 HTML을 반환하도록 모킹하여 실제 실패 상황을 적절히 시뮬레이션했습니다.
109-133: 활성 사용자의 개인화된 콘텐츠 렌더링을 잘 테스트했습니다.사용자 정보, 게시물 통계, 트렌드 분석 등 핵심 요소들의 포함 여부를 적절히 검증합니다.
134-157: 비활성 사용자에 대한 차별화된 처리를 잘 테스트했습니다.활성 사용자용 콘텐츠의 부재와 리마인더 메시지 포함을 적절히 검증합니다.
158-176: 예외 처리가 적절히 테스트되었습니다.템플릿 렌더링 실패 시 예외가 제대로 전파되는지 확인하는 중요한 테스트입니다.
178-196: 정상 사용자의 뉴스레터 렌더링을 잘 테스트했습니다.토큰 상태에 따른 조건부 콘텐츠 렌더링을 적절히 검증합니다.
197-233: 토큰 만료 사용자와 예외 처리 케이스를 잘 테스트했습니다.토큰 상태에 따른 조건부 렌더링과 예외 상황 처리를 모두 적절히 검증합니다.
insight/tests/conftest.py (2)
110-174: 새로운 픽스처들이 잘 설계되었습니다.주간 사용자 통계, 리마인더, 뉴스레터 관련 데이터를 체계적으로 제공하는 유용한 픽스처들입니다. dataclass 구조를 잘 활용하고 있습니다.
176-243: 기존 픽스처 개선이 적절합니다.
get_previous_week_range()함수 사용으로 timezone 문제가 해결되었고, 사용자 활동 상태에 따른 차별화된 테스트 데이터 제공이 잘 구현되었습니다.
🔥 변경 사항
./insight/tests/tasks위치에WeeklyNewsletterBatch의 단위 테스트를 추가하였습니다../insight/tests/conftest.py에 추가하였습니다.🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
X
📌 체크리스트
Summary by CodeRabbit
Summary by CodeRabbit
테스트
기능 개선