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

핵심기능 E2E 테스트 - 카테고리, 검색 #648

Closed
wants to merge 9 commits into from

Conversation

Hain-tain
Copy link
Contributor

⚡️ 관련 이슈

📍주요 변경 사항

1. 카테고리 관련 E2E 테스트 추가

  • 카테고리 편집 모달에서 새 카테고리를 추가 및 삭제할 수 있다.
  • 카테고리 편집 모달에서 카테고리명을 수정 및 삭제할 수 있다.
  • 카테고리는 최대 15글자까지만 입력할 수 있다.

2. 검색 관련 E2E 테스트 추가

  • 검색창에 테스트를 입력하면 테스트가 내용에 포함된 템플릿 목록을 확인할 수 있다.
  • 검색창에 ㅁㅅㅌㅇ를 입력할 경우 검색 결과가 없습니다가 나온다.

🎸기타

  • login 하는 로직이 반복되어, 해당 로직을 testUtils라는 파일 내에 loginToCodezap로 만들어 사용하였습니다. 마위가 templateActions에 만든 함수와 중복되는 로직이기 때문에 논의 후 하나로 통일해야 합니다.
  • 카테고리는 중복으로 생성할 수 없는데, 각 브라우저에서 병렬적으로 테스트가 이루어지기 때문에 생성하는 카테고리에 browserName 을 포함하도록 하였습니다.
  • 카테고리는 중복으로 생성할 수 없기 때문에 테스트 이후 무조건 삭제를 해야했습니다. 따라서 별도의 삭제 테스트를 만들지 않고 생성+ 삭제, 수정+삭제 테스트로 진행하였습니다.
  • 현재 검색 테스트는 '테스트' 라는 키워드를 검색 후 '테스트2'라는 템플릿이 있는지 확인합니다. 따라서 테스트 계정('ll' 계정)의 템플릿 수정 삭제 시 유의 부탁드립니다.

@Hain-tain Hain-tain added feature 기능 추가 FE 프론트엔드 labels Sep 15, 2024
@Hain-tain Hain-tain added this to the 5차 스프린트🍗 milestone Sep 15, 2024
@Hain-tain Hain-tain self-assigned this Sep 15, 2024
Copy link
Contributor

@Jaymyong66 Jaymyong66 left a comment

Choose a reason for hiding this comment

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

고생했습니다 헤인!
반복되는 로직을 utill로 분리하고 재사용하는 등 테스트 코드이지만 코드 품질을 신경쓰는 모습이 인상깊었어요.
몇가지 코멘트가 있어서 남겨봅니다.
추가로 merge 커밋을 고려해서 브랜치명도 좀 더 구체화하면 좋겠다는 생각이 드네요! (저한테도 하는 이야기... ㅋㅋ)

Comment on lines +59 to +61
for (const char of rawCategoryName) {
await page.keyboard.type(char);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 Locator.fill()로 입력하지 않고 page.keyboard.type으로 입력한 이유가 있나요? 다음 링크의 caution에서는 fill을 권장하는 것 같아서요!
https://playwright.dev/docs/api/class-keyboard#keyboard-type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Locator.fill()은 복사붙여넣기 처럼 동작하기 때문에 16자 이상의 글자를 fill로 넣으면 아무것도 입력되지않아 해당 테스트에서 실패하였습니다. 🤣

(16자 이상의 글자를 복사붙여넣기하면 아예 복사붙여넣기가 되지 않는데, 그 이유는 validate 함수에서 아예 16자 이상의 입력을 막고있기 때문인 것 같습니다.)

따라서 15자 이후에 계속 입력하여도 15자까지만 카테고리가 생성된다는 것을 테스트하기 위해 page.keyboard.type를 사용하였습니다!

Comment on lines 28 to 30
export const waitForSuccess = async ({ page, url }: WaitForSuccessProps) => {
await page.waitForResponse((response) => response.url().includes(url) && response.status() === 200);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 유틸함수로 만드는 것 좋네요!
한가지 반영하지 않아도 되는 제안) url이 처음에는 route에 쓰이는 url인줄 알고 잘못 썼었는데, apiUrl 같은 네이밍이나 유틸함수이니까 JsDocs로 타입에 대한 설명을 붙이면 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아주 좋은 제안입니다! apiUrl 로 변경하겠습니다~!!

Comment on lines 5 to 7
test.beforeEach(async ({ page }) => {
await loginToCodezap({ page, username: 'll', password: 'llll1111' });
});
Copy link
Contributor

Choose a reason for hiding this comment

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

한가지 고민, 이런 테스트용 아이디를 소스코드에 노출해도 괜찮을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호... 이 부분 env로 관리하는게 좋겠네요!!

const deleteCategory = async ({ page, newCategoryName }: Props) => {
await page.getByRole('button', { name: '카테고리 편집' }).click();

await page.getByText(newCategoryName).nth(1).hover();
Copy link
Contributor

Choose a reason for hiding this comment

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

요런 nth(1) 같은 경우엔 변수에 할당해서 명시적으로 무엇에 hover하는지 하면 좋겠네요!
ex) const deleteButton = page.getByText(newCategoryName).nth(1);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 feature 기능 추가
Projects
Status: Weekend Done
Development

Successfully merging this pull request may close these issues.

2 participants