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 테스트 - 템플릿, 인증/인가 #647

Merged
merged 26 commits into from
Sep 23, 2024

Conversation

Jaymyong66
Copy link
Contributor

@Jaymyong66 Jaymyong66 commented Sep 15, 2024

⚡️ 관련 이슈

📍주요 변경 사항

  1. 생각보다 실제 DB와 함께 E2E 테스트를 진행하니 의존성이나 CI에서 문제가 많았습니다. 이 부분에 대해선 추가적인 조사와 논의가 필요해보입니다.
    예를 들어 템플릿을 생성한 후 마이페이지로 이동하여 템플릿 카드를 클릭해 상세 페이지로 간다면, API 요청이 오고 화면이 렌더링할 때 까지 기다리는 시간이 필요했습니다. mocking이나 proxy를 고려해볼 수 있겠습니다.

  2. 현재 테스트 하고 있는 스펙은 다음과 같습니다.

    템플릿 도메인

    • 템플릿 업로드 시, 파일명을 입력하지 않으면 파일명을 입력해주세요라는 토스트 메시지가 나온다.
    • 템플릿 제목, 설명, 파일명, 소스코드, 태그를 입력하고 저장버튼을 눌러 템플릿을 생성한다. 목록 페이지에서 새로 생성된 제목의 템플릿 카드를 확인할 수 있다.
    • 템플릿 카드를 누르면 템플릿 제목, 설명, 작성자, 생성날짜, 변경날짜, 카테고리, 코드 스니펫 목록을 확인할 수 있다.
    • test1 템플릿의 제목을 test2로 변경하고, test2태그를 추가로 등록한다.
    • 템플릿 삭제 버튼을 누르면 삭제 확인 모달이 뜨고, 삭제 확인 모달에서 삭제 버튼을 누르면, 템플릿이 삭제되고 내탬플릿 화면으로 이동한다.

    인증/인가 도메인

    • 코드잽 서비스에 들어가서 로그인 할 수 있다.
  3. 인증/인가 도메인의 회원가입의 경우, 실제 DB와 연결하여 테스트하고 있기 때문에 같은 아이디로 반복적으로 테스트가 불가하기 때문에 우선 제외하였습니다.

  4. templateActions.ts 파일은 자주 사용되는 로그인 로직과 템플릿 업로드 로직을 선언적으로 사용하도록 함수화한 것들입니다.
    -> templates.actions.ts, utils.ts로 변경되었습니다.

🎸기타

  1. 현재 src/playwright.config에는 baseURL: 'http://127.0.0.1:3000'로 작성되어있는데, 이는 CI를 github actions에서 돌리기 위해 설정한 값입니다. 로컬에서 직접 코드잽 서비스에서 해당 테스트 스크립트들을 실행시키기 위해선 아래처럼 바꾼 후 테스트를 실행시켜야합니다.
    더불어 다음 webServer 코드도 주석으로 제거하고 실행시켜 주세요.
use : {
  // ...
 baseURL: 'https://www.code-zap.com',
},
// webServer: {
//    command: 'npm run dev',
//    url: 'http://127.0.0.1:3000',
//    reuseExistingServer: !process.env.CI,
//  },
  1. 템플릿 수정을 테스트하기 위해 test1이라는 템플릿을 업로드 하는데, 이게 따로 테스트로 분리해서 템플릿을 업로드 하고, 해당 수정 테스트에서도 업로드 해주어야 수정이 되더라구요... 왜 그런지는 좀 더 찾아볼텐데, 우선 테스트가 통과하여 PR 올립니다.
    -> 응답을 wait 하는 것으로 처리하였습니다

  2. 보다 가독성 있고 정확한 테스트 코드를 위해선 프로덕션 코드에 testid 같은 data-attribute나 id, class 추가를 고려해봄직 합니다.

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

@Hain-tain Hain-tain left a comment

Choose a reason for hiding this comment

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

e2e 테스트 금방 할 수 있을 줄 알았는데, 생각보단 어렵더라고요..!🤣
연휴에 고생 많으셨습니다..👍

코멘트 몇 개 남겨두었으니 확인 부탁드립니다!!


+) 아래는 pr 본문에 대한 저의 생각들입니다.

  • 회원가입의 경우 테스트를 하기 위해서는 회원 탈퇴 기능이 있어야 할 것 같은데, 생각해보니 회원가입은 저희의 핵심기능이 아니니 추가하지 않아도 될 것 같습니다! (지금 테스트케이스로도 최소한의 QA 및 기능 확인은 가능하다고 생각합니다.)
  • 실제 DB와 함께 E2E 테스트를 계속 할지, 모킹을 할지에 대해서는 연휴 이후에 함께 논의해봅시다!

