Skip to content

Conversation

@Jihyun3478
Copy link
Contributor

@Jihyun3478 Jihyun3478 commented Mar 7, 2025

  • 통계 배치 클래스(main.py - Scraper 클래스) 내 5개 메서드에 대응하는 성공/실패 테스트 코드 구현
  • 테스트 커버리지 100% 완료

PS. PR이 너무 늦었네요,,🥲

Summary by CodeRabbit

Summary by CodeRabbit

  • 테스트
    • 시스템의 핵심 기능 품질 보증을 위해 포괄적인 테스트 케이스를 추가하였습니다. 이번 업데이트에서는 토큰 관리, 대량 데이터 처리, 일별 통계 업데이트 및 사용자 데이터 처리 기능의 안정성을 검증하여 전반적인 시스템 신뢰성과 성능 향상에 기여합니다.
  • CI/CD
    • CI 워크플로우에 환경 변수 설정을 추가하여 보안 키를 안전하게 포함할 수 있도록 하였습니다.
  • 삭제
    • 기본적인 덧셈 기능과 관련된 테스트 파일이 삭제되었습니다.
  • 개선
    • 토큰 업데이트 프로세스의 오류 처리를 강화하여 유효하지 않은 토큰에 대한 조기 종료 조건을 추가하였습니다.

@Jihyun3478 Jihyun3478 self-assigned this Mar 7, 2025
@notion-workspace
Copy link

@coderabbitai
Copy link

coderabbitai bot commented Mar 7, 2025

Walkthrough

새로운 테스트 스위트가 추가되어 Scraper 클래스의 주요 메서드를 검증합니다. 해당 테스트 파일은 pytest 프레임워크와 모의 객체(mocks)를 사용하며, fixtures를 통해 Scraper 인스턴스와 테스트용 User 객체를 초기화합니다. 테스트 항목에는 토큰 업데이트, 대량 게시물 삽입, 일일 통계 업데이트, 제한된 게시물 통계 조회, 사용자 처리 기능이 포함되어 있습니다. 또한 CI 워크플로우에서 환경 변수를 설정하는 부분에 새로운 키가 추가되었습니다. 이전의 간단한 테스트 파일은 삭제되었습니다.

Changes

파일 변경 요약
scraping/.../test_main.py Scraper 클래스의 기능을 검증하는 새로운 테스트 스위트가 추가되었습니다. 테스트는 fixtures, 토큰 업데이트, 대량 게시물 삽입, 일일 통계 업데이트, 제한된 게시물 통계 조회, 사용자 처리 과정을 포함합니다.
.github/workflows/test-ci.yaml CI 워크플로우의 ".env 파일 생성" 단계에 9개의 새로운 환경 변수 할당이 추가되었습니다. 각 키는 GitHub 비밀에서 가져온 값입니다.
scraping/.../test_sample.py 간단한 덧셈 기능과 관련된 테스트 파일이 삭제되었습니다.
scraping/.../main.py update_old_tokens 메서드에 새로운 조건 검사가 추가되어, 유효하지 않은 토큰에 대한 처리가 개선되었습니다.

Sequence Diagram(s)

sequenceDiagram
    participant T as 테스트 실행기
    participant S as Scraper
    participant M as 모의 함수
    T->>S: process_user(user)
    S->>M: fetch_velog_user_chk(user)
    alt 유효한 사용자
        S->>S: update_old_tokens(user)
        S->>M: fetch_all_velog_posts(user)
    else
        S-->>T: 오류 반환
    end
Loading

Poem

나는 귀여운 토끼, 코드 숲을 달리네,
테스트가 추가되어 발랄한 하루네.
모의 함수들과 함께 춤을 추며,
Scraper 검증에 hop, hop 뛰어오르네.
코드와 꿈 사이, 내 발자국은 반짝반짝! 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

🔭 Outside diff range comments (1)
scraping/tests/test_main.py (1)

1-120: ⚠️ Potential issue

환경 변수 설정 관련 문제 해결 필요

파이프라인 실패 로그에서 AES_KEY_1 환경 변수가 설정되지 않았다는 오류가 발생했습니다. 테스트 환경에서 이 환경 변수를 설정하거나 모킹해야 합니다.

테스트 설정 파일에 환경 변수를 추가하거나, 아래와 같이 테스트 코드에서 환경 변수를 모킹하는 방법을 고려해보세요:

@pytest.fixture(autouse=True)
def setup_env_vars(monkeypatch):
    """테스트에 필요한 환경 변수 설정"""
    monkeypatch.setenv("AES_KEY_1", "test_key_for_encryption")

이 픽스처를 TestScraper 클래스 내에 추가하면 모든 테스트에서 자동으로 사용됩니다.

🧰 Tools
🪛 GitHub Actions: Test CI

[error] 117-117: django.core.exceptions.ImproperlyConfigured: Set the AES_KEY_1 environment variable

🧹 Nitpick comments (2)
scraping/tests/test_main.py (2)

69-81: 일일 통계 업데이트 테스트 강화 필요

test_update_daily_statistics 메서드는 sync_to_async가 호출되었는지만 확인하고 있습니다. 더 강력한 테스트를 위해 호출 인자와 횟수도 검증하는 것이 좋습니다.

아래와 같이 구체적인 검증을 추가하는 것을 고려해보세요:

    await scraper.update_daily_statistics(post_data, stats_data)

    mock_sync_to_async.assert_called()
+   # 어떤 인자로 호출되었는지 검증
+   mock_sync_to_async().assert_called_once()
+   # 호출된 함수의 인자 검증
+   args, kwargs = mock_sync_to_async.call_args
+   assert 'update_or_create' in str(args[0])

96-120: 사용자 처리 테스트 검증 강화 필요

test_process_user 메서드는 update_old_tokens가 호출되었는지만 검증하고 있습니다. process_user 메서드의 전체 결과를 검증하는 추가 검증이 필요합니다.

아래와 같이 추가 검증을 고려해보세요:

    with patch.object(
        scraper, "update_old_tokens", new_callable=AsyncMock
    ) as mock_update_tokens:
+       with patch.object(
+           scraper, "bulk_insert_posts", new_callable=AsyncMock, return_value=True
+       ) as mock_bulk_insert:
            await scraper.process_user(user, MagicMock())

    mock_update_tokens.assert_called_once()
+   # 추가 검증
+   mock_fetch_user_chk.assert_called_once()
+   mock_fetch_posts.assert_called_once()
+   if mock_fetch_posts.return_value:  # 빈 목록이 아닌 경우
+       mock_bulk_insert.assert_called_once()
🧰 Tools
🪛 GitHub Actions: Test CI

[error] 117-117: django.core.exceptions.ImproperlyConfigured: Set the AES_KEY_1 environment variable

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffa063e and ccd5ca8.

📒 Files selected for processing (1)
  • scraping/tests/test_main.py (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Test CI
scraping/tests/test_main.py

[error] 117-117: django.core.exceptions.ImproperlyConfigured: Set the AES_KEY_1 environment variable


[warning] 2028-2028: RuntimeWarning: coroutine 'AsyncMockMixin._execute_mock_call' was never awaited

🔇 Additional comments (6)
scraping/tests/test_main.py (6)

1-8: 적절한 모듈 임포트와 구조 설정

테스트에 필요한 모듈들이 잘 임포트되어 있습니다. AsyncMock, MagicMock, patch를 활용한 비동기 테스트 환경이 적절히 구성되어 있습니다.


10-27: 테스트 클래스와 픽스처 정의가 명확함

TestScraper 클래스와 필요한 픽스처들(scraper, user)이 잘 정의되어 있습니다. 특히 각 픽스처에 대한 한글 주석이 이해하기 쉽게 작성되어 있습니다.


28-48: 토큰 업데이트 테스트 구현이 완성도 높음

test_update_old_tokens 메서드는 암호화/복호화 과정을 적절히 모킹하고, 사용자 토큰이 올바르게 업데이트되는지 검증합니다. 특히 모킹과 검증 로직이 명확하게 구현되어 있습니다.


49-68: 벌크 삽입 테스트 로직이 적절함

test_bulk_insert_posts 메서드는 배치 크기에 따른 삽입 로직을 적절히 테스트하고 있습니다. 호출 횟수를 검증하는 방식으로 배치 처리 로직을 효과적으로 테스트했습니다.


82-95: 포스트 통계 제한 조회 테스트가 잘 구현됨

test_fetch_post_stats_limited 메서드는 재시도 로직을 효과적으로 테스트하고 있습니다. 실패와 성공 케이스를 모두 처리하는 로직이 잘 검증되었습니다.


96-120: 미대기 코루틴 경고 해결 필요

파이프라인에서 coroutine 'AsyncMockMixin._execute_mock_call' was never awaited 경고가 발생했습니다. 이는 비동기 모킹 객체의 메서드를 await 하지 않았을 때 발생합니다.

특히 test_process_user 메서드에서 mock_update_tokens.assert_called_once()를 사용할 때 비동기 모킹 객체의 결과를 대기하지 않는 문제가 있을 수 있습니다. 아래와 같은 수정을 고려해보세요:

    with patch.object(
        scraper, "update_old_tokens", new_callable=AsyncMock
    ) as mock_update_tokens:
-       await scraper.process_user(user, MagicMock())
+       result = await scraper.process_user(user, MagicMock())
+       # 결과 검증 추가

-   mock_update_tokens.assert_called_once()
+   # 비동기 모킹 객체의 호출 검증
+   assert mock_update_tokens.called
+   assert mock_update_tokens.call_count == 1

이렇게 비동기 모킹 객체의 메서드 호출을 검증하는 방식으로 변경하면 경고를 해결할 수 있습니다.

🧰 Tools
🪛 GitHub Actions: Test CI

[error] 117-117: django.core.exceptions.ImproperlyConfigured: Set the AES_KEY_1 environment variable

Copy link
Member

@Nuung Nuung left a comment

Choose a reason for hiding this comment

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

mocking 방향, return data mocking 다 좋았습니다!
전체 테스트 로직 이전에, 현재 CI 에서 터져서요~ 해당 부분 먼체 체크 이후 PR 리뷰 다시할게요!

Copy link

@cheonHwi cheonHwi left a comment

Choose a reason for hiding this comment

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

저도 테스트 코드 작성하면서 어떤식으로 작성할지 굉장히 고민이 많았는데 어떤 데이터를 어떻게 모킹해서 사용해야하나에 대한 고민했던 부분이 잘 드러난 것 같아요! 파이썬을 쓰지 않는데도 코드에 큰 어려움도 없었네요

첫 PR 고생 많으셨어요!

Copy link
Member

@Nuung Nuung left a comment

Choose a reason for hiding this comment

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

좋았던 점

  • pytest 를 처음 씀에도 픽스쳐나, 목킹에 있어서 아주 좋았습니다 :) 고생많으셨어요.
  • 성공 케이스에 대한 바람직한 테스트들도 작성을 잘 해주신 것 같아요.

아쉬운 점

  • 아쉽게 "성공 케이스만" 있는 테스트 코드는 테스트 코드로써 크게 유의미 하지 않습니다.
  • 비즈니스로직이 이미 잘 돌아가는 코드니까요!
  • 첫 코멘트와 같이 "실패 케이스" 들이 필요합니다. 예를 들어서 "토큰 업데이트 안하는 경우", fetch_post_stats 를 실패하면 어떻게 되는건지, 예외는 뭐가 있는지
  • 그리고 테크니컬리는 "함수가 인자를 잘 받고 파라미터로 활용 했는지", "호출이 된건지 안된건지"
  • 더 어렵게 접근하면 트랜잭션 중 실패를 해서 rollback 을 하는지까지 테스팅을 해야 할 것 같아요 (이는 옵션)
  • 최대한 pytest 에 대한 리뷰가 아니라, 테스트 코드 자체에 대한 피드백을 담아 보려고 했습니다!

@Jihyun3478
Copy link
Contributor Author

저도 테스트 코드 작성하면서 어떤식으로 작성할지 굉장히 고민이 많았는데 어떤 데이터를 어떻게 모킹해서 사용해야하나에 대한 고민했던 부분이 잘 드러난 것 같아요! 파이썬을 쓰지 않는데도 코드에 큰 어려움도 없었네요

첫 PR 고생 많으셨어요!

리뷰 감사합니다!!
모킹이 생각보다 쉬운 개념이면서도 어렵네요😅 천휘님도 파이팅입니다!!🔥

@Jihyun3478
Copy link
Contributor Author

Jihyun3478 commented Mar 8, 2025

좋았던 점

  • pytest 를 처음 씀에도 픽스쳐나, 목킹에 있어서 아주 좋았습니다 :) 고생많으셨어요.
  • 성공 케이스에 대한 바람직한 테스트들도 작성을 잘 해주신 것 같아요.

아쉬운 점

  • 아쉽게 "성공 케이스만" 있는 테스트 코드는 테스트 코드로써 크게 유의미 하지 않습니다.
  • 비즈니스로직이 이미 잘 돌아가는 코드니까요!
  • 첫 코멘트와 같이 "실패 케이스" 들이 필요합니다. 예를 들어서 "토큰 업데이트 안하는 경우", fetch_post_stats 를 실패하면 어떻게 되는건지, 예외는 뭐가 있는지
  • 그리고 테크니컬리는 "함수가 인자를 잘 받고 파라미터로 활용 했는지", "호출이 된건지 안된건지"
  • 더 어렵게 접근하면 트랜잭션 중 실패를 해서 rollback 을 하는지까지 테스팅을 해야 할 것 같아요 (이는 옵션)
  • 최대한 pytest 에 대한 리뷰가 아니라, 테스트 코드 자체에 대한 피드백을 담아 보려고 했습니다!

실패 케이스에 대한 테스트 코드를 작성하는게 확실히

  1. 현재 코드에서 발생할 수 있는 이슈가 존재하는지
  2. 예측하지 못한 상황이 존재하는지

를 확인할 수 있어 서비스가 운영되기 이전에 확인할 수 있는 안전장치가 될 것 같네요!
테스트코드를 구현할 때 조금 더 깊이 고민하는 디테일이 필요할 것 같다는 생각이 드네요! 자세한 피드백 정말 감사합니다!!🚀

