[vscode/engine] getGitLog 함수 관련 테스트 코드 구현 #689
Conversation
Desc - mocking 함수 작성하여 간단한 테스트 코드 구현 - 실제 데이터 처리를 테스트 하기 위해서 mocking 함수 보단 실제 child_process를 사용하는 것이 낫다고 판단하여 로직 수정할 예정
Desc: - 커밋 개수 고려하여 테스트용 git 데이터 추가 (nginx, 커밋 개수 약 8,000개) - git 충돌을 고려하여 압축 파일로 관리
Desc - 지정한 ssh 를 활용하여 clone 받은 후 테스트에 활용하도록 로직 수정 - 테스트 수행 후 clone 받은 파일 제거
Desc - 테스트 실행 시 설정해놓은 test repo에서 clone 받은 뒤, 해당 git 데이터 활용하여 테스트 진행 - test repo로 django 설정 (commit 개수 30,000개 이상으로 성능 테스트에 적합하다고 판단) - 테스트 실행 전/후로 clone 받은 디렉토리 제거하여 패키지에 남지 않도록 설정
feat: getGitLog 관련 테스트 코드 구현
Desc - ts-jest 관련 버전 수정
| @@ -5,7 +5,7 @@ | |||
| "outDir": "dist", | |||
| "lib": ["ES2020"], | |||
| "sourceMap": true, | |||
| "rootDir": "src", | |||
| "rootDir": "./", | |||
There was a problem hiding this comment.
확인이 늦어서 죄송합니다ㅠㅠ
기존에는 .ts 확장자를 가진 파일이 src 디렉토리 내부에만 존재했는데,
jest를 적용하면서 tsconfig.json과 같은 경로에 jest.config.ts 파일이 생성되어서
타입스크립트 관련 config 적용 경로를 변경하게 되었습니다!
There was a problem hiding this comment.
앗. 제가 잘 이해가 가지 않아서 다시 여쭤봅니다~.
engine의 경우에는 rootDir을 기존처럼 src로 둬도 잘 수행됐던거 같은데,
현재는 ./로 수정되지 않으면 안 돌아가는 상황인가요?
https://github.com/githru/githru-vscode-ext/blob/main/packages/analysis-engine/jest.config.ts
https://github.com/githru/githru-vscode-ext/blob/main/packages/analysis-engine/tsconfig.json
There was a problem hiding this comment.
저도 기존에 설정되어있던 경로를 수정하는 부분은 최대한 지양하는 것이 좋을 것 같아서
engine 패키지를 참고했었는데,
왜 그런지는 저도 정확히 모르겠지만
vscode 패키지에서 tsconfig.json의 rootdir을 engine과 동일하게 수정할 경우 관련 알림이 발생하는 상황입니다
(engine 패키지에서는 발생하지 않습니다)
engine과 동일하게 rootdir을 ./src로 적용해도 돌아가기는 하는 상황입니다!�
| import * as fs from "fs"; | ||
| import * as path from "path"; | ||
|
|
||
| import { getGitLog } from "../../utils/git.util"; |
There was a problem hiding this comment.
앗. 이 부분은 바로 상대경로를 쓰는게 아니라, engine pkg lib를 import 해서 쓰셔야 해요!
There was a problem hiding this comment.
영택님 코멘트 주셨던 부분 중 아래의 2가지는 수정하여 반영하였는데
- mock 데이터 활용 방식으로 test 코드 수정
- jest.config.ts 파일 수정 (test가 특정 디렉토리에 한정되지 않도록 전체 경로로 변경)
해당 이슈 (engine 모듈 import 수정) 는 어떻게 해결해야할지 잘 모르습니다..!
getGitLog 함수를 "@githru-vscode-ext/analysis-engine"에서 import 하려고 하니,
해당 패키지에서 export 하지 않는다고 나오는 상황입니다!
getGitLog 함수는 vscode/src/utils/git.util.ts 파일에서 선언된 함수라서 상대 경로로 import 하는 게 맞지 않나 생각이 되는데
(https://github.com/githru/githru-vscode-ext/blob/main/packages/vscode/src/utils/git.util.ts)
혹시 제가 잘못 이해하고 있는 거라면 어떻게 수정하는게 좋을지 코멘트 부탁드립니다..!
|
|
||
| import { getGitLog } from "../../utils/git.util"; | ||
|
|
||
| const TEST_REPO_SSH = "git@github.com:django/django.git"; // Sample repository: Django (https://github.com/django/django), with over 30,000 commits |
There was a problem hiding this comment.
test에 쓰이는 데이터는 동적인 데이터 말고, 정적인 데이터를 박아놓는게 좋습니다. (그럴일이 잘 없겠지만) 장고 repo가 이름이 바뀌거나 없어질 수도 있구요.
There was a problem hiding this comment.
앗 그런데 해당 링크가 안열려서 (저만 그런 걸지도 모르겠지만..?)
혹시 어떤 내용일까요..?!
| const cloneTestRepo = () => { | ||
| cp.execSync(`git clone ${TEST_REPO_SSH} ${testRepoPath}`, { stdio: "inherit" }); | ||
| }; | ||
|
|
||
| const removeFilesExceptGit = () => { | ||
| if (fs.existsSync(testRepoPath)) { | ||
| fs.readdirSync(testRepoPath).forEach((file) => { | ||
| const fullPath = path.join(testRepoPath, file); | ||
| if (file !== ".git") { | ||
| fs.rmSync(fullPath, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
(위와 같은 이유로) 매번 pull 해서 사용하는 로직이 테스트에서 실행되는데,
그냥 mock이나 fake data 를 정적으로 만들어놓는 것이 더 좋은 practice 입니다 😄
이런 경우, 만약에 network이 안되거나 git에 이상이 있을 경우에는,
현재 저희 코드와는 아무 관계없는 이유 때문에 test가 fail하게 되버리죠.
|
|
||
| describe("getGitLog util test", () => { | ||
| it("return git log output", async () => { | ||
| console.time("getGitLog Execution Time"); |
There was a problem hiding this comment.
오 .time() 이란게 있었는지 몰랐군요 : ) 감사합니다.
단, test에서 현재 validation을 하는 코드가 아니라면,
불필요한 코드는 사실 삭제하는 것이 맞긴 합니다 : )
(만약에 해당 test가 10s 안에 실행되어야 한다 등의 테스트 시나리오가 있다면 활용하는것이 맞지요)
| expect(result).toContain("commit"); | ||
| expect(result).toContain("Author"); | ||
| expect(result).toContain("AuthorDate"); | ||
| expect(result).toContain("Commit"); | ||
| expect(result).toContain("CommitDate"); |
There was a problem hiding this comment.
테스트 코드는 언제나 환영입니다. :)
테스트 코드의 목적이 해당 expect에 적힌 정도를 테스트 하고자 함이었다면
적당히 Mock 객체를 만들어서 테스트 하는 것도 괜찮을 것 같습니다.
There was a problem hiding this comment.
리뷰 주신 부분 수정해서 반영하도록 하겠습니다!
추가적으로 한가지 더 문의 드릴 부분이 있는데,
현재 CI 에러가 나는 원인이 vscode 패캐지의 package.json 스크립트에서 test 명령어를 찾을 수 없기 때문인 것 같은데
(아래에 에러 메세지 첨부)
- npm error location /home/runner/work/githru-vscode-ext/githru-vscode-ext/packages/vscode
npm error Missing script: "test"
discussion에 말씀드린 것처럼,
기존에 test 스크립트가 존재하긴 했지만 관련 코드가 작동하지 않는 상황인데 (주석처리 되어있음)
기존에 존재하던 코드/파일을 제거 하기에는 좀 애매해서
기존 test 스크립트를 test:e2e로 수정하고,
jest를 적용한 스크립트를 test:unit으로 새롭게 추가한 상황입니다.
CI에러를 해결하기 위해서는 test 스크립트를 다시 지정해줘야 할 것 같은데,
test:e2e로 지정한 스크립트를 제거하고, test:unit으로 생성한 스크립트를 test로 수정하는 게 좋을까요?
There was a problem hiding this comment.
CI에러를 해결하기 위해서는 test 스크립트를 다시 지정해줘야 할 것 같은데, test:e2e로 지정한 스크립트를 제거하고, test:unit으로 생성한 스크립트를 test로 수정하는 게 좋을까요?
넵 그렇게 하시면 되겠습니다.
packages/vscode/jest.config.ts
Outdated
| preset: "ts-jest", | ||
| testEnvironment: "node", | ||
| verbose: true, | ||
| rootDir: "./src/test/utils", |
There was a problem hiding this comment.
test/util 만 test하는게 아니라, 전체 or src폴더를 지정해야 하는게 아닐까요?
There was a problem hiding this comment.
해당 코드 관련해서는 기존에 vscode 패키지에 존재하던 test 관련 파일을 고려한 것인데
(vscode/src/test/suite/extension.test.ts)
https://github.com/githru/githru-vscode-ext/blob/main/packages/vscode/src/test/suite/extension.test.ts
해당 파일이 이전에 생성된 이후 현재는 활용되지 않는 파일로 생각되어
일단 금번 PR에 생성한 파일만 test에 적용되도록 test/util로 범위를 한정한 상황입니다
테스트 경로를 전체로 수정해주는 것이 좋을까요?
There was a problem hiding this comment.
테스트 경로를 전체로 수정해주는 것이 좋을까요?
넵. 곧 다른 테스트가 늘어날꺼니까요!!
어차피 폴더 안을 다 뒤져서 있는 tc 를 다 실행하기 때문에,
보통은(특별한 이유가 없는 이상) 저렇게 rootDir을 특정 폴더로 지정하지 않습니다 :-)
이 설정 파일이 vscode 패키지 전체에 대한 설정임을 고려해서 생각해보시면 좋을 것 같습니다 😸
SW란게 항상 수정이 덜 되는 방향으로 움직이는 거라고 생각한다면,
좀 오버하자면 OCP에 해당하는 케이스라고 보면 되겠네요 : )
https://ko.wikipedia.org/wiki/%EA%B0%9C%EB%B0%A9-%ED%8F%90%EC%87%84_%EC%9B%90%EC%B9%99
Desc - 테스트 적용 범위 수정 - 기존 : src/util - 수정 : 전체 경로
Desc - test 관련 스크립트 수정 (test:e2e, test:unit 스크립트 제거 후 test 스크립트 설정)
Desc - mock 데이터 및 함수를 설정하여 테스트를 수행하도록 수정 (기존에는 직접 git clone 받은 후 테스트 하도록 구현됨)
Desc - out 디렉토리는 테스트에서 제외되도록 설정 - 제외 사유 : .ts 파일이 .js로 트랜스파일링 되는 과정에서 jest 파일의 import 구문을 인식하지 못하여 오류가 발생하므로 테스트에서 제외하도록 설정
Desc - 이전에 생성 후 활용하지 않는 테스트 파일 존재 (extension.test.ts) - 해당 테스트 파일로 인해 테스트 실패가 발생하므로, 확장자를 .test.ts에서 .ts로 변경하여 테스트에서 제외
Desc - 변경 전 테스트 케이스에서 필요했던 코드 제거 (clone 받았던 git repo 디렉토리 관련 .gitignore 코드)
Related issue
#681
Result
getGitLog_Test.mp4
Work list
vscode/src/utils/git.tils.ts에 선언된getGitLog함수를 테스트 하는 코드입니다test repository로 설정한 원격 저장소에서 git clone을 받은 후, 해당 git 데이터를 활용하여 테스트를 진행합니다test repository는Django로 설정하였습니다 (커밋 개수 30,000개 이상으로, 성능 측정에 적합한 케이스라고 판단)test repository를 제거하여 패키지에 해당 파일이 남지 않도록 처리하였습니다(
.gitignore에test repository를 추가하여 오류로test repository가 제거되지 않아도 문제가 발생하지 않도록 설정)console.time/console.timeEnd메서드를 활용하여getGitLog함수의 처리 시간을 측정할 수 있도록 구현하였습니다(타이머 라벨명 :
getGitLog Execution Time)Discussion
"node ./out/test/runTest.js")" ~ Download VS Code, unzip it and run the integration test ~ ")test:e2e로 수정하고 unit 테스트 관련 스크립트를test:unit로 추가