Comment on lines 3 to 14
export const loginToCodezap = async (page: Page, username: string, password: string) => {
await page.goto('/');
await page.getByRole('link', { name: '로그인', exact: true }).getByRole('button').click();
await page
.locator('div')
.filter({ hasText: /^아이디 \(닉네임\)$/ })
.locator('div')
.click();
await page.locator('input[type="text"]').fill(username);
await page.locator('input[type="text"]').press('Tab');
await page.locator('input[type="password"]').fill(password);
await page.locator('form').getByRole('button', { name: '로그인', exact: true }).click();
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 분리해주는거 너무 좋네요👍

저는 testUtils 라는 파일 내부에 분리해주었는데 혹시 그걸 사용하는 방향으로 합쳐보면 어떨까요?
(로그인은 모든 테스트에서 활용되기 때문입니다!)

Copy link
Contributor

Choose a reason for hiding this comment

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

(아주 작은 제안🌱) 이렇게 별도의 파일분리도 좋지만, 이 함수를 사용하는 파일 하위에 두어도 응집도 측면에서 괜찮을 것 같아요!
여러 파일에서 재사용된다면 파일 분리가 의미가 깊겠지만 지금 uploadTemplateToCodezaptemplates.spec.ts에서만 사용되고 있기 때문에 같은 파일 내에 있어도 좋을 것 같아요!

@@ -20,7 +20,7 @@ const config: Config = {
'react-syntax-highlighter/dist/esm': 'react-syntax-highlighter/dist/cjs',
},
transformIgnorePatterns: ['/node_modules/(?!react-syntax-highlighter)'],
testPathIgnorePatterns: ['/node_modules/', '/e2eTests/', '/tests-examples/'],
testPathIgnorePatterns: ['/e2eTests/'],
Copy link
Contributor

Choose a reason for hiding this comment

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

(질문🙋🏻‍♀️) 혹시 이렇게 e2eTests 폴더만 명시적으로 적용해주신 이유가 있으실까요..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

node_modules는 default 값이고, test-examples는 예시 테스트 코드라 지웠어요!
playwright로 작성한 e2e 테스트는 jest 명령어(스크립트로는 npm run test)를 실행 시 동작하지 않도록 했습니다.
단위테스트와 스크립트를 분리하는게 낫다고 판단하였고, 아직 CI에서 npm run test 시 e2e가 동작하면 안됩니닷.
스크린샷 2024-09-16 오전 3 42 51

});

// page redirect
await page.waitForTimeout(3000);
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 3초를 기다리는 이유는 아마 api 통신 이후 화면 전환이 되는 것을 기다려주시는 것 같아요!

  await page.waitForResponse((response) => response.url().includes(url) && response.status() === 200);

위와 같은 코드로 api 통신 결과를 기다릴 수 있고

  await 원하는요소.waitFor({ state: 'visible' });

위와 같은 코드로 원하는 요소가 보일때까지 기다릴 수 있다고 하더라고요!!ㅎㅎ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 아래 코드는 동작이 안되어서 어쩔 수 없이 해본건데, 위의 코드로 한번 해봐야겠네욥 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 안된다면 다음 소스코드로 테스트해봐도 좋을 것 같아요.
waitForLoadState
waitForResponse

// 네트워크 요정이 500ms 동안 유휴 될 때까지 기다린다.
await page.waitForLoadState({ waitUntil: 'networkidle' });

// 정확히 특정 요청이 완료될 때까지 대기
await page.waitForResponse('/api/code-zap/[endpoint]');

Copy link
Contributor

@vi-wolhwa vi-wolhwa left a comment

Choose a reason for hiding this comment

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

우와,, 추석 동안 playwright도 기능이 빵빵한 만큼 공부할게 많은 것 같아요!
추석동안 고생많으셨습니당
몇 가지 코멘트 남겼으니 확인 부탁드려요!

});

// page redirect
await page.waitForTimeout(3000);
Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 안된다면 다음 소스코드로 테스트해봐도 좋을 것 같아요.
waitForLoadState
waitForResponse

// 네트워크 요정이 500ms 동안 유휴 될 때까지 기다린다.
await page.waitForLoadState({ waitUntil: 'networkidle' });

// 정확히 특정 요청이 완료될 때까지 대기
await page.waitForResponse('/api/code-zap/[endpoint]');

Comment on lines 6 to 10
await page
.locator('div')
.filter({ hasText: /^아이디 \(닉네임\)$/ })
.locator('div')
.click();
Copy link
Contributor

Choose a reason for hiding this comment

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

다음과 같이 변경할 수 있을 것 같아용

await page.locator('text=아이디 (닉네임)').click();

Comment on lines 5 to 10
await page.getByRole('link', { name: '로그인', exact: true }).getByRole('button').click();
await page
.locator('div')
.filter({ hasText: /^아이디 \(닉네임\)$/ })
.locator('div')
.click();
Copy link
Contributor

Choose a reason for hiding this comment

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

(질문!) 두 동작 사이에 로그인 페이지로 리다이렉트가 완료될 때까지 기다리는 동작이 필요하지 않을까요?
(템플릿 생성 페이지로 리다이렉트 할 때도 동일합니다.)

await ...clic()이 리다이렉트를 기다려주는걸까요??

Copy link
Contributor

Choose a reason for hiding this comment

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

그리고, authentication.spec.ts 파일에서 loginToCodezap 함수를 사용하지 않는 이유가 있을까요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. playwright의 Locator는 기본적으로 auto-waiting이라 괜찮다고 알고 있어요! 제가 다른 곳에서 3000ms씩 기다려준 것은 지금 보니 api 응답을 기다리는 행위였네요. Locator, Auto-waiting

  2. 개인적으로는 authentication.spec.ts 파일의 로그인 테스트에 직접적인 동작들이라, utils에 있는 loginToCodezap을 같은 로직이어도 사용하진 않았긴 합니다. 다음 링크에서 보면 현재 모든 테스트의 beforeEach로 로그인 동작을 직접 하고 있는데, 링크와 같이 로그인을 setup 할 수 있겠더라구요. 그렇다면 setup하는 로그인 동작과 직접 로그인을 테스트하는 로직이 분리가 되어야할 것 같아서 일단 사용하지 않았습니다.

Comment on lines 14 to 15
await page.getByPlaceholder('제목을 입력해주세요').click();
await page.getByPlaceholder('제목을 입력해주세요').fill('asdf');
Copy link
Contributor

Choose a reason for hiding this comment

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

다른 모든 테스트 코드에 대한 공통 사항입니다!
fill() 함수는 click() 함수가 없더라도 동작하기 때문에 테스트 코드가 많아졌을 경우를 고려하면 click()를 제거하여 성능을 개선할 수 있을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

사용자 동작을 codegen으로 생성하다보니 click까지 포함되었는데, 성능을 생각하면 그렇겠네요. 사용자의 동작과 완전히 같게 할지 고민을 해봐야겠습니다

}

// 저장 버튼 클릭
await page.getByRole('button', { name: '저장' }).click();
Copy link
Contributor

Choose a reason for hiding this comment

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

저장 후 페이지가 정상적으로 리다이렉트 되는지까지 검사하면 좋을 것 같아요!

위에서 질문드렸던 내용인데, 만약 await로 리다이렉트를 기다릴 수 있다면 요렇게 해볼 수 있을 것 같네용

await expect(page).toHaveURL('/templates');

@Jaymyong66
Copy link
Contributor Author

waitForSuccess 관련해서 method도 인자로 받아 구분할까 고민을 하였지만, 우선은 201 status를 추가하는 것으로 마쳤습니다.
ex) /templates이지만 템플릿 목록을 GET 해오는 상황에서는 성공할 시 200 코드이고, 템플릿 업로드 POST일땐 201 코드입니다.

@Jaymyong66
Copy link
Contributor Author

헤인의 다음 PR - #672 를 반영하여 변경하였습니다.

Comment on lines 6 to 8
await page.locator('input[type="text"]').fill('ll');
await page.locator('input[type="text"]').press('Tab');
await page.locator('input[type="password"]').fill('llll1111');
Copy link
Contributor

Choose a reason for hiding this comment

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

env 로 변경 부탁드립니다~!!

PLAYWRIGHT_TEST_ID=
PLAYWRIGHT_TEST_PASSWORD=

Copy link
Contributor Author

Choose a reason for hiding this comment

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

참고) PLAYWRIGHT_TEST_ID -> PLAYWRIGHT_TEST_USERNAME 으로 변경했습니닷

Comment on lines 57 to 60
const title = page.getByText('테스트2').first();
const name = page.getByText('ll', { exact: true });
const editedDate = page.getByText('2024년 9월 12일');
const createdDate = page.getByText('(2024년 9월 11일)');
Copy link
Contributor

Choose a reason for hiding this comment

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

생각해보니 이 부분은 개발 서버(beta) 로 하면 변경되어야 할 것 같아요! (현재 개발 서버의 테스트용 계정에는 9월 11일에 작성된 테스트2 템플릿은 존재하지 않습니다.)


