-
Notifications
You must be signed in to change notification settings - Fork 4
✅ test: feed-crawler 유닛 테스트 작성 #489
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
이 PR은 feed-crawler 모듈의 핵심 비즈니스 로직에 대한 포괄적인 유닛 테스트를 추가합니다. 추상 클래스 패턴을 사용하는 코드의 템플릿 메서드 실행 흐름과 에러 처리를 중점적으로 검증하며, RSS/Atom 파서, 크롤러 워크플로우, AI 워커의 재시도 로직 등을 테스트합니다.
주요 변경사항:
- 고정된 날짜를 사용하여 시간 의존적인 테스트의 안정성을 개선
- URL 기반 조건부 fetch 모킹으로 순서 의존성 제거
- 추상 클래스의 템플릿 메서드 패턴과 에러 핸들링 검증에 집중
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| feed-crawler/test/unit/parser.spec.ts | 고정 날짜 사용으로 시간 의존성 제거, URL 기반 조건부 모킹으로 테스트 안정성 향상 |
| feed-crawler/test/unit/parser-util.spec.ts | ParserUtil의 썸네일 추출, URL 처리, HTML 엔티티 변환 로직에 대한 포괄적 테스트 추가 |
| feed-crawler/test/unit/feed-parser-manager.spec.ts | 파서 선택 로직, 에러 복원력, short-circuit 동작 검증 테스트 추가 |
| feed-crawler/test/unit/feed-crawler.spec.ts | 크롤링 워크플로우, 조기 종료 시나리오, 병렬 처리 동작 테스트 추가 |
| feed-crawler/test/unit/claude-event-worker.spec.ts | AI 워커의 재시도 로직, deathCount 경계값, API 응답 파싱, rate limiting 테스트 추가 |
| feed-crawler/test/unit/abstract-queue-worker.spec.ts | 템플릿 메서드 패턴의 실행 순서와 에러 처리 검증 테스트 추가 |
… into test/feed-crawler-unit
Jo-Minseok
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.
고생하셨습니다! 리뷰 확인 부탁드려요!
| const FIXED_DATE_UTC = FIXED_DATE.toUTCString(); | ||
| const FIXED_DATE_ISO = FIXED_DATE.toISOString(); | ||
|
|
||
| const RSS_20_SAMPLE = `<?xml version="1.0" encoding="UTF-8"?> |
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.
P5) XML 샘플 파일은 따로 빼는게 좋을듯해요! 테스트 코드 읽을 때 테스트 보다는 XML 길이에 더 초점이 가는듯하여 가독성 저하가 발생하는 것 같습니다!
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.clearAllMocks(); |
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.
P3) 제 생각에는 이 테스트 코드에서는 clearAllMocks 보다는 restoreAllMocks가 맞을듯 합니다! 성윤님께서 원래 fetch 구현을 복원하는 의도로 하신듯 합니다! clearAllMocks를 사용하면 덮어쓴 함수 기능들은 그대로 유지되기 때문에 restoreAllMocks가 맞을듯 해요!
| 함수 | 호출 기록 | mock 구현 | spy 복원 |
|---|---|---|---|
| clearAllMocks() | 초기화 O | 유지 | 유지 |
| restoreAllMocks() | 초기화 X | 복원 | 복원 |
| }, | ||
| ]; | ||
|
|
||
| beforeEach(() => { |
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.
P2) 여기에는 jest.clearAllMocks가 필요할듯 합니다!
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.clearAllMocks(); |
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.
P3) Spy를 원래 기능으로 복구하고자 하신다면 resetAllMocks 사용하셔야 합니다!
| import logger from '../../src/common/logger'; | ||
|
|
||
| // logger 모킹 | ||
| jest.mock('../../src/common/logger', () => ({ |
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.
P5) 저희 feed-crawler/src/common/logger.ts에 58번 라인
silent: process.env.NODE_ENV === 'test',사용중인데 로거를 모킹하신 이유가 궁금합니다
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.
// Then
expect(mockLogger.error).toHaveBeenCalledWith(
expect.stringContaining('처리 중 오류 발생'),
);
expect(mockLogger.info).toHaveBeenCalledWith(
expect.stringContaining('작업 완료'),
);공통 로직의 경우 진행완료 여부를 logger의 호출 여부로 검증하기위해 모킹을 진행했습니다!
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.
앗! 그런 부분이 있었네요 못봤습니다 죄송합니다 😢
Mocking을 하면 기존 기능을 해치는거라 실질적으로 호출을 확인하고자 한다면 Mocking보다는 spyOn으로 함수 감시를 하는건 어떨까요?
// Given
const infoSpy = jest.spyOn(logger, 'info');
const errorSpy = jest.spyOn(logger, 'error');
// When
...
// Then
expect(errorSpy).toHaveBeenCalledWith(
expect.stringContaining('처리 중 오류 발생'),
);
expect(infoSpy).toHaveBeenCalledWith(
expect.stringContaining('작업 완료'),
);이런식으로 Jest 에서 함수 감시가 가능합니다! 훨씬 코드도 간단해지는 것 같아요!
spyOn은 내부적으로 호출 정보를 기록하는거라 기능을 바꿔버리는 Mocking과는 달라서 검증을 하고 싶으시면 이 방법도 좋을듯 합니다!
jest.mock('../../src/common/logger', () => ({
default: {
info: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
debug: jest.fn(),
},
__esModule: true,
}));
const mockLogger = logger as jest.Mocked<typeof logger>;만약 spyOn으로 감시를 하게 된다면 모킹 코드는 지우셔도 동작합니다! 제 환경에서 테스트 통과하는 것을 확인했습니다.
CodeVac513
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.
수고많으셨습니다!
🔨 테스크
Issue
📋 작업 내용
추상클래스가 사용된 로직의 유닛 테스트를 어떻게 해야 할까?
추상 클래스를 활용하는 케이스는 현재 feed-crawler내에 크게 두가지가 존재합니다.
BaseFeedParser(이 경우는FeedParserManager를 테스틀하여 처리합니다.)AbstractQueueWorker이러한 패턴에서는 추상 클래스가 "공통적으로 어떤 순서로 무엇을 할지"를 정의하고, 구체 클래스는 그중 특정 구현체마다 달라진 작업을 "어떻게 할지"만 구현합니다. 이때 내부 구현 메서드들은
protected으로 캡슐화 되어 관리됩니다.이런식의 구성이 존재할 때, 저희는
processQueue()가 어떻게 동작하고 어떤 결과를 수행했는지는 관심이 없으며 테스트 대상에서 제외됩니다. (이 포스팅을 읽어보시면 좋습니다.)중점적으로 테스트 할 대상
근데
ClaudeEventWorker는 왜 테스트 하나요?ClaudeEventWorker는 AI 에이전트에게 별도의 호출이 필요한 케이스이기에 예외적으로 구현체를 테스트 하였습니다.그 외 핵심 로직 유닛 테스트 작성
feed-crawler 및 parser-util등 비즈니스 로직 실행에 필요한 로직들에 대한 테스트를 작성하였습니다.
요약
abstract-queue-worker.spec.tsparser.spec.tsfeed-parser-manager.spec.tsfeed-crawler.spec.tsclaude-event-worker.spec.ts