PS. 해당 부분은 오늘 내로 이번 PR에 반영하겠습니다!

Copy link
Member

@six-standard six-standard left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!
첫 PR인데도 잘 해주신 것 같아요~~

좋았던 점

  • 모킹 구조가 안정적으로 잡혀있어 만족스러웠습니다~

아쉬웠던 점

  • 실패 케이스에 대한 테스트가 없어서 아쉽네요
    예상치 못한 상황의 오류와 (토큰 만료, 잘못된 값 등), 추후 리팩토링시 발생할 수 있는 오류를 잡아낼 수 있을까 싶습니다

@Jihyun3478
Copy link
Contributor Author

수고하셨습니다! 첫 PR인데도 잘 해주신 것 같아요~~

좋았던 점

  • 모킹 구조가 안정적으로 잡혀있어 만족스러웠습니다~

아쉬웠던 점

  • 실패 케이스에 대한 테스트가 없어서 아쉽네요
    예상치 못한 상황의 오류와 (토큰 만료, 잘못된 값 등), 추후 리팩토링시 발생할 수 있는 오류를 잡아낼 수 있을까 싶습니다

피드백 감사합니다! 실패 케이스를 고려하지 못했네요🥲
해당 부분은 오늘 내로 반영하겠습니다~!!

@Nuung Nuung added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 8, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
scraping/tests/test_main.py (1)

98-122: 🛠️ Refactor suggestion

process_user 테스트 개선 필요

현재 테스트는 유저 처리 성공 케이스만 테스트하고 있습니다. 토큰 업데이트 실패, 게시물 가져오기 실패 등의 다양한 시나리오와 트랜잭션 처리에 대한 테스트가 필요합니다.

@patch("scraping.main.fetch_velog_user_chk")
@patch("scraping.main.fetch_all_velog_posts")
@patch("scraping.main.AESEncryption")
@pytest.mark.asyncio
async def test_process_user_success(
    self, mock_aes, mock_fetch_posts, mock_fetch_user_chk, scraper, user
):
    """유저 데이터 전체 처리 성공 테스트"""
    mock_encryption = mock_aes.return_value
    mock_encryption.decrypt.side_effect = lambda token: f"decrypted-{token}"
    mock_encryption.encrypt.side_effect = lambda token: f"encrypted-{token}"

    mock_fetch_user_chk.return_value = (
        {"access_token": "new-token"},
        {"data": {"currentUser": {"username": "testuser"}}},
    )
    mock_fetch_posts.return_value = []

    with patch.object(
        scraper, "update_old_tokens", new_callable=AsyncMock
    ) as mock_update_tokens:
        await scraper.process_user(user, MagicMock())

    mock_update_tokens.assert_called_once()
+   # 파라미터 검증
+   args, kwargs = mock_update_tokens.call_args
+   assert user in args
+   assert mock_encryption in args
+   assert {"access_token": "new-token"} in args or {"access_token": "new-token"} in str(kwargs)
+   
+   # fetch_velog_user_chk 호출 검증
+   mock_fetch_user_chk.assert_called_once()
+   args, kwargs = mock_fetch_user_chk.call_args
+   assert "decrypted-encrypted-refresh-token" in str(args) or "decrypted-encrypted-refresh-token" in str(kwargs)

또한 실패 케이스 테스트도 추가해야 합니다:

@patch("scraping.main.fetch_velog_user_chk")
@patch("scraping.main.AESEncryption")
@pytest.mark.asyncio
async def test_process_user_token_update_failure(
    self, mock_aes, mock_fetch_user_chk, scraper, user
):
    """토큰 업데이트 실패 테스트"""
    mock_encryption = mock_aes.return_value
    mock_encryption.decrypt.side_effect = lambda token: f"decrypted-{token}"
    
    # 토큰 갱신 실패 시뮬레이션
    mock_fetch_user_chk.return_value = (None, None)
    
    with patch.object(
        scraper, "update_old_tokens", new_callable=AsyncMock
    ) as mock_update_tokens:
        # 구현에 따라 예외 처리 방식이 다를 수 있음
        await scraper.process_user(user, MagicMock())
    
    # 토큰 업데이트 시도하지 않음
    mock_update_tokens.assert_not_called()

트랜잭션 처리에 대한 테스트도 고려해야 합니다:

@patch("scraping.main.fetch_velog_user_chk")
@patch("scraping.main.fetch_all_velog_posts")
@patch("scraping.main.AESEncryption")
@patch("scraping.main.transaction.atomic")
@pytest.mark.asyncio
async def test_process_user_transaction_rollback(
    self, mock_atomic, mock_aes, mock_fetch_posts, mock_fetch_user_chk, scraper, user
):
    """트랜잭션 롤백 테스트"""
    mock_encryption = mock_aes.return_value
    
    # 성공적인 토큰 갱신
    mock_fetch_user_chk.return_value = (
        {"access_token": "new-token"},
        {"data": {"currentUser": {"username": "testuser"}}},
    )
    
    # 게시물 가져오기 실패
    mock_fetch_posts.side_effect = Exception("API Error")
    
    # transaction.atomic 컨텍스트 매니저 모의
    mock_cm = MagicMock()
    mock_atomic.return_value = mock_cm
    
    with patch.object(
        scraper, "update_old_tokens", new_callable=AsyncMock
    ) as mock_update_tokens:
        try:
            await scraper.process_user(user, MagicMock())
        except Exception:
            pass
    
    # transaction.atomic이 호출되었는지 확인
    mock_atomic.assert_called_once()
    # 컨텍스트 매니저가 진입되었는지 확인
    mock_cm.__enter__.assert_called_once()
    # 예외 발생 시 __exit__이 호출되었는지 확인
    mock_cm.__exit__.assert_called_once()
♻️ Duplicate comments (1)
scraping/tests/test_main.py (1)

30-50: 🛠️ Refactor suggestion

토큰 업데이트 테스트에 파라미터 검증과 실패 케이스 추가 필요

현재 테스트는 성공 케이스만 다루고 있으며, 모의 객체가 올바른 파라미터로 호출되었는지 검증하지 않습니다. 또한 토큰이 변경되지 않는 경우나 업데이트 실패 케이스에 대한 테스트가 누락되어 있습니다.

다음과 같이 개선하세요:

  1. 실제 파라미터 검증 추가
  2. 토큰 변경이 없는 케이스 테스트 추가
  3. 실패 케이스 테스트 추가
@patch("scraping.main.AESEncryption")
@pytest.mark.asyncio
async def test_update_old_tokens_success(self, mock_aes, scraper, user):
    """토큰 업데이트 성공 테스트"""
    mock_encryption = mock_aes.return_value
    mock_encryption.decrypt.side_effect = lambda token: f"decrypted-{token}"
    mock_encryption.encrypt.side_effect = lambda token: f"encrypted-{token}"

    new_tokens = {
        "access_token": "new-access-token",
        "refresh_token": "new-refresh-token",
    }

    with patch.object(user, "asave", new_callable=AsyncMock) as mock_asave:
        result = await scraper.update_old_tokens(user, mock_encryption, new_tokens)

    assert result is True
    mock_asave.assert_called_once()
    assert user.access_token == "encrypted-new-access-token"
    assert user.refresh_token == "encrypted-new-refresh-token"
