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

test: 테스트 한글화와 테스트 커버리지 개선 #81

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

gwansikk
Copy link
Contributor

@gwansikk gwansikk commented Apr 27, 2024

Overview

테스트 코드의 한글화와 테스트 커버리지 개선하는 작업을 포함하고 있습니다.

추가로 상세한 설명이 필요한 부분이나 같이 얘기해야 할 부분은 코멘트에 작성했어요. 💬

테스트 코드 한글화

현재 공식 문서가 국제화(i18n)가 되고 있는 것으로 확인이 되는 데 만약 테스트 코드를 한글로 할 경우, 한글을 모르는 유저(외국인 등)의 기여 참여를 저해하는 건지 고민해 볼 필요도 있을 거 같아요.

기존에는 테스트 코드 명이 한글 혹은 영어로 혼재되어 작성되어 있어요. 테스트 코드 작성에 통일성을 주고자 작업하게 되었습니다.
대부분 한글로 작성되어 있어 영문으로 된 테스트 명을 한글로 자연스럽게 작성했어요.

  • 문장일 경우 마침표(.)를 붙였어요.
  • 오타(맞춤법, 띄어쓰기)를 수정했어요.

테스트 커버리지 개선

테스트 커버리지 100%를 달성하기 위해 의미있게 필요한 케이스를 작성했어요.

getFirstConsonants에 대한 테스트 코드도 추가했습니다. #67 에서 초성을 뜻하는 단어를 통일하고자 변경되어 deprecated 상태가 되었는데, 곧바로 제거되지 않을 예정이라면 실제로 제거 전까지 해당 테스트를 유지해야 한다고 생각해요. (예상치 못한 부작용이나 오류를 방지하기 위함)

before after
image image

PR Checklist

  • I read and included theses actions below
  1. I have read the Contributing Guide
  2. I have written documents and tests, if needed.

Copy link

changeset-bot bot commented Apr 27, 2024

⚠️ No Changeset found

Latest commit: ea84c0f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Apr 27, 2024

@gwansikk is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

describe('hasValueInReadOnlyStringList', () => {
const testReadonlyList = ['ㄱ', 'ㄴ', 'ㄷ'] as const;

it('should return true if an element exists in a read-only string list', () => {
it('read-only 문자열 리스트에 요소가 존재한다면 true를 반환한다.', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

read-only는 명확한 의미를 전달하기 위해 영문으로 작성했어요.

Copy link
Member

Choose a reason for hiding this comment

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

동의합니다! 한국어로 바꿔서 어색할 부분은, 영어 그대로 남겨두는것이 좋은 것 같아요!

Copy link
Member

@okinawaa okinawaa left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@okinawaa okinawaa merged commit 36313f9 into toss:main Apr 29, 2024
1 check failed
@gwansikk gwansikk deleted the test branch April 29, 2024 05:37
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.

None yet

2 participants