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: FAQ 컴포넌트 테스트 코드 작성 #339

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

kimyouknow
Copy link
Member

@kimyouknow kimyouknow commented Jan 14, 2024

작업 내용

제곧내~

스크린샷

요런 컴포넌트

2024-01-14.4.50.37.mov

사용 방법

레퍼런스

#335

Copy link

vercel bot commented Jan 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
www-depromeet-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 28, 2024 3:25pm

Copy link

Bundle Sizes

Compared against 842c9ee

Route: No significant changes found

Dynamic import: None found.

Comment on lines 110 to 125
});
it('🟢 [지원자격]탭을 클릭하면 [지원자격]질문들 항목을 표시한다.', async () => {
const user = userEvent.setup();

render(<FAQ />, {
wrapper: Provider,
});

await user.click(screen.getByRole('tab', { name: '지원자격' }));

expect(screen.getByRole('tab', { name: '지원자격', selected: true })).toBeInTheDocument();
expect(screen.getByRole('tab', { name: '면접', selected: false })).toBeInTheDocument();
expect(screen.getByRole('tab', { name: '활동', selected: false })).toBeInTheDocument();

expect(screen.getByRole('list', { name: '지원자격' })).toBeInTheDocument();
});
Copy link
Member Author

@kimyouknow kimyouknow Jan 14, 2024

Choose a reason for hiding this comment

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

고민했던 부분~

  1. 면접 질문 항목들을( ex. Q 어떻게 진행되나요, A: 00으로 진행됩니다)와 같은 하드코딩된 값들을 검증해야할까??
  • 각 질문들은 구현 세부 사항인듯해서 질문 항목들을 감싼 컴포넌트가 렌더링 되었는지만 확인해주는 로직으로 구현했어요.
    • 예를 들어, 10개의 질문에 대한 변경 (10개 -> 9개 혹은 10개 중 한 질문에 대한 수정) 사항이 있을 때 이 테스트가 깨져야 할까요???
  1. tc 설명이랑 다른 로직을 검증하는 것처럼 보여서 고민...

Copy link