+   # 올바른 파라미터로 decrypt 호출되었는지 검증
+   mock_encryption.decrypt.assert_any_call("encrypted-access-token")
+   mock_encryption.decrypt.assert_any_call("encrypted-refresh-token")
+   # 올바른 파라미터로 encrypt 호출되었는지 검증
+   mock_encryption.encrypt.assert_any_call("new-access-token")
+   mock_encryption.encrypt.assert_any_call("new-refresh-token")

추가로 구현해야 할 테스트 메서드 예시:

@patch("scraping.main.AESEncryption")
@pytest.mark.asyncio
async def test_update_old_tokens_no_change(self, mock_aes, scraper, user):
    """토큰 변경이 없는 경우 테스트"""
    mock_encryption = mock_aes.return_value
    mock_encryption.decrypt.side_effect = lambda token: f"decrypted-{token}"
    
    # 토큰이 변경되지 않은 경우 (decrypt된 값과 같은 새 토큰)
    new_tokens = {
        "access_token": "decrypted-encrypted-access-token",
        "refresh_token": "decrypted-encrypted-refresh-token",
    }

    with patch.object(user, "asave", new_callable=AsyncMock) as mock_asave:
        result = await scraper.update_old_tokens(user, mock_encryption, new_tokens)

    assert result is True
    # 변경이 없으므로 저장 호출되지 않음
    mock_asave.assert_not_called()
    # 원래 토큰 값이 유지됨
    assert user.access_token == "encrypted-access-token"
    assert user.refresh_token == "encrypted-refresh-token"
🧹 Nitpick comments (2)
scraping/tests/test_main.py (2)

1-9: 사용되지 않는 import 제거 필요

django.db.transactionposts.models.Post가 가져오기되었지만 코드에서 사용되지 않고 있습니다. 미사용 임포트는 제거하는 것이 좋습니다.

from unittest.mock import AsyncMock, MagicMock, patch
-from django.db import transaction
+
import pytest
import uuid

from users.models import User
-from posts.models import Post
from scraping.main import Scraper
🧰 Tools
🪛 Ruff (0.8.2)

2-2: django.db.transaction imported but unused

Remove unused import: django.db.transaction

(F401)


8-8: posts.models.Post imported but unused

Remove unused import: posts.models.Post

(F401)


51-70: bulk_insert_posts 테스트에 실패 케이스 및 엣지 케이스 추가 필요

현재 테스트는 성공적인 삽입만 테스트하고 있습니다. 빈 데이터 리스트, 예외 발생 상황 등의 엣지 케이스를 추가하면 더 완전한 테스트가 될 것입니다.

@patch("scraping.main.Post.objects.bulk_create", new_callable=AsyncMock)
@pytest.mark.asyncio
async def test_bulk_insert_posts_empty_data(self, mock_bulk_create, scraper, user):
    """빈 데이터 리스트 테스트"""
    posts_data = []
    
    result = await scraper.bulk_insert_posts(user, posts_data, batch_size=10)
    
    assert result is True  # 또는 구현에 따라 False일 수 있음
    mock_bulk_create.assert_not_called()

또한 예외 처리 테스트도 추가하면 좋을 것입니다:

@patch("scraping.main.Post.objects.bulk_create", new_callable=AsyncMock)
@pytest.mark.asyncio
async def test_bulk_insert_posts_exception(self, mock_bulk_create, scraper, user):
    """예외 발생 상황 테스트"""
    posts_data = [{"id": "post-1"}]
    mock_bulk_create.side_effect = Exception("Database error")
    
    # 구현에 따라 예외를 잡아내거나 전파할 수 있음
    try:
        result = await scraper.bulk_insert_posts(user, posts_data, batch_size=10)
        assert result is False  # 실패 시 False 반환 가정
    except Exception:
        # 또는 예외가 전파되는 것이 예상된 동작일 수 있음
        pass
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1641fde and 1b2be1d.

📒 Files selected for processing (1)
  • scraping/tests/test_main.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
scraping/tests/test_main.py

2-2: django.db.transaction imported but unused

Remove unused import: django.db.transaction

(F401)


8-8: posts.models.Post imported but unused

Remove unused import: posts.models.Post

(F401)

🔇 Additional comments (1)
scraping/tests/test_main.py (1)

1-122: 🛠️ Refactor suggestion

전체적으로 실패 케이스 테스트 부족

PR 코멘트에서 언급된 것처럼 현재 테스트는 성공 케이스만 다루고 있어 실제 서비스에서 발생할 수 있는 다양한 예외 상황을 검증하지 못합니다. 토큰 갱신 실패, API 호출 오류, 데이터베이스 예외 등의 실패 케이스 테스트를 추가해야 합니다.

테스트 커버리지가 83%로 보고되었지만, 적절한 실패 케이스 테스트가 없다면 코드의 견고성을 보장할 수 없습니다. 각 메서드에 대해 최소한 다음과 같은 실패 케이스를 추가하는 것이 좋습니다:

  1. 외부 API 호출 실패
  2. 데이터베이스 작업 실패
  3. 비동기 작업 타임아웃
  4. 파라미터 유효성 검증 실패
  5. 트랜잭션 롤백 검증

또한 모의 객체가 올바른 파라미터로 호출되었는지 검증하는 assert 구문을 각 테스트에 추가하는 것이 필요합니다.

🧰 Tools
🪛 Ruff (0.8.2)

2-2: django.db.transaction imported but unused

Remove unused import: django.db.transaction

(F401)


8-8: posts.models.Post imported but unused

Remove unused import: posts.models.Post

(F401)

Comment on lines 71 to 83
@patch("scraping.main.sync_to_async", new_callable=MagicMock)
@pytest.mark.asyncio
async def test_update_daily_statistics_success(self, mock_sync_to_async, scraper):
"""데일리 통계 업데이트 또는 생성 성공 테스트"""
post_data = {"id": "post-123"}
stats_data = {"data": {"getStats": {"total": 100}}, "likes": 5}

mock_sync_to_async.return_value = AsyncMock()

await scraper.update_daily_statistics(post_data, stats_data)

mock_sync_to_async.assert_called()

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

update_daily_statistics 테스트 개선 필요

현재 테스트는 sync_to_async가 호출되었는지만 확인하고 있습니다. 올바른 파라미터로 호출되었는지 검증하지 않으며, 실패 케이스도 다루지 않고 있습니다.

@patch("scraping.main.sync_to_async", new_callable=MagicMock)
@pytest.mark.asyncio
async def test_update_daily_statistics_success(self, mock_sync_to_async, scraper):
    """데일리 통계 업데이트 또는 생성 성공 테스트"""
    post_data = {"id": "post-123"}
    stats_data = {"data": {"getStats": {"total": 100}}, "likes": 5}

-   mock_sync_to_async.return_value = AsyncMock()
+   mock_async_func = AsyncMock()
+   mock_sync_to_async.return_value = mock_async_func

    await scraper.update_daily_statistics(post_data, stats_data)

    mock_sync_to_async.assert_called()
