360 feature implement testing library and write fundamental UI tests #405
Conversation
…nt-testing-library-and-write-fundamental-ui-tests
📝 Walkthrough워크스루이 풀 리퀘스트는 프로젝트의 테스팅 인프라를 강화하기 위한 포괄적인 변경 사항을 포함하고 있습니다. Playwright와 Jest를 활용한 엔드투엔드 및 단위 테스트 설정이 추가되었으며, GitHub Actions 워크플로우를 통해 자동화된 테스트 프로세스를 구현했습니다. 또한, 사용자 인증, 공지사항 작성, 프로필 보기 등 다양한 시나리오를 테스트하는 새로운 테스트 케이스들이 도입되었습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User
participant WebApp
participant AuthService
participant NoticeService
User->>WebApp: 로그인 시도
WebApp->>AuthService: 인증 요청
AuthService-->>WebApp: 인증 성공
User->>WebApp: 공지사항 작성
WebApp->>NoticeService: 공지사항 저장
NoticeService-->>WebApp: 저장 완료
시 (토끼의 노래)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (11)
src/app/[lng]/(write)/write/NoticeEditor.tsx (4)
70-72: [성능 개선 제안]isEditable변수의 계산 로직 개선
isEditable변수가calculateRemainingTime함수를 사용하여 분과 초를 비교하고 있습니다. 하지만remain.minutes > 0 && remain.seconds > 0조건은 의도한 결과를 항상 보장하지 않을 수 있습니다. 대신 전체 남은 시간을 밀리초나 초 단위로 계산하여 비교하면 더 정확한 결과를 얻을 수 있습니다.
174-184: [성능 개선 제안] 로컬 스토리지 저장 시 디바운싱 적용
handleChange함수에서 매번 변경될 때마다 로컬 스토리지에 저장하고 있습니다. 이는 성능에 영향을 줄 수 있으므로 디바운싱을 적용하여 일정 시간 동안 입력이 없을 때만 저장하도록 개선할 수 있습니다.
227-234: [사용자 경험 개선] 알림 메시지 중복 제거
handleModify함수에서 제출 중임을 알리는 알림을 표시하고 있지만, 이전에 이미 로딩 상태를 나타내는 처리(예: 버튼 비활성화 등)가 있다면 중복될 수 있습니다. 불필요한 알림을 제거하여 사용자 경험을 개선할 수 있습니다.
516-531: [조건부 렌더링 최적화] 데드라인 토글 및 선택기 렌더링 로직 개선
hasDeadline상태에 따라DateTimePicker컴포넌트를 렌더링하고 있습니다. 이 부분을 조건부 렌더링 대신&&연산자를 사용하여 간결하게 표현할 수 있습니다.제안된 수정 사항:
<Toggle isSwitched={hasDeadline} onSwitch={(e) => { setHasDeadline(e.target.checked); sendLog(LogEvents.noticeWritingPageCheckDeadline, { hasDeadline: e.target.checked, }); }} /> - <div className={'w-1'} /> - - {hasDeadline && ( - <DateTimePicker dateTime={deadline} setDateTime={setDeadline} /> - )} + {hasDeadline && ( + <> + <div className={'w-1'} /> + <DateTimePicker dateTime={deadline} setDateTime={setDeadline} /> + </> + )}e2e/example.spec.ts (1)
17-17: [오타 수정] 주석 내용 수정 필요주석에서
'Expects page to have a heading with the name of Installation.'라는 문장이 있습니다.'...with the name of "Installation".'로 수정하면 더 명확합니다.제안된 수정 사항:
- // Expects page to have a heading with the name of Installation. + // 페이지에 "Installation"이라는 제목의 헤딩이 있는지 확인합니다.playwright.config.ts (1)
19-20: CI 환경의 재시도 전략을 개선해야 합니다현재 CI 환경에서 2회의 재시도만 설정되어 있습니다. 불안정한 UI 테스트의 특성상 더 많은 재시도가 필요할 수 있습니다.
다음과 같이 수정하세요:
- retries: process.env.CI ? 2 : 0, + retries: process.env.CI ? 3 : 0,jest.config.ts (2)
21-25: 코드 커버리지 수집을 활성화해야 합니다테스트 커버리지 측정이 비활성화되어 있습니다. 품질 관리를 위해 커버리지 수집을 활성화하는 것이 좋습니다.
다음과 같이 수정하세요:
- // collectCoverage: false, + collectCoverage: true, + collectCoverageFrom: [ + 'src/**/*.{js,jsx,ts,tsx}', + '!src/**/*.d.ts', + '!src/index.tsx', + ],
158-161: 테스트 파일 매칭 패턴을 구체화해야 합니다현재 테스트 파일 매칭 패턴이 주석 처리되어 있어 기본값을 사용합니다. 프로젝트의 구조에 맞게 구체적인 패턴을 지정하는 것이 좋습니다.
다음과 같이 수정하세요:
- // testMatch: [ - // "**/__tests__/**/*.[jt]s?(x)", - // "**/?(*.)+(spec|test).[tj]s?(x)" - // ], + testMatch: [ + "<rootDir>/src/**/__tests__/**/*.{js,jsx,ts,tsx}", + "<rootDir>/src/**/*.{spec,test}.{js,jsx,ts,tsx}" + ],.gitignore (1)
42-46: 중복된 Playwright 관련 항목 최적화 필요현재 설정된 Playwright 관련 항목들 중 일부가 중복되어 있습니다:
/test-results/와test-results/playwright-report/와playwright-reportplaywright/.auth와playwright다음과 같이 최적화하는 것을 추천드립니다:
/test-results/ /playwright-report/ /blob-report/ /playwright/.github/workflows/playwright.yml (1)
1-27: GitHub Actions 워크플로우 최적화 제안현재 워크플로우에 다음과 같은 최적화가 필요합니다:
- 의존성 캐싱 추가
- 불안정한 테스트를 위한 재시도 메커니즘
- 병렬 테스트 실행 구성
다음과 같이 개선하는 것을 추천드립니다:
name: Playwright Tests on: push: branches: [ main, master ] pull_request: branches: [ main, master ] jobs: test: timeout-minutes: 60 runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: node-version: lts/* + - name: Get yarn cache directory path + id: yarn-cache-dir-path + run: echo "dir=$(yarn config get cacheFolder)" >> $GITHUB_OUTPUT + - name: Cache yarn dependencies + uses: actions/cache@v4 + with: + path: ${{ steps.yarn-cache-dir-path.outputs.dir }} + key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} + restore-keys: | + ${{ runner.os }}-yarn- - name: Install dependencies run: npm install -g yarn && yarn - name: Install Playwright Browsers run: yarn playwright install --with-deps - name: Run Playwright tests - run: yarn playwright test + run: yarn playwright test --retries=2 --workers=2 - uses: actions/upload-artifact@v4 if: always() with: name: playwright-report path: playwright-report/ retention-days: 30package.json (1)
12-13: 테스트 실행 스크립트 구분 필요현재 Jest와 Playwright 테스트가 혼재되어 있어 혼란을 야기할 수 있습니다.
다음과 같이 스크립트를 구분하는 것을 추천드립니다:
- "test": "jest", - "test:watch": "jest --watch" + "test:unit": "jest", + "test:unit:watch": "jest --watch", + "test:e2e": "playwright test", + "test:e2e:ui": "playwright test --ui"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
.github/workflows/playwright.yml(1 hunks).gitignore(1 hunks)e2e/example.spec.ts(1 hunks)jest.config.ts(1 hunks)package.json(3 hunks)playwright.config.ts(1 hunks)src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/Content.tsx(1 hunks)src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/NoticeInfo.tsx(2 hunks)src/app/[lng]/(write)/write/NoticeEditor.tsx(1 hunks)src/app/components/shared/Toggle/toggle.module.css(0 hunks)src/app/components/shared/Zabo/Zabo.tsx(1 hunks)src/app/i18next/locales/en/translation.json(1 hunks)src/app/i18next/locales/ko/translation.json(1 hunks)tests/auth.setup.ts(1 hunks)tests/public.spec.ts(1 hunks)tests/user.spec.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/app/components/shared/Toggle/toggle.module.css
✅ Files skipped from review due to trivial changes (5)
- src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/Content.tsx
- src/app/i18next/locales/ko/translation.json
- src/app/i18next/locales/en/translation.json
- src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/NoticeInfo.tsx
- src/app/components/shared/Zabo/Zabo.tsx
🧰 Additional context used
📓 Learnings (1)
src/app/[lng]/(write)/write/NoticeEditor.tsx (1)
Learnt from: dohyun-ko
PR: gsainfoteam/ziggle-fe#388
File: src/app/[lng]/write/NoticeEditor.tsx:221-222
Timestamp: 2024-11-18T12:01:16.111Z
Learning: It is acceptable to include the entire `noticeToSubmit` object in the logs in `src/app/[lng]/write/NoticeEditor.tsx` when calling `sendLog`.
🪛 GitHub Actions: Playwright Tests
tests/auth.setup.ts
[error] 12-12: Connection refused when attempting to access http://localhost:3000/en. The local development server may not be running.
| } | ||
| } |
There was a problem hiding this comment.
[로딩 상태 초기화 누락] isLoading 상태 재설정 필요
handleModify 함수에서 !updatedNoticeId인 경우 실패 알림을 표시하지만 setIsLoading(false)를 호출하지 않고 있습니다. 로딩 상태를 재설정하여 사용자 인터페이스가 정확하게 반영되도록 해야 합니다.
제안된 수정 사항:
if (!updatedNoticeId) {
Swal.fire({
text: t('write.alerts.submitFail'),
icon: 'error',
confirmButtonText: t('alertResponse.confirm'),
});
+ setIsLoading(false);
}Committable suggestion skipped: line range outside the PR's diff.
| const checkLocalStorage = async () => { | ||
| if (localStorage.getItem(NOTICE_LOCAL_STORAGE_KEY)) { | ||
| const { koreanTitle, englishTitle, koreanBody, englishBody } = | ||
| JSON.parse(localStorage.getItem(NOTICE_LOCAL_STORAGE_KEY) ?? '{}'); | ||
|
|
||
| if (!koreanTitle && !englishTitle && !koreanBody && !englishBody) | ||
| return; | ||
|
|
||
| const confirm = await Swal.fire({ | ||
| text: t('write.hasSavedNotice'), | ||
| icon: 'question', | ||
| showCancelButton: true, | ||
| confirmButtonText: t('alertResponse.yes'), | ||
| cancelButtonText: t('alertResponse.no'), | ||
| }); | ||
|
|
||
| if (!confirm.isConfirmed) return; | ||
|
|
||
| setKoreanTitle(koreanTitle); | ||
| setEnglishTitle(englishTitle); | ||
| if (englishTitle) { | ||
| setHasEnglishContent(true); | ||
| } | ||
|
|
||
| if (koreanEditorRef.current) { | ||
| koreanEditorRef.current.setContent(koreanBody); | ||
| } | ||
|
|
||
| if (englishEditorRef.current) { | ||
| englishEditorRef.current.setContent(englishBody); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
[잠재적 무한 재귀 위험] 에디터 내용 설정 로직 개선 필요
trySetKrContent와 trySetEnContent 함수에서 에디터가 로드될 때까지 1초마다 재귀적으로 호출하고 있습니다. 이는 에디터가 로드되지 않을 경우 무한 재귀를 유발할 수 있습니다. 이 방식 대신 에디터의 onInit 콜백을 활용하여 에디터가 로드된 후 내용을 설정하는 것이 바람직합니다.
제안된 수정 사항:
- function trySetKrContent() {
- if (koreanEditorRef.current && notice?.content) {
- koreanEditorRef.current.setContent(notice.content);
- } else {
- setTimeout(trySetKrContent, 1000); // Retry after 1 second
- }
- }
- trySetKrContent();
+ <Editor
+ onInit={(evt, editor) => {
+ koreanEditorRef.current = editor;
+ if (notice?.content) {
+ editor.setContent(notice.content);
+ }
+ }}
+ ...otherProps
+ />Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| await page.click('#zabo'); | ||
|
|
||
| await expect(page).toHaveURL(/localhost:3000\/..\/notice\/\d*/); |
There was a problem hiding this comment.
[정규표현식 수정 필요] URL 매칭 패턴 수정
toHaveURL에서 사용하는 정규표현식에서 '../'는 상위 디렉토리를 의미하므로 '\/notice\/\d*'로 수정해야 합니다.
제안된 수정 사항:
- await expect(page).toHaveURL(/localhost:3000\/..\/notice\/\d*/);
+ await expect(page).toHaveURL(/localhost:3000\/notice\/\d+/);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await expect(page).toHaveURL(/localhost:3000\/..\/notice\/\d*/); | |
| await expect(page).toHaveURL(/localhost:3000\/notice\/\d+/); |
| import { expect, test } from '@playwright/test'; | ||
|
|
||
| test('view notice', async ({ page }) => { | ||
| await page.goto('localhost:3000/'); |
There was a problem hiding this comment.
[URL 오타 수정 필요] 주소에 오타가 있습니다
테스트 코드에서 페이지를 이동할 때 'localhost:3000/' 대신 'http://localhost:3000/'로 수정해야 합니다. 프로토콜이 누락되어 페이지 이동이 제대로 이루어지지 않을 수 있습니다.
제안된 수정 사항:
await page.goto('localhost:3000/');
+ // 'http://'를 추가합니다.
+ await page.goto('http://localhost:3000/');Committable suggestion skipped: line range outside the PR's diff.
| await page.getByPlaceholder('Email').fill('test@gm.gist.ac.kr'); | ||
| await page | ||
| .getByPlaceholder('Password') | ||
| .fill(process.env.TEST_IDP_PASSWORD || 'test'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 계정 정보를 환경 변수로 관리해야 합니다
이메일이 하드코딩되어 있어 보안상 위험할 수 있습니다. 비밀번호와 마찬가지로 환경 변수로 관리하는 것이 좋습니다.
다음과 같이 수정하세요:
- await page.getByPlaceholder('Email').fill('test@gm.gist.ac.kr');
+ await page.getByPlaceholder('Email').fill(process.env.TEST_IDP_EMAIL || 'test@gm.gist.ac.kr');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await page.getByPlaceholder('Email').fill('test@gm.gist.ac.kr'); | |
| await page | |
| .getByPlaceholder('Password') | |
| .fill(process.env.TEST_IDP_PASSWORD || 'test'); | |
| await page.getByPlaceholder('Email').fill(process.env.TEST_IDP_EMAIL || 'test@gm.gist.ac.kr'); | |
| await page | |
| .getByPlaceholder('Password') | |
| .fill(process.env.TEST_IDP_PASSWORD || 'test'); |
| test('write a notice', async ({ page }) => { | ||
| // await login(page); | ||
|
|
||
| await page.goto('localhost:3000/en'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 URL을 수정해야 합니다
auth.setup.ts와 마찬가지로, 하드코딩된 URL을 baseURL 설정을 활용하도록 수정해야 합니다.
다음과 같이 수정하세요:
- await page.goto('localhost:3000/en');
+ await page.goto('/en');Also applies to: 47-47
| await page | ||
| .getByPlaceholder('Enter a title here') | ||
| .fill('Test Notice Korean Title'); | ||
|
|
||
| await page.locator('#tinymce').fill('Test Notice Korean Content'); | ||
|
|
||
| await page.getByText('English').click(); | ||
|
|
||
| await page | ||
| .getByPlaceholder('Enter a title here') | ||
| .fill('Test Notice English Title'); | ||
|
|
||
| await page.locator('#tinymce').fill('Test Notice English Content'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
공지사항 작성 후 검증이 필요합니다
공지사항 작성 후 실제로 작성된 내용을 확인하는 검증 단계가 없습니다.
다음과 같은 검증 단계를 추가하세요:
await page.getByText('Confirm').click();
+
+ // 작성된 공지사항 확인
+ await expect(page).toHaveURL(/\/notice\/\d+/);
+ await expect(page.getByText('Test Notice Korean Title')).toBeVisible();
+ await expect(page.getByText('Test Notice English Title')).toBeVisible();Committable suggestion skipped: line range outside the PR's diff.
| // webServer: { | ||
| // command: 'npm run start', | ||
| // url: 'http://127.0.0.1:3000', | ||
| // reuseExistingServer: !process.env.CI, | ||
| // }, |
There was a problem hiding this comment.
CI 환경을 위한 웹 서버 설정이 필요합니다
현재 주석 처리된 webServer 설정으로 인해 CI 환경에서 테스트가 실패하고 있습니다.
다음과 같이 수정하세요:
- // webServer: {
- // command: 'npm run start',
- // url: 'http://127.0.0.1:3000',
- // reuseExistingServer: !process.env.CI,
- // },
+ webServer: {
+ command: 'npm run start',
+ url: 'http://127.0.0.1:3000',
+ reuseExistingServer: !process.env.CI,
+ timeout: 120 * 1000, // 웹 서버 시작을 위한 충분한 시간 제공
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // webServer: { | |
| // command: 'npm run start', | |
| // url: 'http://127.0.0.1:3000', | |
| // reuseExistingServer: !process.env.CI, | |
| // }, | |
| webServer: { | |
| command: 'npm run start', | |
| url: 'http://127.0.0.1:3000', | |
| reuseExistingServer: !process.env.CI, | |
| timeout: 120 * 1000, // 웹 서버 시작을 위한 충분한 시간 제공 | |
| }, |
| "@testing-library/dom": "^10.4.0", | ||
| "@testing-library/jest-dom": "^6.5.0", | ||
| "@testing-library/react": "^16.0.1", |
There was a problem hiding this comment.
테스팅 라이브러리 버전 불일치 문제
현재 설정된 테스팅 라이브러리 버전이 React 18과 호환되지 않을 수 있습니다:
- @testing-library/react v16.0.1은 React 18에 비해 매우 오래된 버전입니다
- @testing-library/dom v10.4.0도 최신 버전이 아닙니다
다음과 같이 버전을 업데이트하는 것을 강력히 권장드립니다:
- "@testing-library/dom": "^10.4.0",
- "@testing-library/jest-dom": "^6.5.0",
- "@testing-library/react": "^16.0.1",
+ "@testing-library/dom": "^9.3.4",
+ "@testing-library/jest-dom": "^6.5.0",
+ "@testing-library/react": "^14.2.1",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@testing-library/dom": "^10.4.0", | |
| "@testing-library/jest-dom": "^6.5.0", | |
| "@testing-library/react": "^16.0.1", | |
| "@testing-library/dom": "^9.3.4", | |
| "@testing-library/jest-dom": "^6.5.0", | |
| "@testing-library/react": "^14.2.1", |
| "node-fetch": "2", | ||
| "postcss": "^8", | ||
| "prettier": "^3.1.0", | ||
| "prettier-plugin-tailwindcss": "^0.5.7", | ||
| "prop-types": "^15.8.1", | ||
| "react-cookie": "^6.1.1", | ||
| "rxjs": "^7.8.1", |
There was a problem hiding this comment.
💡 Codebase verification
불필요한 의존성 제거 권장
검증 결과 다음 의존성들은 프로젝트에서 실제로 사용되지 않고 있어 제거해도 안전합니다:
rxjs: 프로젝트 코드에서 사용되지 않음 (TinyMCE 벤더 파일의 참조는 제외)ts-node: npm 스크립트나 Jest 설정에서 사용되지 않음
🔗 Analysis chain
불필요한 의존성 검토 필요
다음 의존성들이 현재 프로젝트에서 실제로 사용되는지 검토가 필요합니다:
rxjs: Playwright나 Jest 테스트에 필수적이지 않습니다ts-node: Jest 설정에만 필요하다면ts-jest로 대체 가능합니다
다음 스크립트로 해당 패키지들의 사용 여부를 확인해보세요:
Also applies to: 92-92
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# rxjs 사용 여부 확인
echo "Checking rxjs usage:"
rg -l "from 'rxjs'" || rg -l "from \"rxjs\""
# ts-node 사용 여부 확인
echo "Checking ts-node usage:"
rg -l "ts-node" --type yaml --type json
Length of output: 228
Script:
#!/bin/bash
# Additional rxjs usage patterns
echo "Checking additional rxjs patterns:"
rg -l "require.*rxjs" || rg -l "Observable|Subject|BehaviorSubject|ReplaySubject"
# Check Jest config files
echo "Checking Jest configuration:"
fd -e config.js -e config.ts -e config.json . | grep -i jest
cat jest.config.js 2>/dev/null || cat jest.config.ts 2>/dev/null || cat jest.config.json 2>/dev/null
# Check for ts-node in npm scripts
echo "Checking ts-node in npm scripts:"
jq '.scripts' package.json
Length of output: 7295
|
Too many conflicts with the master branch. This will be handled in a new branch. |
|
close #360 |
Playwright 를 이용한 테스팅을 추가했습니다
Summary by CodeRabbit
새로운 기능
의존성 업데이트
테스트 구성
사용자 인터페이스 개선
현지화