Skip to content
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

13-7 [BE] [논문 상세 - 논문 정보] Request batch test 코드 작성 #92

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

leesungbin
Copy link
Member

@leesungbin leesungbin commented Dec 6, 2022

개요

Request batch에 대한 test 코드 템플릿 작성

  • 현재 Backend 테스트 coverage
-------------------------------|---------|----------|---------|---------|-----------------------
File                           | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s     
-------------------------------|---------|----------|---------|---------|-----------------------
...
 backend/src                   |      30 |        0 |      25 |      25 |                       
  app.module.ts                |       0 |      100 |     100 |       0 | 1-8                   
  main.ts                      |       0 |      100 |       0 |       0 | 1-20                  
  util.ts                      |   54.54 |        0 |   28.57 |      50 | 3,14,24-33            
 backend/src/batch             |   67.36 |     37.5 |   69.38 |   67.23 |                       
  batch.config.ts              |     100 |       50 |     100 |     100 | 8                     
  batch.queue.ts               |   77.77 |       25 |      75 |      75 | 7,18                  
  batch.service.ts             |   63.63 |    42.85 |   42.85 |   63.33 | 27-40,45              
  batcher.abstract.ts          |   76.47 |    40.38 |    87.5 |   77.63 | 78-80,115-132,194-198 
  batcher.doi.ts               |   70.83 |        0 |   83.33 |   70.83 | 40-48                 
  batcher.search.ts            |   32.25 |    16.66 |      25 |   32.25 | 15,26-55              
 backend/src/batch/tests       |     100 |      100 |     100 |     100 |                       
  batcher.mock.ts              |     100 |      100 |     100 |     100 |                       
 backend/src/ranking           |   42.55 |        0 |   33.33 |   39.02 |                       
  ranking.controller.ts        |     100 |      100 |     100 |     100 |                       
  ranking.module.ts            |       0 |        0 |     100 |       0 | 1-23                  
  ranking.service.ts           |   30.76 |        0 |       0 |      25 | 9-27,33-38            
 backend/src/ranking/entities  |   83.33 |      100 |     100 |   83.33 |                       
  ranking.dto.ts               |       0 |      100 |     100 |       0 | 3                     
  ranking.entity.ts            |     100 |      100 |     100 |     100 |                       
 backend/src/ranking/tests     |   95.65 |       80 |     100 |     100 |                       
  ranking.service.mock.ts      |   95.45 |       80 |     100 |     100 | 25                    
  rankingData.mock.ts          |     100 |      100 |     100 |     100 |                       
 backend/src/search            |   74.78 |    60.78 |   80.76 |   76.76 |                       
  search.controller.ts         |   83.33 |    46.15 |   88.88 |   84.31 | 44,78-79,90,95-98     
  search.module.ts             |       0 |      100 |       0 |       0 | 1-37                  
  search.service.ts            |   85.71 |    65.78 |   81.25 |   89.18 | 61-65,102,127         
 backend/src/search/entities   |      85 |      100 |    37.5 |      85 |                       
  crossRef.entity.ts           |     100 |      100 |     100 |     100 |                       
  search.dto.ts                |   66.66 |      100 |       0 |   66.66 | 13,24-26,34-36,53     
 backend/src/search/tests      |    74.5 |       50 |   64.28 |      76 |                       
  crossref.mock.ts             |     100 |      100 |     100 |     100 |                       
  search.service.mock.ts       |   73.46 |       50 |   64.28 |      75 | 18-28,38,77-78,86     
  searchdata.mock.ts           |     100 |      100 |     100 |     100 |                       
-------------------------------|---------|----------|---------|---------|-----------------------

작업사항

  • src/batch/tests 에 테스트 코드 작성 (진행중)
  • 기존 테스트 코드 통과

리뷰 요청사항

Comment on lines +3 to +19
export function mockRedisQueue(popItem: () => string) {
const lpush = jest.fn().mockResolvedValue(true);
const rpush = jest.fn().mockResolvedValue(true);
const lpop = jest
.fn()
.mockImplementation((key: string, size: number) => Array.from({ length: size }, () => popItem()));
const llen = jest.fn().mockResolvedValue(10);
return {
lpush,
rpush,
lpop,
llen,
} as unknown as Redis;
}

export const popDoiItem = () => `0:0:-1:https://api.crossref.org/works/10.1234/asdf`;
export const popSearchItem = () => `0:0:-1:https://api.crossref.org/works?query=keyword&rows=1000&cursor=examplecursor`;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export function mockRedisQueue(popItem: () => string) {
const lpush = jest.fn().mockResolvedValue(true);
const rpush = jest.fn().mockResolvedValue(true);
const lpop = jest
.fn()
.mockImplementation((key: string, size: number) => Array.from({ length: size }, () => popItem()));
const llen = jest.fn().mockResolvedValue(10);
return {
lpush,
rpush,
lpop,
llen,
} as unknown as Redis;
}
export const popDoiItem = () => `0:0:-1:https://api.crossref.org/works/10.1234/asdf`;
export const popSearchItem = () => `0:0:-1:https://api.crossref.org/works?query=keyword&rows=1000&cursor=examplecursor`;
export function mockRedisQueue(item: string) {
const lpush = jest.fn().mockResolvedValue(true);
const rpush = jest.fn().mockResolvedValue(true);
const lpop = jest
.fn()
.mockImplementation((key: string, size: number) => Array.from({ length: size }, () => item);
const llen = jest.fn().mockResolvedValue(10);
return {
lpush,
rpush,
lpop,
llen,
} as unknown as Redis;
}
export const DOI_ITEM =`0:0:-1:https://api.crossref.org/works/10.1234/asdf`;
export const SEARCH_ITEM = `0:0:-1:https://api.crossref.org/works?query=keyword&rows=1000&cursor=examplecursor`;

함수로 선언하신 이유가 있나요? 상수로 사용해도 될 것 같아서요~
mockRedisQueue(popDoiItem)
개인적으로는 이 부분에서 redisQueue에 popDoiItem이라는 parameter가 좀 모호하게 느껴져서, mockRedisQueue(DOI_ITEM) 처럼 상수로 사용하는게 가독성이 더 좋은 것 같아요.

@@ -15,7 +15,7 @@ export class PaperInfo {
constructor(body: PaperInfo) {
this.title = body.title;
this.authors = body.authors;
this.doi = this.key = body.doi;
this.doi = this.key = body.doi.toLowerCase();
Copy link
Member

Choose a reason for hiding this comment

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

이부분 처리해주셨군요 👍
그럼 프론트쪽엔 LowerCase로 변환하는 로직이 있을필요는 없을 것 같은데.. 프론트로직에선 빼도 될까요? (지금 곳곳에 주렁주렁 달린 toLowerCase가 많아요 ..)

프론트에선 api 요청시 보내는 쿼리에 포함된 doi, 캐싱 키로 쓰이는 doi만 LowerCase로 변환해줘도 될 것 같아서요~

Copy link
Collaborator

@Palwol Palwol left a comment

Choose a reason for hiding this comment

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

좋네요👍
항상 테스트 꼼꼼하게 신경써주셔서 감사합니다.

@leesungbin leesungbin merged commit 8c83faf into dev Dec 8, 2022
@leesungbin leesungbin deleted the feature/batcher-test branch December 9, 2022 00:30
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