+   # 호출된 비동기 함수가 올바른 파라미터로 호출되었는지 검증
+   mock_async_func.assert_called_once()
+   # 함수 호출 시 전달된 인자 검증 (실제 구현에 맞게 수정 필요)
+   args, kwargs = mock_async_func.call_args
+   assert "post-123" in str(args) or "post-123" in str(kwargs)
+   assert 100 in str(args) or 100 in str(kwargs)
+   assert 5 in str(args) or 5 in str(kwargs)

또한 실패 케이스 테스트도 추가하는 것이 좋습니다:

@patch("scraping.main.sync_to_async", new_callable=MagicMock)
@pytest.mark.asyncio
async def test_update_daily_statistics_exception(self, mock_sync_to_async, scraper):
    """데일리 통계 업데이트 실패 테스트"""
    post_data = {"id": "post-123"}
    stats_data = {"data": {"getStats": {"total": 100}}, "likes": 5}

    mock_async_func = AsyncMock(side_effect=Exception("Database error"))
    mock_sync_to_async.return_value = mock_async_func

    # 구현에 따라 예외 처리 방식 결정
    try:
        await scraper.update_daily_statistics(post_data, stats_data)
        # 예외를 내부적으로 처리하는 경우
    except Exception:
        # 예외를 전파하는 경우
        pass

    mock_sync_to_async.assert_called()
    mock_async_func.assert_called_once()

Comment on lines +84 to +97
@patch("scraping.main.fetch_post_stats")
@pytest.mark.asyncio
async def test_fetch_post_stats_limited_success(self, mock_fetch, scraper):
"""fetch_post_stats 성공 테스트"""
mock_fetch.side_effect = [None, None, {"data": {"getStats": {"total": 100}}}]

result = await scraper.fetch_post_stats_limited(
"post-123", "token-1", "token-2"
)

assert result is not None
mock_fetch.assert_called()
assert mock_fetch.call_count == 3

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

fetch_post_stats_limited 테스트 개선 필요

현재 테스트는 세 번째 시도에서 성공하는 케이스만 다루고 있습니다. 최대 재시도 횟수 초과 시 동작, 파라미터 전달 검증 등이 누락되어 있습니다.

@patch("scraping.main.fetch_post_stats")
@pytest.mark.asyncio
async def test_fetch_post_stats_limited_success(self, mock_fetch, scraper):
    """fetch_post_stats 성공 테스트"""
    mock_fetch.side_effect = [None, None, {"data": {"getStats": {"total": 100}}}]

    result = await scraper.fetch_post_stats_limited(
        "post-123", "token-1", "token-2"
    )

    assert result is not None
    mock_fetch.assert_called()
    assert mock_fetch.call_count == 3
+   # 파라미터 검증
+   for call_args in mock_fetch.call_args_list:
+       args, kwargs = call_args
+       assert "post-123" in str(args) or "post-123" in str(kwargs)
+       assert "token-1" in str(args) or "token-1" in str(kwargs) or "token-2" in str(args) or "token-2" in str(kwargs)

또한 실패 케이스 테스트도 추가해야 합니다:

@patch("scraping.main.fetch_post_stats")
@pytest.mark.asyncio
async def test_fetch_post_stats_limited_max_retries(self, mock_fetch, scraper):
    """최대 재시도 횟수 초과 테스트"""
    # 항상 None 반환 (실패)
    mock_fetch.return_value = None

    result = await scraper.fetch_post_stats_limited(
        "post-123", "token-1", "token-2"
    )

    assert result is None
    # 실제 구현의 최대 재시도 횟수에 맞게 수정 필요
    assert mock_fetch.call_count >= 3  # 최소 기대 호출 횟수

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
scraping/tests/test_main.py (1)

148-160: 🛠️ Refactor suggestion

통계 업데이트 테스트 개선이 필요합니다.

현재 테스트는 sync_to_async가 호출되었는지만 확인하고 있으며, 함수의 내부 동작이나 결과에 대한 검증이 부족합니다. 이전 리뷰에서도 지적되었듯이 올바른 파라미터로 호출되었는지, 비동기 래핑 함수가 올바르게 실행되었는지 검증하는 부분이 추가되어야 합니다.

다음과 같이 개선할 수 있습니다:

@patch("scraping.main.sync_to_async", new_callable=MagicMock)
@pytest.mark.asyncio
async def test_update_daily_statistics_success(self, mock_sync_to_async, scraper):
    """데일리 통계 업데이트 또는 생성 성공 테스트"""
    post_data = {"id": "post-123"}
    stats_data = {"data": {"getStats": {"total": 100}}, "likes": 5}

-   mock_sync_to_async.return_value = AsyncMock()
+   mock_async_func = AsyncMock()
+   mock_sync_to_async.return_value = mock_async_func

    await scraper.update_daily_statistics(post_data, stats_data)

    mock_sync_to_async.assert_called()
+   # 비동기 함수가 호출되었는지 확인
+   mock_async_func.assert_called_once()
+   # 함수에 전달된 인자들 확인
+   # post_id와 view_count, like_count 값이 올바르게 전달되었는지 검증

또한 실패 케이스에 대한 테스트도 추가하는 것이 좋습니다.

🧹 Nitpick comments (2)
scraping/tests/test_main.py (2)

161-174: 게시물 통계 조회 테스트 개선이 필요합니다.

현재 테스트는 함수 호출 횟수와 결과 값만 확인하고 있습니다. 이전 리뷰 코멘트에서 지적된 것처럼 함수 호출 시 전달된 파라미터가 올바른지 확인하는 검증 로직이 추가되어야 합니다.

@patch("scraping.main.fetch_post_stats")
@pytest.mark.asyncio
async def test_fetch_post_stats_limited_success(self, mock_fetch, scraper):
    """fetch_post_stats 성공 테스트"""
    mock_fetch.side_effect = [None, None, {"data": {"getStats": {"total": 100}}}]

    result = await scraper.fetch_post_stats_limited(
        "post-123", "token-1", "token-2"
    )

    assert result is not None
    mock_fetch.assert_called()
    assert mock_fetch.call_count == 3
+   # 파라미터 검증
+   for call_args in mock_fetch.call_args_list:
+       args, kwargs = call_args
+       assert args[0] == "post-123"
+       assert args[1] == "token-1"
+       assert args[2] == "token-2"

1-241: 전반적인 테스트 커버리지 개선이 필요합니다.

PR 목표에서 언급된 83% 테스트 커버리지는 좋은 수치이지만, 몇 가지 추가적인 테스트로 더 강화할 수 있습니다:

  1. update_daily_statistics 메소드의 실패 케이스 테스트가 없습니다. 예를 들어, 유효하지 않은 통계 데이터가 전달될 때의 동작을 검증하는 테스트가 필요합니다.

  2. process_user 메소드에서 token이 None이거나 인증이 실패했을 때의 시나리오를 테스트하는 것이 좋습니다.

  3. PR 코멘트에서 언급된 것처럼 트랜잭션 처리와 롤백 메커니즘에 대한 테스트가 보강되어야 합니다. 현재의 테스트는 잘못된 import 경로로 인해 제대로 작동하지 않을 수 있습니다.

이러한 추가 테스트를 통해 코드의 견고성과 안정성을 더욱 향상시킬 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b2be1d and abd467c.

