chore: migrate tests from karma to jest #79
Conversation
chore: remove karma-jasmine test environment
package.json
Outdated
| @@ -41,19 +40,11 @@ | |||
| "eslint-config-prettier": "^6.11.0", | |||
| "eslint-config-tui": "^2.2.0", | |||
| "eslint-loader": "^3.0.0", | |||
| "eslint-plugin-jest": "^24.3.6", | |||
| "eslint-plugin-prettier": "^3.1.3", | |||
| "file-loader": "^0.9.0", | |||
| "istanbul-instrumenter-loader": "^1.0.0", | |||
There was a problem hiding this comment.
이것도 카르마에서 사용되는 패키지라서 삭제해야 합니다. 다른 컴포넌트도 확인 부탁드립니다.
There was a problem hiding this comment.
모든 컴포넌트에 대해 사용하지 않는 패키지 삭제하도록 하겠습니다.
| @@ -65,7 +66,7 @@ describe('DateRangePicker', function() { | |||
| it('should set null to end-date when changing start-date to a later date than end-date', function() { | |||
| picker.setStartDate(new Date(2017, 0, 20)); | |||
|
|
|||
| expect(picker.getEndDate()).toEqual(null); | |||
| expect(picker.getEndDate()).toBeNull(); | |||
There was a problem hiding this comment.
getEndDate가 null일 수 있군요. index.d.ts에 타입 정의에서 리턴에 null도 추가해야겠어요. todo 리스트에 넣어 주세요.
public getEndDate: () => Date | null;
There was a problem hiding this comment.
네! todo list에 추가하도록 하겠습니다.
test/calendar/body.spec.js
Outdated
| body._dateLayer.changeLanguage = jest.fn(); | ||
| body._monthLayer.changeLanguage = jest.fn(); | ||
| body._yearLayer.changeLanguage = jest.fn(); | ||
|
|
||
| body.changeLanguage('ko'); | ||
| expect(body._dateLayer.changeLanguage).toHaveBeenCalledWith('ko'); |
There was a problem hiding this comment.
DOM 요소를 통해 검증하거나 다른 방법을 통해 검증할 수는 없을까요? 내부 구현에 너무 깊숙이 접근해서 검증하고 있어서요.
There was a problem hiding this comment.
render 메서드를 호출하는 것으로 DOM 요소를 생성하도록 했고, changeLanguage 메서드에 의해 한글로 변환되는 요소(date의 요일, month의 월)이 한글인지 toMatch matcher를 이용해 검증하는 것으로 변경했습니다.
test/calendar/body.spec.js
Outdated
| body._dateLayer.remove = jest.fn(); | ||
| body._monthLayer.render = jest.fn(); |
There was a problem hiding this comment.
이 부분도 마찬가지네요. DOM 요소를 통해 검증할 수 있으면 좋을 것 같아요. 이런 테스트는 아무래도 내부 구현을 조금만 변경해도 다 깨질테니까요
There was a problem hiding this comment.
render 메서드 호출 이전과 이후 DOM 요소를 비교해 검증하는 것으로 변경했습니다.
test/calendar/header.spec.js
Outdated
| 'December 2016' | ||
| ); | ||
| }); | ||
|
|
||
| it('should fire "click" custom event when click a button', function() { | ||
| var spy = jasmine.createSpy('button click handler'); | ||
| var spy = jest.fn(); | ||
| header.on('click', spy); | ||
|
|
||
| header._container.querySelector('.' + constants.CLASS_NAME_NEXT_MONTH_BTN).click(); |
There was a problem hiding this comment.
이런 코드는 함수로 추상화해서 사용하는 것도 좋겠어요~! (중간에 _container는 제거할 수 있음 제거해도 좋겠네요)
function clickBtnInHeader(className) {
header._container.querySelector(`.${className}`).click();
}There was a problem hiding this comment.
말씀 해주신 것처럼 반복되는 동작에 대해 함수로 추상화 해서 사용하는 것으로 변경했습니다.
test/calendar/header.spec.js
Outdated
| @@ -85,7 +85,8 @@ describe('Calendar', function() { | |||
| }); | |||
|
|
|||
| it('"changeLanaguage" should re-initilize formatters', function() { | |||
There was a problem hiding this comment.
꼭 필요한 검증 사항인가요~?퍼블릭한 프로퍼티나 메서드로만 검증하는게 좋을 것 같아요.
There was a problem hiding this comment.
내부 메서드를 검증하는 불필요한 테스트 삭제했습니다.
Please check if the PR fulfills these requirements
fix #xxx[,#xxx], where "xxx" is the issue number)Description
Thank you for your contribution to TOAST UI product. 🎉 😘 ✨