@minsoo-web minsoo-web 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 34 to 39
it('🟢 가장 위에 있는 질문은 답변이 보이는 상태로 렌더링한다.', () => {
render(<FAQ />, {
wrapper: Provider,
});

const questionListContainer = screen.getByRole('list', { name: '지원자격' });

Choose a reason for hiding this comment

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

가장 위에 있는 질문이 지원자격이 아니게 될 수 있으니, 세부 구현에 대한 테스트가 아닐지 우려됩니다!
description을 "지원자격은 답변이 보이는 상태로 렌더링 한다" 로 바꾸거나,
getByRole 부분에 name 옵션 설정을 빼고 첫 번째 요소를 가져오는 테스트로 바꾸는 건 어떨까요??

Copy link
Member Author

@kimyouknow kimyouknow Jan 28, 2024

Choose a reason for hiding this comment

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

헉 좋은리뷰 감사해요!

민수님 말대로 "지원자격"이라는 맥락이 description만 봤을 때는 세부구현으로 보이네요

list 항목을 다른 곳에서도 써서 faq-list-label라는 형태의 aria를 주입했는데 여전히 세부구현같긴하네요. 조금 더 고민해보겠습니당

feat: 세부구현~
refactor: 답변이 보이는 질문 리스트 셀렉터 수정

expect(screen.getByRole('tab', { name: '면접', selected: false })).toBeInTheDocument();
expect(screen.getByRole('tab', { name: '활동', selected: false })).toBeInTheDocument();
});
it('🟢 가장 위에 있는 질문은 답변이 보이는 상태로 렌더링한다.', () => {

Choose a reason for hiding this comment

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

🟢 은 혹시 어떤 의미인가요! 컨벤션으로 정해둔 건가요..?!

Copy link
Member Author

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 33
it('🟢 [지원자격]탭이 활성화된 상태로 렌더링한다.', () => {
render(<FAQ />, {
wrapper: Provider,
});

expect(screen.getByRole('tab', { name: '지원자격', selected: true })).toBeInTheDocument();
expect(screen.getByRole('tab', { name: '면접', selected: false })).toBeInTheDocument();
expect(screen.getByRole('tab', { name: '활동', selected: false })).toBeInTheDocument();
});

Choose a reason for hiding this comment

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

테스트와 description이 조금 다른 것 같아요!
지원자격 탭이 활성화된 상태로 렌더링하는 거라면 31, 32 라인이 필요가 없고,
지원자격 탭만 활성화 되는 테스트라면 description 이 수정되면 좋을 것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 44 to 55
it('🟢 가장 위에 있는 질문 외에 나머지 질문들은 답변이 보이지 않은 상태로 렌더링한다.', () => {
render(<FAQ />, {
wrapper: Provider,
});

const questionListContainer = screen.getByRole('list', { name: '지원자격' });
const [, ...restQuestionItems] = within(questionListContainer).getAllByRole('button');

restQuestionItems.forEach(x => {
expect(x).toHaveAttribute('aria-expanded', 'false');
});
});

Choose a reason for hiding this comment

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

25 라인의 테스트와 이 테스트는 어떤 점에서 다른 것인지 궁금합니다.
추가로, aria-expanded 는 html attribute 일 뿐이지, 보이는지 안 보이는지와는 다른 영역이 아닐까? 의견 남겨봅니다

Comment on lines 59 to 66
it('🟢 답변이 보이는 질문을 클릭하면 질문이 가려진다.', async () => {
const user = userEvent.setup();

render(<FAQ />, {
wrapper: Provider,
});

const questionListContainer = screen.getByRole('list', { name: '지원자격' });

Choose a reason for hiding this comment

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

이것두 조금 비슷한 결인데, 지원자격이라는 텍스트가 "지원 자격" 이라고만 바뀌어도 이 테스트 코드는 깨질 것 같아요,
"답변이 보이는 질문" 을 가져오는 get 부분을 다른 방법으로 수정해보면 어떨까요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

#339 (comment) <- 동일한 방식으로 해결해봤어요

@kimyouknow
Copy link
Member Author

refactor: 답변이 보이는 질문 리스트 셀렉터 수정
여기서 구현한 aria 속성 + #339 (comment) 여기서 남겨준 속성 관련 의견을 보니 aria 속성에 대한 이해가 아직 부족한 것 같네요~

스터디 전에 찾아보고 가겠습니당 좋은 리뷰 감사해요 👍 @minsoo-web

@kimyouknow kimyouknow merged commit 0a37947 into base/faq-refactor Feb 12, 2024
10 checks passed
@kimyouknow kimyouknow deleted the feat/faq-test branch February 12, 2024 12:45
kimyouknow added a commit that referenced this pull request Feb 18, 2024
* [chore]: vitest + @testing-library 세팅 (#336)

* chore: vscode 기본 세팅

- explicit - Triggers Code Actions when explicitly saved. Same as true.
- https://code.visualstudio.com/updates/v1_85#_code-actions-on-save-and-auto

* chore: vitest 관련 라이브러리 설치

* chore: vitest 세팅

* feat: vitest Provider

* feat: formatSingleDigit vitest  잘 돌아가나 확인용 tc

* feat: vitest + react 잘 돌아가나 FAQ 컴포넌트  tc 작성

* [refactor]: formatSingleDigit 선행 조건 강화 및 테스트 코드 수정 (#337)

* feat: 테스트 케이스 추가

* feat: formatSingleDigit 선행조건 강화

* refactor: 음수 및 정수가 아닌 숫자 에러처리

* fix: infinity 관련 tc 추가

* [chore]: FAQ 컴포넌트 test-case 작성 (#338)

chore: faq - tc

* feat: FAQ 컴포넌트 테스트 코드 작성 (#339)

* refactor: 컴포넌트 aria-label 추가 및 시맨틱 태그 부여

* feat: faq 컴포넌트 테스트 코드 작성

* chore: 스토리북 props

* feat: [지원자격] 탭 활성 description 수정 및 테스트 세분화

* refactor: 답변이 보이는 질문 리스트 셀렉터 수정

* feat: 세부구현~
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.

2 participants