📒 Files selected for processing (2)
  • scraping/main.py (1 hunks)
  • scraping/tests/test_main.py (1 hunks)
🔇 Additional comments (11)
scraping/main.py (1)

40-41: 강화된 토큰 유효성 검증 추가!

토큰이 None인 경우를 명시적으로 확인하는 방어적 코딩을 추가해 메서드의 안정성을 향상시켰습니다. 복호화가 성공적으로 이루어지지 않았을 때 발생할 수 있는 문제를 사전에 방지합니다.

scraping/tests/test_main.py (10)

1-10: 테스트 코드의 필요한 import들이 올바르게 설정되었습니다.

테스트에 필요한 모듈들을 적절히 가져오고 있습니다. AsyncMock, MagicMock과 patch는 비동기 함수들을 테스트하는 데 중요합니다.


12-28: 테스트 설정 및 fixture 구현이 적절합니다.

pytest 픽스처를 사용하여 테스트 환경을 적절하게 설정했습니다. Scraper 인스턴스와 테스트용 User 객체를 생성하는 fixture는 코드 재사용성을 높여 각 테스트에서 반복적으로 인스턴스를 생성하지 않아도 됩니다.


30-52: 토큰 업데이트 성공 케이스 테스트가 잘 구현되었습니다.

성공 케이스에 대한 테스트가 적절하게 구현되었으며, mock 객체를 통해 필요한 의존성을 효과적으로 대체하고 있습니다. 또한 함수 호출 시 전달된 인자들이 올바른지 확인하는 assert 구문들도 적절히 포함되어 있습니다.


53-71: 토큰 변경 없음 케이스 테스트가 잘 구현되었습니다.

토큰이 변경되지 않는 경우에 대한 테스트가 적절하게 구현되었습니다. 기존 피드백에서 언급된 mock_asave.assert_not_called()를 사용하여 이러한 경우 asave 메소드가 호출되지 않는지 확인하는 것은 좋은 접근 방식입니다.


73-91: 토큰 만료 실패 케이스 테스트가 잘 구현되었습니다.

토큰이 만료된 경우의 실패 시나리오를 적절하게 테스트하고 있습니다. 이러한 실패 케이스 테스트는 코드의 안정성을 확보하는 데 중요합니다.


92-107: 복호화 실패 케이스 테스트는 메인 코드 변경사항을 검증합니다.

이 테스트는 main.py에 추가된 새로운 기능(복호화된 토큰이 None인 경우 처리)을 정확하게 검증하고 있습니다. 실제 프로덕션 환경에서 발생할 수 있는 중요한 에지 케이스를 테스트하고 있어 코드 변경의 신뢰성을 높입니다.


108-127: 게시물 삽입 성공 테스트가 적절합니다.

게시물 배치 삽입 성공 테스트는 bulk_create가 여러 번 호출되는지 확인하고 있습니다. 배치 크기와 데이터 크기에 따라 호출 횟수가 맞는지 검증하는 것은 좋은 접근 방식입니다.


128-146: 게시물 삽입 실패 테스트가 적절하게 구현되었습니다.

예외 상황에 대한 테스트를 통해 실패 시나리오를 적절하게 처리하는지 확인하고 있습니다. 이는 코드의 안정성을 위해 중요합니다.


175-187: 게시물 통계 조회 실패 테스트가 추가되었습니다.

실패 케이스에 대한 테스트가 적절하게 구현되었습니다. 이를 통해 실패 상황에서의 코드 동작을 검증할 수 있습니다.


188-212: 유저 처리 성공 테스트가 잘 구현되었습니다.

유저 데이터 처리 성공 케이스에 대한 테스트가 적절하게 구현되었습니다. 다양한 mock 객체와 패치를 통해 외부 의존성을 효과적으로 대체하고 있습니다.

Comment on lines 227 to 240
@pytest.mark.django_db(transaction=True)
async def test_process_user_partial_failure_rollback(scraper, user):
"""통계 업데이트 중 실패 시 롤백 확인 테스트"""
mock_session = AsyncMock()

with patch("myapp.scraper.fetch_post_stats_limited", side_effect=Exception("Faile to fetch post stats_limited failed")):
try:
async with transaction.atomic():
await scraper.process_user(user, mock_session)
except Exception:
pass

assert Post.objects.filter(user=user).exists()
assert not any(post.statistics for post in Post.objects.filter(user=user))
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

통계 업데이트 실패 롤백 테스트에 오류가 있습니다.

이 테스트에도 몇 가지 중요한 오류가 있습니다:

  1. self 파라미터가 누락되어 메소드 선언이 잘못되었습니다.
  2. patch 경로가 잘못되었습니다. myapp.scraper.fetch_post_stats_limited가 아닌 scraping.main.fetch_post_stats_limited이어야 합니다.
  3. 오타: "Faile" -> "Failed"

다음과 같이 수정해야 합니다:

@pytest.mark.django_db(transaction=True)
-async def test_process_user_partial_failure_rollback(scraper, user):
+async def test_process_user_partial_failure_rollback(self, scraper, user):
    """통계 업데이트 중 실패 시 롤백 확인 테스트"""
    mock_session = AsyncMock()

-   with patch("myapp.scraper.fetch_post_stats_limited", side_effect=Exception("Faile to fetch post stats_limited failed")):
+   with patch("scraping.main.fetch_post_stats_limited", side_effect=Exception("Failed to fetch post stats_limited")):
        try:
            async with transaction.atomic():
                await scraper.process_user(user, mock_session)
        except Exception:
            pass

    assert Post.objects.filter(user=user).exists()
    assert not any(post.statistics for post in Post.objects.filter(user=user))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@pytest.mark.django_db(transaction=True)
async def test_process_user_partial_failure_rollback(scraper, user):
"""통계 업데이트 중 실패 시 롤백 확인 테스트"""
mock_session = AsyncMock()
with patch("myapp.scraper.fetch_post_stats_limited", side_effect=Exception("Faile to fetch post stats_limited failed")):
try:
async with transaction.atomic():
await scraper.process_user(user, mock_session)
except Exception:
pass
assert Post.objects.filter(user=user).exists()
assert not any(post.statistics for post in Post.objects.filter(user=user))
@pytest.mark.django_db(transaction=True)
async def test_process_user_partial_failure_rollback(self, scraper, user):
"""통계 업데이트 중 실패 시 롤백 확인 테스트"""
mock_session = AsyncMock()
with patch("scraping.main.fetch_post_stats_limited", side_effect=Exception("Failed to fetch post stats_limited")):
try:
async with transaction.atomic():
await scraper.process_user(user, mock_session)
except Exception:
pass
assert Post.objects.filter(user=user).exists()
assert not any(post.statistics for post in Post.objects.filter(user=user))

Comment on lines 213 to 226
@pytest.mark.django_db(transaction=True)
async def test_process_user_failure_rollback(scraper, user):
"""유저 데이터 처리 실패 시 롤백 확인 테스트"""
mock_session = AsyncMock()

with patch("myapp.scraper.fetch_velog_user_chk", side_effect=Exception("Failed to fetch user data")):
try:
async with transaction.atomic():
await scraper.process_user(user, mock_session)
except Exception:
pass

assert not Post.objects.filter(user=user).exists()

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