// 로그인 동작을 모든 테스트 전에 실행
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.

이 부분도 env로 변경 부탁드립니다~!!

Hain-tain
Hain-tain previously approved these changes Sep 20, 2024
Copy link
Contributor

@Hain-tain Hain-tain left a comment

Choose a reason for hiding this comment

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

고생많으셨습니다! 👍
특히 test.beforeEach 로 로그인을 매번 했어야 하는데, 이렇게 설정해두니 훨씬 편하고 요청을 덜 보내니까 테스트를 돌릴때 물리적 리소스도 줄어들어서 정말 좋은 것 같네요!!
이제 기본적인 기능에 대한 QA 는 걱정을 덜어도 되겠어요!ㅎㅎ

다만, 아래 부분에 대해서는 인지하고 넘어가면 좋을 것 같아서 적어둡니다!!

  1. 카테고리의 경우, 중복된 이름으로 생성이 되지 않기 때문에 매번 생성 후 삭제해주고 있는데, 테스트 실패로 카테고리가 삭제되지 않으면 다음 번에 테스트를 돌릴 때 무조건 실패합니다. 따라서 테스트가 실패하더라도 실행할 로직을 따로 정의해주어야 하는지, 아니면 매번 테스트 이전에 기존 카테고리 목록을 확인하고 삭제한 이후에 테스트를 하게 할 것인지 조치를 취해두면 좋을 것 같습니다.
  2. 현재 템플릿 생성 이후 삭제하는 로직이 없어서 테스트를 돌릴때마다 개발 서버에 템플릿이 생성되고있는데, deleteTemplate 을 해줄 것인지 아니면 수동 삭제하거나 그냥 계속 둘 것인지에 대해서도 고려해보면 좋을 것 같습니다.

Comment on lines +8 to +9
"e2e": "playwright test",
"e2e:ui": "playwright test --ui",
Copy link
Contributor

Choose a reason for hiding this comment

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

굿굿! 스크립트 추가하니까 훨씬 편하고 좋네요! 👍

@Jaymyong66
Copy link
Contributor Author

Jaymyong66 commented Sep 20, 2024

헤인의 코멘트를 반영하였습니다.

  • 테스트 실패 시에도 카테고리 혹은 템플릿이 생성된 것을 지우도록 try-catch-finally로 처리했습니다. catch문에서 throw를 하지 않으면 테스트가 실패해도 해당 테스트케이스가 결과적으로는 성공했다고 나오니 throw에 주의해주세요.

    try-catch를 쓰지 않고 afterEach도 고려해보았는데, 그럼 각 테스트케이스에서 사용하는 지워야할 친구의 title을 어떻게 가져오지 하다가 let 변수 말고는 일단 아이디어가 떠오르지 않았습니당. 좋은 아이디어 있으면 주세여~

let createdCategoryName = '';

test.afterEach(async ({ page }, testInfo) => {
  if (createdCategoryName && !testInfo.error) { // testInfo.error가 없으면 테스트가 성공한 상태입니다.
    await deleteCategory({ page, categoryName: createdCategoryName });
    await waitForSuccess({ page, apiUrl: '/categories' });

    const categoryButton = getCategoryButton({ page, categoryName: createdCategoryName });
    await expect(categoryButton).not.toBeVisible();
  }
});
  • 다음 스크린샷과 같이 템플릿 생성 테스트 후 테스트 템플릿을 삭제하는 과정에서 한번씩 실패하는 경우가 있었습니다. 원인을 생각해보다가 동시에 3가지 환경에서 실행하다보니 다른 환경에서의 템플릿이 not visible이 아닌 visible로 걸리는게 아닐까 하여 템플릿 title에 해당 브라우저 환경(browserName)을 추가했습니다.(ex. 템플릿생성테스트-chrome)
스크린샷 2024-09-21 오전 3 03 42 스크린샷 2024-09-21 오전 3 03 54

Copy link
Contributor

@Hain-tain Hain-tain left a comment

Choose a reason for hiding this comment

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

진짜 최고👍👍💯💯👏👏

Copy link
Contributor

@vi-wolhwa vi-wolhwa left a comment

Choose a reason for hiding this comment

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

진짜 완전 고생하셨습니다 LGTM🍀

@vi-wolhwa vi-wolhwa merged commit f64ad48 into dev/fe Sep 23, 2024
3 checks passed
@vi-wolhwa vi-wolhwa deleted the feat/636-e2e-init branch September 23, 2024 06:10
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.

3 participants