4-2 과제 레스토랑 정보 확인 및 예약 시스템 구축하기 #100
Conversation
redux mock을 사용하기위한 셋팅을했습니다
자주 쓸법한 테스트 변수들을 셋팅해주었습니다
restaurants 데이터의 adress 부분을 수정했습니다
app쪽에 렌더링 관련 테스트 코드를 작성하였습니다
There was a problem hiding this comment.
- 저번에 말씀주셨던게 이렇게 테스트 코드를 작성하고 다음에 실제 로직 구현에 들어가는게 맞을까요?
- 구현 전 테스트 코드를 먼저 작성하신 점은 잘하셨는데, mock이랑, fixtures 등이 필요할 때 만들어지지 않은 점이 아쉬워요!
- 예를 들어, 지금 작성해 놓으신 테스트 코드에 useDispatch 도 안 쓰였는데, 미리 만들어져 있어요.
- 저라면, App 테스트 코드 부터 먼저 작성하고, 그때그떄 필요한 mock이랑 fixture를 추가하는 순서로 진행할 것 같아요!
- TDD를 하게 되면, 나의 코드가 무엇이 부족한지 또는 필요한지를 발견하고, 점진적으로 개선시키는 것도 하나의 재미더라구요!
- 테스트 코드의 역할범위에 대해 나누는 기준이있을까요?
app쪽 테스트 코드를 짜는데 최상단에 있는 컴포넌트이다보니 모든 로직을 다 구현한채로 들어가야하나 싶더라구요..!
혹시 나누는 기준이 있을까요?
- 컴포넌트별로 들어가는 테스트 코드를 각각 다르게 작성하는 기준이 있는지가 궁금하신거 맞나용?ㅎㅎ
- 컴포너트별로 관심사가 다르니, 그에 맞게 테스트 코드도 작성해주는게 좋을 것 같아요!
- App의 경우, 렌더링 정도만 테스트하고, 앱이 제대로 동작하는지는 E2E 테스트에게 책임을 넘겨도 괜찮을 것 같아요!
- 이런 부분은 팀에서 어떻게 결정하느냐에 따라 나누는 기준이 다 다를 것 같아요!
- 테스트 작성에 익숙해지시면, 나만의 기준을 한번 세워보시는 것도 좋아요!
src/App.test.jsx
Outdated
| it('input-button을 렌더링한다', () => { | ||
| const { getByText } = renderApp(); | ||
|
|
||
| expect(getByText(/완료/)).not.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
이 테스트는 무엇을 기대하는 테스트인가요? "완료" 버튼이 렌더링 된다는 것을 기대한 테스트인가요? "완료" 버튼은 요구사항에 없는 버튼이기도 하고, 테스트 설명과 테스트 코드만 보면 어떤 테스트인지 알기 어려운것 같아요ㅠ
There was a problem hiding this comment.
수정했습니다
헉.. 등록 버튼을 렌더링하는 테스트를 짜려했는데 완료로 해버렸네요
input-button을 렌더링 한다 -> 등록 버튼을 렌더링한다로 수정해보았습니다!
src/App.test.jsx
Outdated
|
|
||
| describe('App', () => { | ||
| useSelector.mockImplementation((selector) => selector({ | ||
| Restaurants: restaurants, |
There was a problem hiding this comment.
Restaurants 대문자로 사용하면 컴포넌트랑 헷갈릴것 같아요! 소문자로 통일합시다!
src/fixtures/restaurant-dummy.js
Outdated
| export const restaurantName = '한식뷔페'; | ||
|
|
||
| export const restaurantType = '한식'; | ||
|
|
||
| export const restaurantAdress = '주소'; | ||
|
|
||
| export const restaurants = [ | ||
| { | ||
| id: 1, restaurantName: '한식뷔페', restaurantType: '한식', restaurantAdress: '서울특별시 신림동', | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
수정했습니다
fixtures 라는 폴더를 따로 만들어주었으니 dummy라는 설명이 안붙어도 되겠네요..!
src/App.test.jsx
Outdated
| it('restaurants-restaurantName을 렌더링한다', () => { | ||
| const { getByText } = renderApp(); | ||
|
|
||
| expect(getByText(/한식 뷔페/)).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('restaurants-restaurantType을 렌더링한다', () => { | ||
| const { getByText } = renderApp(); | ||
|
|
||
| expect(getByText(/한식/)).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('restaurants-restaurantAdress을 렌더링한다', () => { | ||
| const { getByText } = renderApp(); | ||
|
|
||
| expect(getByText(/서울특별시 신림동/)).not.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
restaurants-빼도 테스트를 설명하는데 문제가 없을 것 같애요!
restaurants-이 오히려 읽는데 불편해요ㅠㅠ- fixture에 만들어놓은 테스트 데이터를 활용해서 테스트 코드를 작성해보세요!
There was a problem hiding this comment.
수정했습니다!
넵 fixture에 만들어 놓구 사용을 안했네요 😅
질문있습니다!
restaurantsName , restaurantsType ,restaurantsAdress 이런 네이밍들을
restaurants 라는 배열에 들어있으니까 심플하게
name , type , adress 로 바꿔봤는데 괜찮을까요?!
app test 코드의 button 을 렌더링하는 부분의 설명과 , 테스트 코드를 수정했습니다
useSelector의 넘겨주는 변수명을 소문자로 통일 했습니다
restaurant_dummy -> restaurant 로 변경했습니다
restaurants 라는 객체 안에있어서 restaurant 이라는 네이밍을 뺐습니다
렌더링테스트하는 부분에 fixtures-restaurant 데이터를 사용했습니다
store생성후 index쪽에 셋팅을 해주었습니다
UpdateResturantName 함수를 테스트하는 코드를 작성하였습니다
state.nameInput으로 변경했습니다
reducer.test 작성후 UpdateResturantName이 들어있는 reduer파일을 작성해주었습니다
UpdateRestaurantType을 테스트하는 코드를 작성했습니다
typeInput을 업데이트 하는 함수를 추가했습니다
adressInput을 업데이트해주는 테스트 케이스를 작성하였습니다
adressInput을 업데이트하는 함수를 작성해주었습니다
restaurant 배열에 정보가 추가되는 테스트코드를 작성했습니다
주소같은 데이터로 주소 -> 서울특별시 신림동으로 바꿔주었습니다
reducer파일에 restaurants 를 업데이트하는 함수를 추가했습니다
|
안녕하세요 선생님 ..! |
src/actions.js
Outdated
| export function UpdateRestaurantName(nameInput) { | ||
| return { | ||
| type: 'UpdateRestaurantName', | ||
| payload: { | ||
| nameInput, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export function UpdateRestaurantType(typeInput) { | ||
| return { | ||
| type: 'UpdateRestaurantType', | ||
| payload: { | ||
| typeInput, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export function UpdateRestaurantAdress(adressInput) { |
src/reducer.js
Outdated
| nameInput: '', | ||
| typeInput: '', | ||
| adressInput: '', |
There was a problem hiding this comment.
Input이라는 용어를 빼도 괜찮을 것 같아요!name,type,address가 레스토랑의 이름, 유형, 주소이라는 걸 더 드러나도록 변경해보시는건 어떠신가요?변수명을 바꿀 수도 있고객체로 관리할 수도 있고 방법은 여러가지 일 것 같아요!name,type,address이것만 봤을 때, 사용자의 정보인지 레스토랑의 정보인지 헷갈릴수도 있을 것 같아서요!
There was a problem hiding this comment.
수정했습니다
레스토랑의 이름 , 유형 , 주소 라는 것이 더 명확하게
restaurantName , restaurantType , restaurantAdress 이런식으로 변경해주었습니다!
함수명을 소문자로 변경했습니다
name , type , adress input들의 변수명들을 좀더 명확하게 변경했습니다
input을 테스트하는 코드를 작성하였습니다
input 테스트 코드 작성후 로직을 구현하였습니다
완료 -> 등록 으로 변경해주었습니다
완료 버튼을 렌더링하는 테스트 코드를 추가했습니다
inputContainer의 테스트 코드를 작성하였습니다. 이벤트를 다루는 코드들만 분리해서 작성했습니다
inputContainer의 로직을 구현하는 코드를 작성하였습니다
nameinput , typeinput , adressinput 을 의미를 명확하게끔 restaurantName , restaurantType , restaurantAdress 으로 변경해주었습니다
레스토랑 리스트를 보여주는 코드를 작성하였습니다
레스토랑리스트를 보여주는 로직구현을 했습니다
app쪽에 여태 작업한 리스트, 인풋 컴포넌트를 병합하는 작업을했습니다
|
안녕하세요 어느덧 마지막 요일이네요!! |
src/Input.jsx
Outdated
| handleChangeRestaurantName, | ||
| handleChangeRestaurantType, | ||
| handleChangeRestaurantAdress, | ||
| handleClickAddRestaurant, |
There was a problem hiding this comment.
저는 직접적으로 로직을 처리하는 이벤트 핸들러 함수의 경우 handle~ 을 사용하고, 중간에 전달하는 역할 정도만 하는 props 같은 이름으로 사용할 때는 on~ 으로 사용하는 편이에요. 실제 일을 처리하는 관점과 사용하는 입장에서 바라볼 때의 의도를 각각 드러내도록 아래처럼 네이밍해보시는건 어떤가요?
There was a problem hiding this comment.
수정하였습니다
저도 작성하면서 헷갈리는경우가있었는데 on은 전달 handle은 사용 이렇게 구분을 해주니 의도를 더 명확히 알수있네요!
src/RestaurantList.test.jsx
Outdated
| it('restaurantName을 렌더링한다', () => { | ||
| const { getByText } = renderListContainer(); | ||
|
|
||
| expect(getByText(name)).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('restaurantType을 렌더링한다', () => { | ||
| const { getByText } = renderListContainer(); | ||
|
|
||
| expect(getByText(type)).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('restaurantAdress를 렌더링한다', () => { | ||
| const { getByText } = renderListContainer(); | ||
|
|
||
| expect(getByText(adress)).not.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
하나의 아이템이 아닌 여러개의 아이템이 보이는 걸로 테스트를 바꿔보는건 어떠신가요?
There was a problem hiding this comment.
수정했습니다
리스트 한개에 모든 정보들이 다 렌더링되니 그렇게 변경하는게 좋은거같네요!!
src/InputContainer.jsx
Outdated
| } | ||
|
|
||
| return ( | ||
| // eslint-disable-next-line react/jsx-props-no-spreading |
There was a problem hiding this comment.
수정했습니다
저긴 제가 여러가지 방법시도하면서 남아있던 의미없는 주석이라 제거했습니다!
src/Input.jsx
Outdated
| @@ -0,0 +1,18 @@ | |||
| export default function Input({ | |||
There was a problem hiding this comment.
수정했습니다
input이 여러개있으니 form라는 단어가 더 적절하네요! 수정했습니다
input 태그에 name을 추가했습니다
테스트에서 input name을 변경했습니다
직접적으로 사용하는부분은 handle이라는 네이밍을 앞에붙이고 전달역할하는것은 on이라고 수정했습니다
각각 하나의 아이템으로 테스트했던 부분을 통합하여 테스트를 진행하게끔 변경하였습니다
Input 컴포넌트의 이름을 더 의미가명확한 Form으로 변경해주었습니다
|
피드백 주신 내용들 수정완료했습니다!! |
안녕하세요 선생님! 과제 2번째를 진행해보았는데 아직 미완성이지만 pr 먼저 드립니다..!
질문있습니다!
app쪽 테스트 코드를 짜는데 최상단에 있는 컴포넌트이다보니 모든 로직을 다 구현한채로 들어가야하나 싶더라구요..!
혹시 나누는 기준이 있을까요?