유저 처리 실패 롤백 테스트에 오류가 있습니다.

이 테스트에는 몇 가지 중요한 오류가 있습니다:

  1. self 파라미터가 누락되어 메소드 선언이 잘못되었습니다.
  2. patch 경로가 잘못되었습니다. myapp.scraper.fetch_velog_user_chk가 아닌 scraping.main.fetch_velog_user_chk이어야 합니다.

다음과 같이 수정해야 합니다:

@pytest.mark.django_db(transaction=True)
-async def test_process_user_failure_rollback(scraper, user):
+async def test_process_user_failure_rollback(self, scraper, user):
    """유저 데이터 처리 실패 시 롤백 확인 테스트"""
    mock_session = AsyncMock()

-   with patch("myapp.scraper.fetch_velog_user_chk", side_effect=Exception("Failed to fetch user data")):
+   with patch("scraping.main.fetch_velog_user_chk", side_effect=Exception("Failed to fetch user data")):
        try:
            async with transaction.atomic():
                await scraper.process_user(user, mock_session)
        except Exception:
            pass

    assert not Post.objects.filter(user=user).exists()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@pytest.mark.django_db(transaction=True)
async def test_process_user_failure_rollback(scraper, user):
"""유저 데이터 처리 실패 시 롤백 확인 테스트"""
mock_session = AsyncMock()
with patch("myapp.scraper.fetch_velog_user_chk", side_effect=Exception("Failed to fetch user data")):
try:
async with transaction.atomic():
await scraper.process_user(user, mock_session)
except Exception:
pass
assert not Post.objects.filter(user=user).exists()
@pytest.mark.django_db(transaction=True)
async def test_process_user_failure_rollback(self, scraper, user):
"""유저 데이터 처리 실패 시 롤백 확인 테스트"""
mock_session = AsyncMock()
with patch("scraping.main.fetch_velog_user_chk", side_effect=Exception("Failed to fetch user data")):
try:
async with transaction.atomic():
await scraper.process_user(user, mock_session)
except Exception:
pass
assert not Post.objects.filter(user=user).exists()

Copy link

@coderabbitai coderabbitai bot left a 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 (5)
scraping/tests/test_main.py (5)

109-128: 배치 삽입 성공 테스트의 검증 로직이 개선될 수 있습니다.

배치 삽입 테스트가 기본적으로 잘 구현되었지만, 실제 호출 인자를 검증하지 않고 있습니다. 각 배치에 전달된 Post 객체가 올바른 값을 가지고 있는지 확인하는 검증을 추가하는 것이 좋겠습니다.

    @patch("scraping.main.Post.objects.bulk_create", new_callable=AsyncMock)
    @pytest.mark.asyncio
    async def test_bulk_insert_posts_success(self, mock_bulk_create, scraper, user):
        """Post 객체 배치 분할 삽입 성공 테스트"""
        posts_data = [
            {
                "id": f"post-{i}",
                "title": f"Title {i}",
                "url_slug": f"slug-{i}",
                "released_at": "2025-03-07",
            }
            for i in range(50)
        ]

        result = await scraper.bulk_insert_posts(user, posts_data, batch_size=10)

        assert result is True
        mock_bulk_create.assert_called()
        assert mock_bulk_create.call_count == 5
+        
+        # 각 배치에 전달된 객체 검증
+        for call_idx, call in enumerate(mock_bulk_create.call_args_list):
+            args, _ = call
+            batch_posts = args[0]
+            assert len(batch_posts) <= 10  # 배치 크기 확인
+            
+            # 첫 번째 배치의 첫 번째 Post 객체 데이터 검증
+            if call_idx == 0:
+                first_post = batch_posts[0]
+                assert first_post.post_id == "post-0"
+                assert first_post.title == "Title 0"
+                assert first_post.url_slug == "slug-0"

148-173: 데일리 통계 업데이트 성공 테스트가 개선되었습니다.

이전 리뷰에서 제안된 대로 테스트가 개선되었습니다. 특히 호출된 함수와 전달된 파라미터를 검증하는 로직이 추가되었습니다. 다만, 검증 방식이 다소 복잡합니다. 문자열 비교보다 더 직접적인 검증 방법을 고려해 볼 수 있습니다.

    @pytest.mark.asyncio
    async def test_update_daily_statistics_success(self, scraper):
        """데일리 통계 업데이트 또는 생성 성공 테스트"""
        post_data = {"id": "post-123"}
        stats_data = {"data": {"getStats": {"total": 100}}, "likes": 5}

        with patch(
            "scraping.main.sync_to_async", new_callable=MagicMock
        ) as mock_sync_to_async:
            mock_async_func = AsyncMock()
            mock_sync_to_async.return_value = mock_async_func

            await scraper.update_daily_statistics(post_data, stats_data)

            mock_sync_to_async.assert_called()
            mock_async_func.assert_called_once()

-            for call_args in mock_sync_to_async.call_args_list:
-                args, kwargs = call_args
-
-                assert callable(args[0])
-
-                if kwargs:
-                    assert "post-123" in str(kwargs.get("post_data", ""))
-                    assert 100 in str(kwargs.get("stats_data", ""))
+            # 더 직접적인 비교 방식 사용
+            args, kwargs = mock_sync_to_async.call_args
+            assert callable(args[0])
+            
+            # mock_async_func 호출시 전달된 인자 확인
+            call_args, call_kwargs = mock_async_func.call_args
+            if call_kwargs:
+                assert call_kwargs.get("post_data", {}).get("id") == "post-123"
+                assert call_kwargs.get("stats_data", {}).get("data", {}).get("getStats", {}).get("total") == 100
+                assert call_kwargs.get("stats_data", {}).get("likes") == 5

216-228: 최대 재시도 횟수 초과 테스트가 잘 구현되었습니다.

이전 리뷰 피드백대로 최대 재시도 후 실패할 경우에 대한 테스트가 추가되었습니다. 다만, assert mock_fetch.call_count >= 3은 다소 느슨한 검증입니다. 정확한 호출 횟수를 검증하는 것이 좋습니다.

    @patch("scraping.main.fetch_post_stats")
    @pytest.mark.asyncio
    async def test_fetch_post_stats_limited_max_retries(self, mock_fetch, scraper):
        """최대 재시도 횟수 초과 테스트"""
        mock_fetch.return_value = None

        result = await scraper.fetch_post_stats_limited(
            "post-123", "token-1", "token-2"
        )

        assert result is None
-       assert mock_fetch.call_count >= 3
+       # 정확한 호출 횟수 검증 (실제 구현에 맞게 조정)
+       assert mock_fetch.call_count == 3

242-266: process_user 성공 테스트의 검증이 제한적입니다.

