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

Feat/#353 유틸함수 테스트 보강 #356

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

ukkodeveloper
Copy link
Collaborator

📝작업 내용

전역 유틸함수 테스트

  • formatOrdinals 테스트

전역 custom hook 테스트

  • useFetch
  • useMutation
  • useDebounceEffect

💬리뷰 참고사항

useFetch와 useMutation은 실패했을 때와 성공했을 때를 나누어서 테스트하였습니다.
mocking이 필요한 테스트라 난이도가 있어서, 핵심적인 부분만 테스트했는데 더 필요한 부분이 있으면 커멘트 남겨주세요.

#️⃣연관된 이슈

close #353

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Unit Test Results

  71 files    71 suites   8s ⏱️
278 tests 278 ✔️ 0 💤 0
281 runs  281 ✔️ 0 💤 0

Results for commit 3031a54.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@Creative-Lee Creative-Lee left a comment

Choose a reason for hiding this comment

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

테스트 보충해 주신다고 고생 많으셨습니다!
코멘트 확인해 주시고 따로 답변주셔도 좋아요 ㅎㅎ
별개로 Approve는 바로 하겠습니다~ 반영해주시고 머지해주세요~ ㅎㅎ

expect(formatOrdinals(1)).toBe('1st');
});

test('숫자 2은 1nd를 반환한다.', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

테스트명에 오류가 있네요~! ㅎㅎ

expect(formatOrdinals(2)).toBe('2nd');
});

test('숫자 3은 1rd를 반환한다.', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도여~


jest.useFakeTimers();

describe('useDebounceEffect 테스트. debounce 제한 딜레이를 0.5초로 둘 경우', () => {
Copy link
Collaborator

@Creative-Lee Creative-Lee Sep 1, 2023

Choose a reason for hiding this comment

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

debounce 제한 딜레이를 0.5초로 둘 경우 이 부분은 아래 테스트 명으로 들어가야 될 것 같아요!

Copy link
Collaborator

@Creative-Lee Creative-Lee Sep 1, 2023

Choose a reason for hiding this comment

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

의도 따로 확인했습니다 ㅎㅎ

});
});

test('fetch가 실패했을 경우, isLoading 은 false, datas는 null, error은 mockError다.', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

*data
오타발견!


expect(result.current.isLoading).toBe(true);

waitFor(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 여기는 await가 안붙고 아래 실패케이스에는 붙는데 어떤 차이인가요?

Copy link
Collaborator

@cruelladevil cruelladevil left a comment

Choose a reason for hiding this comment

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

useDebounceEffect 테스트의 경우 의도대로 동작하지 않는 것 같은데,
당장 해결할 수 있는 문제가 아니라면 조금 더 고민 후에 반영해도 좋을 것 같습니다.
테스트가 가장 어려운 부분인데 고생많으셨습니다


describe('useDebounceEffect 테스트. debounce 제한 딜레이를 0.5초로 둘 경우', () => {
test('0.499초 지났을 때, 한 번도 실행되지 않는다.', () => {
const fn = jest.fn();
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 useFetch 테스트와 마찬가지로 beforeEach를 활용하면 좋을 것 같아요!


jest.useFakeTimers();

describe('useDebounceEffect 테스트. debounce 제한 딜레이를 0.5초로 둘 경우', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 아래 경우에도 통과하고 있는데, 테스트를 어떻게 짜야할지 고민해봐야 할 것 같아요

  test('0.499초 지났을 때, 한 번도 실행되지 않는다.', () => {
    const fn = jest.fn();
    const { rerender } = renderHook(({ deps, delay }) => useDebounceEffect(fn, deps, delay), {
      initialProps: { deps: 'dependency1', delay: 500 },
    });

    expect(fn).not.toBeCalled();

    rerender({ deps: 'dependency2', delay: 5000 });

    act(() => {
      jest.advanceTimersByTime(4000);
    });

    rerender({ deps: 'dependency4', delay: 5000 });

    // 0ms
    expect(fn).not.toBeCalled();

    act(() => {
      jest.advanceTimersByTime(1000);
    });

    // 1000ms
    // fn이 실행되지 않아야 하는데 실행됨
    expect(fn).not.toBeCalled();
  });

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.

[FEAT] util 함수 테스트를 보강한다.
3 participants