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

[view] AuthorBarChart util test 코드 작성 #746

Merged
merged 27 commits into from
Oct 8, 2024
Merged

Conversation

xxxjinn
Copy link
Contributor

@xxxjinn xxxjinn commented Oct 3, 2024

Related issue

Result

image

Work list

  • 이전에 AuthorBarChart의 util 중 일부 테스트 코드만을 작성했었는데, util 파일에 존재하는 모든 함수에 대해 테스트 코드를 구현했습니다.
  • 이전 util 테스트의 expect 값에 type을 선언해주었습니다.

  • sortDataByName: 각각 -1/1/0 return 하는 경우

  • convertNumberFormat

    • 0과 1 사이의 숫자 (0.5)
    • 1보다 큰 숫자 (1000)
  • sortDataByAuthor

    • author1, author2가 기여했다고 했을 때 author1이 작성한 커밋만 보여주는지
    • 존재하지 않는 저자(nonexistentAuthor)의 커밋을 찾는 것이므로 빈 배열 반환

Discussion

전부터 왜 자꾸 이전 커밋 기록이 남는건지,, 의문.....입니다 별 문제는 없었긴 한데

@xxxjinn xxxjinn self-assigned this Oct 3, 2024
@xxxjinn xxxjinn requested review from a team as code owners October 3, 2024 13:16
@@ -87,6 +88,145 @@ describe("getDataByAuthor", () => {
insertion: 3,
deletion: 2,
},
]);
] as AuthorDataType[]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

요부분이 type 추가했다고 말씀드린 부분입니다.

});
});

describe("sortDataByName", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sortDataByName 함수 테스트

});
});

describe("sortDataByAuthor", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sortDataByAuthor 함수 테스트

});
});

describe("convertNumberFormat", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

convertNumberFormat 함수 테스트

@Sang-minKIM
Copy link
Contributor

커밋 기록 남는거 혹시 fork하신 레포랑 githru:main이 있는 upstream이랑 싱크가 안맞아서 그런 건 아닐까요?

@xxxjinn
Copy link
Contributor Author

xxxjinn commented Oct 5, 2024

커밋 기록 남는거 혹시 fork하신 레포랑 githru:main이 있는 upstream이랑 싱크가 안맞아서 그런 건 아닐까요?

오..!. 그런걸까요?.... 항상 pull 받긴 했었는데 어느 순간부터 놓쳤었나봐요 ㅜㅅㅜ 감사합니다!!

@ytaek
Copy link
Contributor

ytaek commented Oct 6, 2024

커밋 기록 남는거 혹시 fork하신 레포랑 githru:main이 있는 upstream이랑 싱크가 안맞아서 그런 건 아닐까요?

오..!. 그런걸까요?.... 항상 pull 받긴 했었는데 어느 순간부터 놓쳤었나봐요 ㅜㅅㅜ 감사합니다!!

저는 origin에 새로운 브랜치로 만들어서 update하면서 해결하긴 하는데 (git switch -C 활용)
정리가 잘 안되면 기존 fork 날리고 다시 fork 해도 됩니다 ㅎㅎㅎ

Copy link
Contributor

@ytaek ytaek left a comment

Choose a reason for hiding this comment

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

LGGGGGGGGGGGGGTM!!!!

Comment on lines +114 to +120
const result = convertNumberFormat(0.5);
expect(result).toBe("0.5");
});

it("should format numbers greater than 1", () => {
const result = convertNumberFormat(1000);
expect(result).toBe("1k");
Copy link
Contributor

Choose a reason for hiding this comment

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

우왕 심플하지만 세밀한 테스트들 넘 좋습니다!! 👍👍👍👍👍

Comment on lines +174 to +195
{
nodeTypeName: "CLUSTER",
commitNodeList: [
{
nodeTypeName: "COMMIT",
commit: {
id: "1",
parentIds: ["0"],
author: { names: ["author1"] },
committer: { names: ["author1"] },
authorDate: "2024-01-01T00:00:00Z",
commitDate: "2024-01-01T00:00:00Z",
diffStatistics: {
insertions: 5,
deletions: 3,
},
message: "Initial commit",
},
seq: 1,
clusterId: 101,
},
],
Copy link
Contributor

Choose a reason for hiding this comment

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

아래 fakeData를 재활용해도 괜찮을 것 같습니다 : )

Copy link
Contributor

@lxxmnmn lxxmnmn left a comment

Choose a reason for hiding this comment

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

엄청 꼼꼼하게 작성해주셨네요!!!!!!
테스트 항목을 자세히 명시해주셔서 이해하기 더 쉬운 것 같아요
참고하겠습니다 🤩🤩👍

@xxxjinn xxxjinn merged commit ec87027 into githru:main Oct 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants