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 : Implement progress page component #31

Merged
merged 5 commits into from
Oct 25, 2024

Conversation

SamTheKorean
Copy link
Contributor

No description provided.

@SamTheKorean SamTheKorean self-assigned this Oct 23, 2024
@SamTheKorean SamTheKorean requested a review from a team as a code owner October 23, 2024 01:21
@SamTheKorean SamTheKorean linked an issue Oct 23, 2024 that may be closed by this pull request
import { render, screen } from "@testing-library/react";
import Progress from "./progress"; // Adjust the import path as needed

describe("<Progress/>", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#18 (comment). 여기서 언급하신 것처럼 test를 it으로 변경할게요.

Copy link
Member

@sounmind sounmind left a comment

Choose a reason for hiding this comment

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

이 코멘트와 같은 맥락입니다. 고생하셨어요!!

Comment on lines 25 to 27
expect(screen.getByText(/Easy: 12\/12/i)).toBeInTheDocument();
expect(screen.getByText(/Med.: 22\/22/i)).toBeInTheDocument();
expect(screen.getByText(/Hard: 1\/1/i)).toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

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

정해진값을 정규식을 사용하여 테스트를 원하신다면 정해진 상수로 테스트하는 방법은 어떨까요?

Comment on lines 25 to 27
expect(screen.getByText(/Easy: 12\/12/i)).toBeInTheDocument();
expect(screen.getByText(/Med.: 22\/22/i)).toBeInTheDocument();
expect(screen.getByText(/Hard: 1\/1/i)).toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

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

만약 정규식으로 테스트를 하고싶다면 숫자부분 수정과 i태그를 수정요청드립니다

Suggested change
expect(screen.getByText(/Easy: 12\/12/i)).toBeInTheDocument();
expect(screen.getByText(/Med.: 22\/22/i)).toBeInTheDocument();
expect(screen.getByText(/Hard: 1\/1/i)).toBeInTheDocument();
expect(screen.getByText(/^Easy: \d{1,2}\/\d{1,2}$/)).toBeInTheDocument();
expect(screen.getByText(/^Med.: \d{1,2}\/\d{1,2}$/)).toBeInTheDocument();
expect(screen.getByText(/^Hard: \d{1,2}\/\d{1,2}$/)).toBeInTheDocument();
  • ^ $ 를통해서 정해진 format작성
  • 숫자부분은 총 문제가 75문제 이하니 2자리를 못넘기기에 2자리까지 제한
  • i태그는 정확한 string비교를 위해 제거 https://ko.javascript.info/regexp-introduction

@SamTheKorean 님은 어떤 방법이 테스트하기에 용이하다고 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

어떤 테스트를 할 지 고민했었는데 추천하신 것처럼 정규식에 제한을 두는 것 좋은 생각인 것 같습니다! 수정하겠습니다.

@SamTheKorean SamTheKorean force-pushed the sam/create-progress-page branch 3 times, most recently from 737f440 to c7d4054 Compare October 25, 2024 13:28
@SamTheKorean SamTheKorean force-pushed the sam/create-progress-page branch from c7d4054 to e6590c1 Compare October 25, 2024 13:31
@SamTheKorean SamTheKorean merged commit d4639a7 into main Oct 25, 2024
1 check passed
@SamTheKorean SamTheKorean deleted the sam/create-progress-page branch October 25, 2024 13:32
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.

프로그레스 페이지 구현 (CSS 빼고, Data 빼고)
3 participants