현재 테스트는 update_old_tokens가 호출되었는지만 검증하고 있습니다. bulk_insert_posts나 다른 주요 함수들이 호출되었는지, 그리고 올바른 매개변수로 호출되었는지를 검증하는 부분이 없습니다.

    @patch("scraping.main.fetch_velog_user_chk")
    @patch("scraping.main.fetch_all_velog_posts")
    @patch("scraping.main.AESEncryption")
    @pytest.mark.asyncio
    async def test_process_user_success(
        self, mock_aes, mock_fetch_posts, mock_fetch_user_chk, scraper, user
    ):
        """유저 데이터 전체 처리 성공 테스트"""
        mock_encryption = mock_aes.return_value
        mock_encryption.decrypt.side_effect = lambda token: f"decrypted-{token}"
        mock_encryption.encrypt.side_effect = lambda token: f"encrypted-{token}"

        mock_fetch_user_chk.return_value = (
            {"access_token": "new-token"},
            {"data": {"currentUser": {"username": "testuser"}}},
        )
        mock_fetch_posts.return_value = []

+       with patch.object(
+           scraper, "bulk_insert_posts", new_callable=AsyncMock, return_value=True
+       ) as mock_bulk_insert, \
        with patch.object(
            scraper, "update_old_tokens", new_callable=AsyncMock
        ) as mock_update_tokens:
            await scraper.process_user(user, MagicMock())

        mock_update_tokens.assert_called_once()
+       mock_bulk_insert.assert_called_once()
+       mock_fetch_user_chk.assert_called_once()
+       mock_fetch_posts.assert_called_once()
+       
+       # 올바른 토큰 정보로 호출되었는지 확인
+       tokens_arg = mock_update_tokens.call_args[0][2]
+       assert tokens_arg == {"access_token": "new-token"}

1-304: 전반적인 테스트 코드 구현이 잘 되어 있습니다만, 일부 실패 테스트가 더 필요합니다.

전체적으로 테스트 코드가 잘 구현되어 있습니다. 하지만 PR 설명에서 언급된 대로 실패 케이스에 대한 테스트가 더 필요합니다. 특히 다음과 같은 실패 케이스에 대한 테스트를 추가하는 것이 좋겠습니다:

  1. 트랜잭션 중 다양한 지점에서 발생할 수 있는 예외 상황
  2. 입력 데이터가 유효하지 않은 경우
  3. 외부 API 응답이 예상과 다른 형식일 경우

또한, 테스트 커버리지 83%는 좋은 시작점이지만, 주요 기능에 대한 실패 케이스를 더 추가하여 커버리지를 향상시키는 것을 고려해보세요.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between abd467c and e958b9d.

📒 Files selected for processing (1)
  • scraping/tests/test_main.py (1 hunks)
🔇 Additional comments (10)
scraping/tests/test_main.py (10)

1-29: 클래스 설정 및 fixture 구현이 잘 되었습니다.

필요한 import 및 fixture를 잘 설정하셨습니다. Scraper 인스턴스와 테스트용 User 객체를 만드는 fixture가 명확하게 구현되어 있습니다.


30-52: 토큰 업데이트 성공 테스트가 잘 구현되었습니다.

토큰 업데이트에 대한 성공 케이스 테스트가 잘 작성되었습니다. 모의 객체의 side_effect 사용 및 assertion이 적절합니다. 이전 리뷰에서 제안한 대로 파라미터 검증(decrypt 함수 호출이 올바른 인자로 이루어졌는지)도 포함되어 있습니다.


53-71: 토큰 변경 없음 케이스 테스트가 잘 추가되었습니다.

이전 리뷰에서 제안한 대로 토큰이 변경되지 않는 경우에 대한 테스트가 적절히 구현되었습니다. mock_asave.assert_not_called()를 사용하여 변경이 없을 때 저장 함수가 호출되지 않는 것을 확인하는 점이 좋습니다.


72-107: 토큰 업데이트 실패 테스트 케이스가 포괄적입니다.

토큰 만료 및 복호화 실패에 대한 테스트 케이스가 잘 구현되었습니다. 복호화 실패 테스트에서 None 반환 시나리오를 테스트한 것이 좋습니다. 두 가지 실패 시나리오 모두 적절히 검증되었습니다.


129-147: 배치 삽입 실패 테스트가 적절히 구현되었습니다.

예외 발생 시 배치 삽입 실패가 올바르게 처리되는지 검증하는 테스트가 잘 구현되었습니다.


174-191: 데일리 통계 업데이트 실패 테스트가 잘 구현되었습니다.

이전 리뷰에서 제안된 대로 데일리 통계 업데이트 실패 테스트 케이스가 추가되었습니다. 예외 처리 및 호출 검증이 적절히 구현되었습니다.


192-215: post_stats 제한 조회 성공 테스트에 파라미터 검증이 추가되었습니다.

이전 리뷰 피드백대로 파라미터 검증 로직이 추가되었습니다. 호출된 함수가 올바른 인자로 호출되었는지 검증하고 있습니다. 다만, 문자열 비교(문자열로 변환 후) 방식은 다소 취약할 수 있으므로 더 명확한 검증 방법을 고려해볼 수 있습니다.


229-241: fetch_post_stats 실패 테스트가 적절히 구현되었습니다.

fetch_post_stats 함수가 실패할 경우에 대한 테스트가 잘 구현되었습니다. 정확한 호출 횟수와 반환값을 검증하고 있습니다.


267-286: process_user 실패 롤백 테스트가 개선되었습니다.

이전 리뷰에서 지적된 문제들이 수정되었습니다. 특히 self 파라미터가 추가되고, 패치 경로가 올바르게 수정되었습니다. 롤백이 제대로 수행되는지 검증하는 로직이 잘 구현되어 있습니다.


287-304: 부분적 실패 롤백 테스트가 개선되었습니다.

이전 리뷰에서 지적된 문제들이 수정되었습니다. 특히 self 파라미터가 추가되고, 패치 경로가 올바르게 수정되었습니다. 통계 업데이트 실패 시 롤백이 제대로 수행되는지 검증하는 로직이 잘 구현되어 있습니다.

Copy link
Member

@Nuung Nuung left a comment

Choose a reason for hiding this comment

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

재시도 초과는 다른 테스트로 빼두어서 해당 코드레빗 리뷰는 skip 해도 괜찮을 것 같습니다! 와 전체적으로 테스팅의 견고함이 엄청나졌는데요?!?!? 대박 고생많으셨어요.

동기식이었어도 어려웠을 테스트 코드 형태인데, 특히 async 라서 더 빡셋네요,, (python 에서는 async 가 흔한 경우가 아니라서)

@Jihyun3478
Copy link
Contributor Author

재시도 초과는 다른 테스트로 빼두어서 해당 코드레빗 리뷰는 skip 해도 괜찮을 것 같습니다! 와 전체적으로 테스팅의 견고함이 엄청나졌는데요?!?!? 대박 고생많으셨어요.

동기식이었어도 어려웠을 테스트 코드 형태인데, 특히 async 라서 더 빡셋네요,, (python 에서는 async 가 흔한 경우가 아니라서)

비동기에 대한 개념을 제대로 다뤄보는게 처음이라서 이해하는데 시간이 조금 필요했던 것 같습니다!
테스트코드 작성 덕분에 이해에 도움이 되었던 것 같아요💪

비동기는 앞으로도 계속해서 학습해 나가야 할 것 같아요!! 이대로 sqush and merge 진행하겠습니다:)

@Jihyun3478 Jihyun3478 merged commit aaff321 into main Mar 9, 2025
2 checks passed
@Jihyun3478 Jihyun3478 deleted the test/main-test branch March 9, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants