코드숨3주차 과제 (6월 24일) by Jongveloper · Pull Request #134 · CodeSoom/react-week3-assignment-1

Jongveloper · 2022-06-20T11:53:19Z

트레이너님 안녕하세요!
과제를 전부 완성하진 못하고 부족한 코드지만
리뷰요청드립니다!
Input 컴포넌트를 테스트 했습니다.
Input이 렌더링 되는지 확인하는 테스트코드를 추가하였습니다.
Input에 있는 폼에 chagnge event로 입력한 값이 value와 같은지 테스트 했습니다.

Input 컴포넌트를 테스트 했습니다. Input이 렌더링 되는지 확인하는 테스트코드를 추가하였습니다. Input에 있는 폼에 chagnge event로 입력한 값이 value와 같은지 테스트 했습니다.

리스트 컴포넌트를 테스트했습니다. tasks가 있을 때 해당 tasks에 있는 title이 나옵니다. tasks가 없을 때 "할 일이 없어요!"가 나옵니다.

Page를 테스트합니다. Page가 렌더링 되는지 확인합니다. Input에 onChangeTitle이 전달되어 이벤트를 수행하는지 체크합니다. List에 Tasks가 잘 전달되는지 확인합니다. 완료 버튼을 눌렀을 때 해당 할 일이 사라집니다.

daadaadaah

1등으로 PR 날려주셨군요!👍 매주 성실히 PR 날려주시는 모습 너무 좋습니다😎
이제 BDD 스타일로 한번 작성해보세요!
BDD 스타일(describe -context-it)에 맞춰서 테스트를 작성해보면 자연스럽게 좋은 테스트를 작성할 수 있고, 이는 곧 우리가 무엇을 해야할지에 대해 좀더 집중하도록 도와줍니다!ㅎㅎ

관련해서 아래 링크들이 도움될 거예요!
[참고]
https://marocchino.net/2016/12/04/about-rspec/
https://ko.javascript.info/testing-mocha#ref-888
https://github.com/ahastudio/til/blob/main/ruby/20161206-rspec-let.md
Ginkgo - Go 언어 개발자를 위한 BDD 테스팅 프레임워크

daadaadaah · 2022-06-20T12:57:03Z

src/Input.test.jsx

+    const inputEl = getByPlaceholderText('할 일을 입력해 주세요');
+
+    // input이 존재하는지 체크합니다.
+    expect(inputEl).toBeInTheDocument();


전반적으로 아래처럼 바로 써줘도 될 것 같은데, 따로 변수화하신 이유가 있으신가요?

expect(getByText('추가')).toBeInTheDocument();

변수화하는 것이 코드 가독성이 더 좋을 것이라 생각했는데!
지금 보니 별 차이가 없는 것 같네요ㅠㅠ...
ㅎㅎ 리팩토링 진행하겠습니다!

daadaadaah · 2022-06-20T13:32:36Z

src/Input.test.jsx

+describe('Input', () => {
+  const handleChange = jest.fn();
+
+  test('Input이 렌더링 되는지 확인합니다.', () => {


저는
test를 정말 간단한 테스트에만 사용하고,
웬만하면 BDD 스타일(describe -context-it) 같이 테스트를 작성하는 템플릿를 사용하는 걸 선호 편입니다!
이 스타일에 익숙해지시면, 지금 작성해놓으신 주석들이 필요 없어요!

앗ㅎㅎ 피드백 감사합니다! 피드백 반영하여 리팩토링 진행하겠습니다!

BDD를 적용하여 리팩토링을 진행하였습니다. 중복되지 않는 코드의 변수 선언을 해지하였습니다. 중복되는 부분의 string을 변수화 시켰습니다.

BDD를 적용하였습니다.

Page를 테스트하였습니다.

Jongveloper · 2022-06-21T11:37:05Z

안녕하세요 트레이너님!
트레이너님이 주신 피드백을 토대로 리팩토링을 진행하고
현재 테스트를 하지 못한 부분에 대해 테스트 코드를 작성하고 있습니다!

daadaadaah

describe -context-it 구조로 작성해주시긴했는데, 각각에 무엇을 작성해야하는지 아직 감을 못잡으신것 같아요ㅠㅠ처음 해보시는거라 많이 어렵죠?ㅜㅜ
저도 코드숨에서 처음 테스트 코드 작성하는 법을 배울 때, 테스트 설명을 어떡해 해야할지, 무엇을 테스트해야할지, 그리고 그것을 어떻게 테스트해야할지 등 내가 무엇을 모른지 조차도 몰라서 막막했던 것 같아요!

이럴 때, 저는 가장 간단한 하나부터 완성하여 BDD 방식의 테스트 코드 작성에 대해 감을 잡았어요!
현재 Jongveloper님 코드에서 가장 먼저 작성해보신 Input 컴포넌트부터 테스트 코드를 완성시켜볼까요?

먼저, Input 컴포넌트에 기대하는 것이 무엇일까요?
저는

앱이 실행되었을 때, 화면에 Input 컴포넌트가 잘 보이는 지를 먼저 기대할 것 같아요!
그럼, 여기서 무엇을 테스트해야할지 1개를 발견한 거예요.
그래서, 아래처럼 작성할 것 같아요!

describe('Input', () => {
    it('Input 이 보인다', () => {
      // ...
    });
});

그다음, 이러한 테스트를 어떻게 테스트 코드로 작성할 지에 대해 생각해볼 것 같아요!
(-> 이 부분은 Jongvelper님이 충분히 작성하실 수 있을 거라서 생략할게요! 단, 웬만하면 it 하나에 expect 하나 구조를 선호하는 편이예요! )

이렇게 해서 1개의 테스트를 완성했어요!

그럼 다음으로,
2. 저는 Input은 사용자의 입력을 받는 요소이므로, 사용자의 입력을 잘 받는지가 궁금할 것 같아요!
그래서, 아래처럼 작성할 것 같아요!

describe('Input', () => {
  it('Input 이 보인다', () => {
    // ...
  });
  describe('사용자가 텍스트를 입력하면', () => {
      it('handleChange 함수가 호출된다', () => {
        // ...
      });
  });
});

그 다음, 1번처럼 똑같이 테스트 코드를 작성해 볼 것 같아요!

describe, context, it 각각이 무슨 역할을 하며, 무엇을 적어야 하는지 한번 공부해보시는게 좋을 것 같아요! given-when-then도 마찬가지구요!

또한, 테스트 코드 작성에서 2가지를 아는 것이 중요하다고 생각해요.

무엇을 테스트 할 것인가
1번을 위해 어떻게 테스트코드를 작성할 것인가

1번이 명확하지 않으면, 2번을 열심히해도 의미없는 테스트가 될 것 같아요!
먼저, 각각의 컴포넌트에서 무엇을 테스트할 것인지 먼저 고민해보고, 이를 위해 어떻게 테스트 코드를 작성할지 쪼개서 과제를 한번 수행해보세요!

daadaadaah · 2022-06-21T14:18:06Z

src/Input.test.jsx

+      // input이 존재하는지 체크합니다.
+      expect(inputEl).toBeInTheDocument();
+      // button이 존재하는지 체크합니다.
+      expect(getByText('추가')).toBeInTheDocument();


input 테스트 코드에 button이 있는지를 확인하는 테스트는 필요 없을 것 같아요!

아하!! ㅎㅎ 감사합니다!
제가 주신 자료에 대한 분석이 미흡했던 것 같네요! ㅎㅎ
오늘 리팩토링 다시 진행하겠습니다!

아 input 태그 테스트로 착각했어요! 필요한 테스트입니다!! 잘하셨어요👍

daadaadaah · 2022-06-21T14:19:02Z

src/Input.test.jsx

+      // button이 존재하는지 체크합니다.
+      expect(getByText('추가')).toBeInTheDocument();
+
+      expect(inputEl.value).toBe('');


17번줄에서 한 테스트로 충분히 테스트가 이루어져서 이부분은 필요없어보여요!

ㅎㅎㅎ 중복되게 테스트를 작성했네요!
ㅎㅎㅎㅎㅎㅎ 피드백 감사합니다.
리팩토링 진행하겠습니다!

daadaadaah · 2022-06-21T14:20:34Z

src/Input.test.jsx

+        />,
+      );
+
+      const inputEl = getByPlaceholderText('할 일을 입력해 주세요');


El 라고 약어를 사용하는 것보다 El 생략해도 충분히 input 이라고 인지할 수 있어서, El을 생략할 것 같아요!

ㅎㅎㅎㅎㅎㅎ 아하!
항상 네이밍이 고민이 많이 되는 것 같습니다!
좋은 피드백 감사합니다!

daadaadaah · 2022-06-21T14:21:14Z

src/Input.test.jsx

+
+  context('input엘리먼트에 새로운 컨텐츠를 입력합니다.', () => {
+    it('input엘리먼트에 chage event로 입력한 값이 value와 같은지 테스트합니다.', () => {
+      const newContents = '숨쉬기';


contents라는 추상적인 용어보다 newTodo라고 구체적으로 표현해보는건 어떠신가요?

ㅎㅎㅎ 생각해보니 newTodo가 더 명확한 표현인 것 같습니다!
리팩토링 진행하겠습니다 좋은 피드백 감사합니다!

daadaadaah · 2022-06-21T14:21:31Z

src/Input.test.jsx

+
+      const inputEl = getByPlaceholderText('할 일을 입력해 주세요');
+
+      // input이 존재하는지 체크합니다.


주석 제거해주세용:)

아아 이 주석이
아직 테스트코드가 익숙치 않아서 남겨둔 것인데! ㅎㅎ
이제는 어떤 용도인지 알고 있으니
리팩토링시 주석 제거하겠습니다!

BDD에 대해 좀 더 공부 후 리팩토링을 진행하였습니다.

명확한 표현으로 변경하였습니다.

버튼 이벤트에 대한 테스트를 추가하였습니다.

Jongveloper · 2022-06-22T12:52:02Z

안녕하세요 트레이너님!
리뷰요청 드립니다!
트레이너님이 주신 피드백에 따라 리팩토링 진행하였습니다!
ㅠㅠㅠㅠㅠ 아홉시 전에 과제 올렸는데
계속 수정하고 수정했습니다!
양해부탁드립니다!
감사합니다!

daadaadaah

다시한번 Describe-Context-It 각각이 어떤 것을 명시하는지 알아봅시당!
Describe는 테스트의 대상을,
Context는 주어진 상황이 달라질 때를 명시할 때
It는 그래서 어떤 결과가 나오는지를 명시합니다.

어제 드린 코멘트 좀만더 꼼꼼히 다시 읽어보시면, 차이를 아실 거예요!
#134 (review)

저는 보통 context는 1) 할일이 없다면, 2)할일이 있다면, 이런식으로 주어진 상황이 달라질 때 써요!

p.s 9시 이후에 수정하셔도 되는데, 꼭 오늘처럼 수정 완료되었다고 코멘트 남겨주세용!ㅎㅎ그래야 저두 언제 리뷰할지 판단할 수 있어서용!😎

daadaadaah · 2022-06-22T13:58:36Z

src/Input.test.jsx

+import Input from './Input';
+
+describe('Input', () => {
+  const onChangeTitle = jest.fn();


handleXXX -> onXXX으로 변경하셨는데, 혹시 특별한 이유가 있으신가요?

함수명을 실제로 동작하는 앱에서 내려주는 이름으로 변경하는게
통일성이 있다고 생각이들어서 변경하게 되었습니다!

handle~으로 바꾸는게 좋을 것 같아요!

실제 일을 처리하는 곳과 사용하는 곳을 구분해서 의도를 각각 드러내줄 수 있게 하는게 좋을 것 같아요!

참고 : https://javascript.plainenglish.io/handy-naming-conventions-for-event-handler-functions-props-in-react-fc1cbb791364

daadaadaah · 2022-06-22T13:59:07Z

src/Input.test.jsx

+    onClickAddTask.mockClear();
+    render((


맥락 구분을 위해 한줄 띄어주시면 좋을 것 같아요!ㅎㅎ

ㅎㅎㅎㅎㅎ 기초적인 부분을 놓쳤네요!
피드백 감사합니다!

daadaadaah · 2022-06-22T15:41:20Z

src/Input.test.jsx

+      />
+    ));
+
+    it('호출됩니다.', () => {


무엇이 호출되는지를 명확히 해주시는게 좋을 것 같아요!

ㅎㅎㅎㅎㅎㅎ넵!
피드백해주신 걸 반영해서 리팩토링 진행하겠습니다!

daadaadaah · 2022-06-22T15:43:58Z

src/Input.test.jsx

+      <Input />
+    ));
+
+    it('label이 존재합니다.', () => {


~이 보인다라는 표현이 좀더 사용자 입장에서 생각하는 표현 같아요!

ㅠㅠ 처음에 보인다로했는데 고민하다가 바꾼건데 ㅎㅎ 판단미스였네요...!
e2e테스트가 아니라.. 존재한다!로 바꿨는데 ㅎㅎ
다시 보인다로 변경해야겠네요 ㅎㅎㅎ

daadaadaah · 2022-06-22T15:50:01Z

src/Input.test.jsx

+  context('유저가 "숨쉬기" 라는 할 일을 입력하면', () => {
+    const { getByPlaceholderText } = render((
+      <Input
+        value="아무것도 안하기"
+        onChange={onChangeTitle}
+      />
+    ));
+
+    it('입력창에 "숨쉬기" 라는 문구가 보인다.', () => {
+      const newContents = '숨쉬기';
+
+      const input = getByPlaceholderText('할 일을 입력해 주세요');
+
+      fireEvent.change(input, { target: { value: newContents } });
+
+      expect(input.value).toBe(newContents);
+    });
+  });


할일 입력 이라는 테스트 대상이 제대로 동작하는지는 어떤 함수가 잘 호출되었는지를 통해 알 수 있을 것 같고, 그러면 테스트 결과로 어떤 함수의 호출된다로 표현하는 걸 선호하는 편이예요! 이 테스트가 빠져있어요!

아하 ㅎㅎㅎ 피드백 감사합니다!
피드백을 토대로 리팩토링 진행하겠습니다!
ㅠㅠㅠ 다희님께서 Input 먼저 확실하게 끝내는게 좋을 것 같다 하셔서
시간 여유를 두고 퇴근하고 주신 자료를 열심히... 몇시간 동안 정독하고 정리해가며 읽었는데
제가 이해가 부족했나봐요!ㅎㅎㅎㅎㅎㅎㅎ
기대에 부합하지 못해 죄송합니다!
ㅎㅎㅎ 열심히 리팩토링 진행해서 좋은 코드 작성할 수 있도록 하겠습니다!
ㅎㅎㅎㅎ 디테일한 피드백 정말 감사합니다!

테스트 부분의 설명 문구를 좀 더 명확하게 변경하였습니다. render의 중복 부분을 함수화시켜 코드를 간결화했습니다. 맥락 구분을 위해 공백을 추가하였습니다.

BDD 스타일로 리팩토링을 진행하였습니다.

잘못된 BDD스타일을 수정하였습니다

Jongveloper · 2022-06-23T11:42:29Z

안녕하세요 트레이너님!
트레이너님이 주신 피드백을 토대로 리팩토링을 진행하였습니다!
리뷰요청드립니다!

daadaadaah

render 함수의 위치가 beforeEach랑 it문 밖에 있는데, it문 마다 render를 해줘야 되요!
오늘 많이 피곤하셨나요? 아니면 아직 테스트 코드 작성이 많이 어려우신가요? PR 매일 남기시는데, 아직 테스트 코드 작성에 익숙해지시지 않으신것 같아서요ㅜㅜ도움이 필요하면 언제든 문의주세요!
제가 남긴 코멘트를 그대로 리팩토링하는것보다 왜 이렇게 하는지, 다른 더 좋은 방법은 없는지 등 고민을 많이 해보시는게 좋을 것 같아요! 일단 올려주신 코드에 대해 코멘트는 남겨드렸는데, 개인적으로는 하나의 컴포넌트라도 제대로된 테스트 코드를 작성하는게 중요하다고 생각합니다. 이 점 과제 진행할 때 참고해주세요! 혹시나 방향성에 대해 고민이 있으시면 언제든 문의주시구요!😃

daadaadaah · 2022-06-23T16:11:18Z

src/Input.test.jsx

+  beforeEach(() => {
+    jest.clearAllMocks();
+
+    rendereInput();


beforeEach는 각각의 테스트함수가 실행되기 전에 실행되는데, render 함수가 들어가 있는게 어색해요!

아하 ㅎㅎㅎ 피드백 감사합니다!
피드백에 따라 리팩토링 진행하겠습니다!
피드백 주신대로 리팩토링을 진행할 때 어떻게 해야할 지
많은 자료들을 찾아보고 리팩토링을 진행하다보니
이것도해보고 저것도 해보면서 좋은코드 작성하는 것이 힘들었던 것 같습니다!

daadaadaah · 2022-06-23T16:13:48Z

src/Input.test.jsx

+    <Input
+      onClick={onClickAddTask}
+      onChangeTitle={onChangeTitle}
+    />,


value props가 빠졌어요!

피드백 감사합니다!
피드백 반영하여 리팩토링 진행하겠습니다!

daadaadaah · 2022-06-23T16:16:34Z

src/Input.test.jsx

+
+      fireEvent.change(input, { target: { value: newContents } });
+
+      expect(input.value).toBe(newContents);


handleChangeTitle 함수가 호출된다 라는 테스트로 수정해야할 것 같아요!

onChange를 테스트하는 자료들을 찾아보았는데
함수가 호출된다라는 부분에 대해 구글링을 해본 결과
제가 구글링을 잘 못하고 있는지... onChange를 호출하는 것에 대한 테스트는 제가 못찾아서...ㅠㅠ
이 부분에 대해 설명해주실 수 있을까요?

daadaadaah · 2022-06-23T16:52:13Z

src/Page.test.jsx

+  context('Page가 전달하는 Props들이 잘 전달되는지 확인합니다.', () => {
+    it('onChangeTitle이 전달되어 이벤트를 수행하는지 체크합니다.', () => {
+      const { getByPlaceholderText } = render(
+        <Page
+          tasks={emptyTask}
+          onChangeTitle={handleChange}
+        />,
+      );
+
+      const inputEl = getByPlaceholderText('할 일을 입력해 주세요');
+
+      fireEvent.change(inputEl, { target: { value: newContents } });
+
+      expect(inputEl.value).toBe(newContents);
+    });
+
+    it('Tasks가 잘 전달되는 지 확인합니다.', () => {
+      const { container } = render((
+        <Page
+          tasks={notEmptyTasks}
+        />
+      ));
+
+      expect(container).toHaveTextContent('숨쉬기');
+      expect(container).toHaveTextContent('아무것도 안하기');
+    });
+
+    it('DeleteTask가 잘 전달되는 지 확인합니다.', () => {
+      const { container, getAllByText } = render((
+        <Page
+          tasks={notEmptyTasks}
+          onClickDeleteTask={handleClick}
+        />
+      ));
+
+      expect(container).toHaveTextContent('숨쉬기');
+      expect(container).toHaveTextContent('완료');
+
+      expect(handleClick).not.toBeCalled();
+
+      fireEvent.click(getAllByText('완료')[0]);
+      fireEvent.click(getAllByText('완료')[1]);
+
+      expect(handleClick).toBeCalledWith(2);
+    });
+  });
+
+  it('onClickAddTask가 잘 전달되는 지 확인합니다.', () => {
+    const { getByText } = render((
+      <Page
+        tasks={emptyTask}
+        onClickAddTask={handleClick}
+      />
+    ));
+
+    const buttonEl = getByText('추가');
+
+    expect(buttonEl).toBeInTheDocument();
+
+    fireEvent.click(buttonEl);
+
+    expect(handleClick).toBeCalledWith(1);
+  });


prop 가 잘 전달되었는지 여부는 불필요한 테스트 같아요!

피드백 감사합니다
이 부분은 리팩토링을 하기 전 코드여서ㅎㅎㅎ
코드 리팩토링 진행하겠습니다

daadaadaah · 2022-06-23T16:53:02Z

src/List.test.jsx

+      });
+    });
+
+    describe('유저가 "완료"', () => {


describe는 테스트 대상을 표현하는 것이므로, "완료 버튼 클릭" 이라고 하는게 좋을 것 같아요!

피드백 감사합니다 피드백 반영하여 리팩토링 진행하겠습니다!

daadaadaah · 2022-06-23T17:04:09Z

src/List.test.jsx

+      context('버튼을 클릭하면', () => {
+        it('onClickDelete가 호출됩니다.', () => {
+          const { getByText } = rendererList(tasks);
+
+          fireEvent.click(getByText('완료'));
+
+          expect(onClickDelete).toBeCalledTimes(1);
+        });
+      });
+
+      context('버튼을 클릭하지 않는다면', () => {
+        it('onClickDelete가 호출되지 않습니다.', () => {
+          rendererList(tasks);
+
+          expect(onClickDelete).not.toBeCalled();
+        });
+      });


클릭했을 때와 안했을 때로 나눠서 생각하신거 좋지만, 클릭하지 않은 거에 대한 테스트는 안해도 될 것 같아요!

ㅎㅎㅎ 피드백 감사합니다!

daadaadaah · 2022-06-23T17:05:14Z

src/Input.test.jsx

+    const { getByText } = rendereInput();
+    context('버튼을 클릭하면', () => {


맥락 구분해주세요!

ㅎㅎㅎㅎ 이 부부을 놓쳤네요!
피드백 감사합니다!

daadaadaah · 2022-06-23T17:05:19Z

src/Input.test.jsx

+    context('버튼을 클릭하면', () => {
+      it('onClickAddTask함수가 호출됩니다.', () => {
+        fireEvent.click(getByText('추가'));
+
+        expect(onClickAddTask).toBeCalledTimes(1);
+      });
+    });
+    context('버튼을 클릭하지 않는다면', () => {
+      it('onClickAddTask함수가 호출되지 않습니다.', () => {
+        expect(onClickAddTask).not.toBeCalled();
+      });
+    });


클릭했을 때와 안했을 때로 나눠서 생각하신거 좋지만, 클릭하지 않은 거에 대한 테스트는 안해도 될 것 같아요!

daadaadaah · 2022-06-23T17:05:38Z

src/Input.test.jsx

+    });
+  });
+
+  describe('유저가 "추가"', () => {


describe는 테스트 대상을 표현하는 것이므로, "추가 버튼 클릭" 이라고 하는게 좋을 것 같아요!

ㅎㅎㅎ 피드백 감사합니다

daadaadaah · 2022-06-23T17:06:30Z

src/Input.test.jsx

+      // input이 존재하는지 체크합니다.
+      expect(inputEl).toBeInTheDocument();
+      // button이 존재하는지 체크합니다.
+      expect(getByText('추가')).toBeInTheDocument();


아 input 태그 테스트로 착각했어요! 필요한 테스트입니다!! 잘하셨어요👍

1. 첫 context에서 Input 컴포넌트가 렌더링 되는 상황에 describe가 적절한지 context가 적절한지 고민하다 컴포넌트가 "렌더링 되는 상황" 이기 때문에 context 키워드를 사용하였습니다. 2. 각 엘리먼트마다 describe(테스트 대상이 되는 클래스, 메소드 이름(해당 테스트에선 테스트 대상이 되는 엘리먼트) 를 작성 후 엘리먼트가 놓인 상황을 context 키워드를 사용하였습니다! 위의 부분에서 describe와 context를 어떤식으로 바꿀 수 있을 지 궁금합니다!

1. list 엘리먼트 렌더링하는 과정에 설명할 테스트 대상을 명시하는 describe를 명시하고 context를 통해 tasks가 비어있는 상황과 tasks가 비어있지 않은 상황을 구분한 후 테스트 대상의 행동을 설명했습니다. 2. button 엘리먼트를 렌더링하는 과정에 설명할 테스트 대상을 명시하는 describe를 명시하고 context를 통해 버튼이 눌리는 상황을 설명 후 해당 대상의 행동을 설명하였습니다.

tasks에 기본 값을 넣어주면 global에러가 발생하는데 이 부분에 대해서 궁금합니다! 기존 컴포넌트 테스트와 다른 방식으로 테스트 하기위해 최대한 다른 방법으로 테스트를 진행하였습니다. Input 컴포넌트의 input 엘리먼트를 테스트하면서 기존 Input 컴포넌트테스트와 달리 getByLabelText로 label이 있다는 것을 증명하고 label의 value를 통해 input이 보인다는 것을 표현했습니다.

최대한 다른 방법으로 테스트를 진행하였습니다! 2cent님의 코드를 참고하였습니다!

Jongveloper · 2022-06-24T16:55:24Z

안녕하세요 트레이너님!
트레이너님의 피드백을 따라 리팩토링을 진행하였습니다!
리뷰요청드립니다!

궁금한 부분과 제가 작성한 코드들에 대한 방식에 대한 부분은
너무 길어질 것 같아 각 커밋 메시지로 남겨놨습니다!
감사합니다!

App을 테스트하는 부분은 2cent 님의 코드를 참고하였습니다!

daadaadaah

늦게까지 작업해서 PR 날리느라 수고많으셨습니다!👍

다양한 시도를 해보는거는 좋으나, 그 시도가 방향성이나 목적이 있었으면 좋겠습니다!

daadaadaah · 2022-06-25T01:14:08Z

src/Input.test.jsx

+
+  describe('input', () => {
+    context('할 일을 입력하면', () => {
+      it('onChangeTitle이 호출됩니다.', () => {


https://github.com/CodeSoom/react-week3-assignment-1/pull/134/files#r905209422 코멘트에서 말씀드렸지만,
이렇게 구분하지 않을 경우 onChangeTitle이 prop의 onChangeTitle인지, 아니면 넣어준 값의 onChangeTitle인지 헷갈릴것 같아요! 그래서, 구분해서 네이밍하는 걸 추천드립니다!

ㅎㅎㅎ항상 네이밍이 어려운 것 같습니다!
추천해주신 클린코드를 구매해놓고 못읽고 있는 상황인데
클린코드를 완독하고 블로그에 꼭 독후감을 남기겠습니다!

daadaadaah · 2022-06-25T01:26:09Z

src/Input.test.jsx

+  context('Input컴포넌트가 렌더링되면', () => {
+    it('label이 보입니다.', () => {
+      const { container } = rendererInput();
+
+      expect(container).toHaveTextContent('할 일');
+    });
+
+    it('button이 보입니다.', () => {
+      const { getByText } = rendererInput();
+
+      expect(getByText('추가')).toBeInTheDocument();
+    });
+
+    it('input이 보입니다.', () => {
+      const { getByPlaceholderText } = rendererInput();
+
+      expect(getByPlaceholderText('할 일을 입력해 주세요')).toBeInTheDocument();
+    });
+  });
+
+  describe('input', () => {
+    context('할 일을 입력하면', () => {
+      it('onChangeTitle이 호출됩니다.', () => {
+        const event = {
+          target: {
+            value: '숨쉬기',
+          },
+        };
+
+        const { getByLabelText } = rendererInput('');
+
+        const input = getByLabelText('할 일');
+
+        fireEvent.change(input, event);
+
+        expect(onChangeTitle).toBeCalled();
+      });
+    });
+  });
+
+  describe('button', () => {
+    context('유저가 "추가" 버튼을 클릭하면', () => {
+      it('onClickAddTask함수가 호출됩니다.', () => {
+        const { getByText } = rendererInput();
+
+        fireEvent.click(getByText('추가'));
+
+        expect(onClickAddTask).toBeCalledTimes(1);
+      });
+    });
+  });


HTML 요소 별로 describe로 묶어 놓으셨는데, 각 요소별로 테스트가 많을 경우에는 필요할 것 같은데, 지금 상황에서는 필요 없을 것 같아요! (다른 컴포넌트들도 만찬가지예요!)

질문에 대한 답변

위의 부분에서 describe와 context를 어떤식으로 바꿀 수 있을 지 궁금합니다!

context가 "대비되는" 상황이 있을 때 사용합니다. 예를 들어, 1) 할일이 있을 때와 2) 할일이 없을 때 등이 있겠죠. 지금 이런 상황이 아니라서 저라면 #134 (review) 코멘트 보시면 아시듯이, 저는 it이랑 describe로 구조화할 것 같아요!

ㅎㅎㅎ
좋은 피드백 감사합니다!

daadaadaah · 2022-06-25T01:38:55Z

src/List.test.jsx

+  const TASK = [{
+    id: 1,
+    title: '뭐라도 하기',
+  },
+  ];


TASKS라는 복수의 의미로 적는게 좋을 것 같아요!

배열의 경우, 테스트 데이터로 1개보다 2~3개 정도로 하는게 좋습니다!

할일 배열이 여러 곳에서 쓰이는데, 이런 데이터를 보통 fixture라고 이야기합니다. 저는 이러한 데이터를 fixtures 폴더 하나 만들어서 따로 관리하는 편이예요!

ㅎㅎㅎ 아하 좋은 패턴을 알려주셔서 감사합니다!
다음부터 할일 배열이라는 것을 따로 관리하겠습니다!

daadaadaah · 2022-06-25T01:41:19Z

src/List.test.jsx

+    context('tasks가 비어있지 않을 시', () => {
+      it('해당 tasks의 title이 보입니다.', () => {
+        const { container } = rendererList({ tasks: TASK });
+
+        TASK.forEach((task) => {
+          expect(container).toHaveTextContent(task.title);
+        });
+      });
+    });
+
+    context('tasks가 비어있을 시', () => {
+      it('"할 일이 없어요!"가 출력됩니다.', () => {
+        const { container } = rendererList({ tasks: [] });
+
+        expect(container).toHaveTextContent('할 일이 없어요!');
+      });
+    });


context를 아주 적절하게 사용하신 부분이예요👍

ㅎㅎㅎ 감사합니다!
트레이너님이 주신 피드백과 자료들을 통해
어느정도 감이 온 것 같습니다!

daadaadaah · 2022-06-25T01:51:54Z

src/List.test.jsx

+  describe('button', () => {
+    context('유저가 "완료" 버튼을 클릭하면', () => {
+      it('onClickDelete가 호출됩니다.', () => {
+        const { getByText } = rendererList({ tasks: TASK });
+
+        fireEvent.click(getByText('완료'));
+
+        expect(onClickDelete).toBeCalledTimes(1);
+      });
+    });
+  });


이 테스트가 tasks가 있을 때에 대한 테스트라서, 저라면, context('tasks가 비어있지 않을 시') 테스트 안에 describe('완료 버튼 클릭') - it('handleClickDelete') 로 구조화할 것 같아요!

아하 ㅎㅎㅎㅎㅎ 제가 너무 단편적으로 describe와 context에 대해 생각을 가졌던 것 같습니다!
이 부분 참고하겠습니다! 감사합니다!

daadaadaah · 2022-06-25T01:52:57Z

src/List.test.jsx

+  });
+
+  describe('list', () => {
+    context('tasks가 비어있지 않을 시', () => {


task가 비어있고 없고 보다, tasks 가 있고 없고로 표현하시는게 좀더 자연스럽다고 생각해요

ㅎㅎㅎㅎ 좋은 피드백 감사합니다!

daadaadaah · 2022-06-25T01:56:42Z

src/Page.test.jsx

+import { render, fireEvent } from '@testing-library/react';
+
+import Page from './Page';
+
+describe('Page', () => {
+  const onChangeTitle = jest.fn();
+  const onClickAddTask = jest.fn();
+  const onClickDeleteTask = jest.fn();
+
+  const todoInWriting = {
+    target: {
+      value: '숨쉬기',
+    },
+  };
+
+  const TASK = [
+    {
+      id: 1,
+      title: '숨쉬기',
+    },
+    {
+      id: 2,
+      title: '아무것도 안하기',
+    },
+  ];
+
+  const EMPTYTASK = [];
+
+  function rendererPage({ tasks, taskTitle }) {
+    return render((
+      <Page
+        tasks={tasks}
+        taskTitle={taskTitle}
+        onChangeTitle={onChangeTitle}
+        onClickAddTask={onClickAddTask}
+        onClickDeleteTask={onClickDeleteTask}
+      />
+    ));
+  }
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  context('Page가 렌더링되면', () => {
+    it('To-do가 보입니다.', () => {
+      const { container } = rendererPage({ tasks: EMPTYTASK });
+
+      expect(container).toHaveTextContent('To-do');
+    });
+  });
+
+  describe('<Input/>', () => {
+    context('Input 컴포넌트가 렌더링되면', () => {
+      it('button이 보입니다.', () => {
+        const { getByText } = rendererPage({ tasks: EMPTYTASK });
+
+        expect(getByText('추가')).toBeInTheDocument();
+      });
+
+      it('input이 보입니다.', () => {
+        const { getByLabelText } = rendererPage({ tasks: EMPTYTASK });
+
+        expect(getByLabelText('할 일').value).toBe('');
+      });
+    });
+
+    describe('input', () => {
+      context('할 일을 입력하면', () => {
+        it('onChangeTitle이 호출됩니다.', () => {
+          const { getByPlaceholderText } = rendererPage({ tasks: EMPTYTASK, taskTitle: '' });
+
+          const input = getByPlaceholderText('할 일을 입력해 주세요');
+
+          fireEvent.change(input, todoInWriting);
+
+          expect(onChangeTitle).toBeCalled();
+        });
+      });
+    });
+
+    describe('button', () => {
+      context('유저가 "추가" 버튼을 클릭하면', () => {
+        it('onClickAddTask 함수가 호출됩니다.', () => {
+          const { getByText } = rendererPage({ tasks: EMPTYTASK });
+
+          fireEvent.click(getByText('추가'));
+
+          expect(onClickAddTask).toBeCalled();
+        });
+      });
+    });
+  });
+
+  describe('List컴포넌트가 렌더링 되면', () => {
+    describe('list', () => {
+      context('tasks가 비어있지 않을 시', () => {
+        it('해당 tasks의 title이 보입니다.', () => {
+          const { container } = rendererPage({ tasks: TASK });
+
+          TASK.forEach((task) => {
+            expect(container).toHaveTextContent(task.title);
+          });
+        });
+      });
+
+      context('tasks가 비어있을 시', () => {
+        it('"할 일이 없어요!"가 출력됩니다.', () => {
+          const { container } = rendererPage({ tasks: EMPTYTASK });
+
+          expect(container).toHaveTextContent('할 일이 없어요');
+        });
+      });
+    });
+
+    describe('button', () => {
+      context('유저가 "완료" 버튼을 클릭하면', () => {
+        it('onClickDelete가 호출됩니다.', () => {
+          const { getAllByText } = rendererPage({ tasks: TASK });
+
+          fireEvent.click(getAllByText('완료')[0]);
+
+          expect(onClickDeleteTask).toBeCalledTimes(1);
+        });
+      });
+    });
+  });
+
+  describe('Item컴포넌트가 렌더링되면', () => {
+    describe('li', () => {
+      context('tasks가 비어있지 않을 시', () => {
+        it('해당 tasks의 title이 보입니다.', () => {
+          const { container } = rendererPage({ tasks: TASK });
+
+          expect(container).toHaveTextContent('숨쉬기');
+        });
+      });
+
+      context('tasks가 비어있을 시', () => {
+        it('"할 일이 없어요!" 가 출력됩니다.', () => {
+          const { container } = rendererPage({ tasks: EMPTYTASK });
+
+          expect(container).toHaveTextContent('할 일이 없어요!');
+        });
+      });
+    });
+
+    describe('button', () => {
+      context('완료버튼을 클릭하면', () => {
+        it('onClickDelete가 호출됩니다.', () => {
+          const { getAllByText } = rendererPage({ tasks: TASK });
+
+          fireEvent.click(getAllByText('완료')[0]);
+          fireEvent.click(getAllByText('완료')[1]);
+
+          expect(onClickDeleteTask).toBeCalledTimes(2);
+        });
+      });
+    });
+  });
+});


"~렌더링" 관련해서 어디서는 context로, 어디서는 describe로 사용하신 것 같고, 3중 중첩 describe 도 많이 복잡해보여요ㅠㅠ그래서, 아래처럼 간단히 해도 괜찮을 것 같아요!

context('tasks가 비어있지 않을 시', () => { it('해당 tasks의 title이 보입니다.', () => { const { container } = rendererPage({ tasks: TASK }); expect(container).toHaveTextContent('숨쉬기'); }); }); context('tasks가 비어있을 시', () => { it('"할 일이 없어요!" 가 출력됩니다.', () => { const { container } = rendererPage({ tasks: EMPTYTASK }); expect(container).toHaveTextContent('할 일이 없어요!'); }); }); describe('완료 버튼 클릭', () => { it('onClickDelete가 호출됩니다.', () => { const { getAllByText } = rendererPage({ tasks: TASK }); fireEvent.click(getAllByText('완료')[0]); fireEvent.click(getAllByText('완료')[1]); expect(onClickDeleteTask).toBeCalledTimes(2); }); });

tasks에 기본 값을 넣어주면 global에러가 발생하는데 이 부분에 대해서 궁금합니다!

좀더 구체적으로 이야기해주실 수 있나요? tasks에 기본값을 넣었다 이야기가 어떻게 코딩하셨는지가 모르겠어요ㅠㅠ

저도 3중 중첩 describe에 대해 가독성이 많이 떨어진다는 생각을 가졌는데! ㅎㅎ
이 부분에 대해서 describe와 context를 나누는 전략을 잘 세워야겠다는 생각이 들었습니다!
좋은 피드백 감사합니다!
확인해보니 global은 오류 메세지가아니라 테스트 커버리지에 대한 문제였던 것 같습니다! ㅎㅎㅎ감사합니다!

daadaadaah · 2022-06-25T04:59:04Z

src/App.test.jsx

+  function renderAppWithInputValue() {
+    const result = rendererApp();
+
+    const { getByLabelText } = result;
+
+    fireEvent.change(getByLabelText('할 일'), todoInWriting);
+
+    return result;
+  }
+
+  function renderAppWithTasks() {
+    const result = rendererApp();
+
+    const { getByRole, getByText } = result;
+
+    TASKS.forEach((task) => {
+      fireEvent.change(getByRole('textbox'), { target: { value: task.title } });
+
+      fireEvent.click(getByText('추가'));
+    });
+
+    return result;
+  }


여기 함수 내에 있는 코드들이 중복되더라도, 그냥 it 문 안에 넣어주세요!

엇 제가 실수를 했었네요! ㅎㅎㅎ
좋은 피드백 감사합니다!

daadaadaah · 2022-06-25T05:03:22Z

src/Page.test.jsx


-      fireEvent.change(inputEl, { target: { value: newContents } });
+      it('input이 보입니다.', () => {
+        const { getByLabelText } = rendererPage({ tasks: EMPTYTASK });


통일감 있게 getByPlaceholderText 로 해주시는게 좋아요!
다르니까 다른 요소 인가 싶었어요!

ㅎㅎ 이 부분에 대해서도 고민을 많이 했는데
통일감 있게 작성할 수 있도록 하겠습니다! ㅎㅎ 좋은 피드백 감사합니다!

daadaadaah · 2022-06-25T05:13:40Z

src/App.test.jsx

+      it('value가 "" 이됩니다.', () => {
+        const { getByLabelText, getByText } = renderAppWithInputValue();
+
+        fireEvent.click(getByText('추가'));
+
+        expect(getByLabelText('할 일')).toHaveDisplayValue('');
+      });


추가되었을 때, 입력란 리셋되는거 까지 고려하신 점 좋지만, 삭제하셔도 될 것 같아요!

ㅎㅎㅎㅎ 좋은 피드백 감사합니다!

Jongveloper · 2022-06-25T05:29:07Z

ㅎㅎㅎ 좋은 피드백 감사합니다!
트레이너님이 남겨주신 피드백 덕분에
BDD스타일에 대한 감이 조금은 온 것 같습니다!
정말 감사합니다! ㅎㅎㅎ

test Input Test

0574959

Input 컴포넌트를 테스트 했습니다. Input이 렌더링 되는지 확인하는 테스트코드를 추가하였습니다. Input에 있는 폼에 chagnge event로 입력한 값이 value와 같은지 테스트 했습니다.

Jongveloper changed the title ~~test Input Test~~ 코드숨3주차 과제 Jun 20, 2022

Jongveloper added 2 commits June 20, 2022 22:10

test List

35e1edf

리스트 컴포넌트를 테스트했습니다. tasks가 있을 때 해당 tasks에 있는 title이 나옵니다. tasks가 없을 때 "할 일이 없어요!"가 나옵니다.

daadaadaah reviewed Jun 20, 2022

View reviewed changes

Jongveloper added 3 commits June 21, 2022 19:36

refactor Apply BDD & variable

0ce521a

BDD를 적용하여 리팩토링을 진행하였습니다. 중복되지 않는 코드의 변수 선언을 해지하였습니다. 중복되는 부분의 string을 변수화 시켰습니다.

refactor BDD

25093eb

BDD를 적용하였습니다.

fix Page Test

798e0be

Page를 테스트하였습니다.

Jongveloper changed the title ~~코드숨3주차 과제~~ 코드숨3주차 과제 (6월 21일) Jun 21, 2022

daadaadaah reviewed Jun 21, 2022

View reviewed changes

Jongveloper added 3 commits June 22, 2022 20:14

refactor BDD

6869614

BDD에 대해 좀 더 공부 후 리팩토링을 진행하였습니다.

rename inputEl => input

3d77823

명확한 표현으로 변경하였습니다.

refactor ChangeExplantion

6da7d72

Jongveloper changed the title ~~코드숨3주차 과제 (6월 21일)~~ 코드숨3주차 과제 (6월 22일) Jun 22, 2022

test Event

c4c5f27

버튼 이벤트에 대한 테스트를 추가하였습니다.

daadaadaah reviewed Jun 22, 2022

View reviewed changes

Jongveloper added 3 commits June 23, 2022 19:17

refactor Input Test

c866caa

테스트 부분의 설명 문구를 좀 더 명확하게 변경하였습니다. render의 중복 부분을 함수화시켜 코드를 간결화했습니다. 맥락 구분을 위해 공백을 추가하였습니다.

refactor BDD

7cf6ac0

BDD 스타일로 리팩토링을 진행하였습니다.

refactor Input

34b46f2

잘못된 BDD스타일을 수정하였습니다

Jongveloper changed the title ~~코드숨3주차 과제 (6월 22일)~~ 코드숨3주차 과제 (6월 23일) Jun 23, 2022

daadaadaah reviewed Jun 23, 2022

View reviewed changes

Jongveloper added 4 commits June 24, 2022 20:04

test App

7840fa4

최대한 다른 방법으로 테스트를 진행하였습니다! 2cent님의 코드를 참고하였습니다!

Jongveloper changed the title ~~코드숨3주차 과제 (6월 23일)~~ 코드숨3주차 과제 (6월 24일) Jun 24, 2022

daadaadaah reviewed Jun 25, 2022

View reviewed changes

daadaadaah merged commit 4175aed into CodeSoom:Jongveloper Jun 28, 2022


		const inputEl = getByPlaceholderText('할 일을 입력해 주세요');

		// input이 존재하는지 체크합니다.


		fireEvent.change(input, { target: { value: newContents } });

		expect(input.value).toBe(newContents);

		const { getByText } = rendereInput();
		context('버튼을 클릭하면', () => {

Conversation

Jongveloper commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daadaadaah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jongveloper commented Jun 21, 2022

Uh oh!

daadaadaah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daadaadaah Jun 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jongveloper commented Jun 22, 2022

Uh oh!

daadaadaah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jongveloper commented Jun 23, 2022

Uh oh!

daadaadaah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jongveloper commented Jun 20, 2022 •

edited

Loading

daadaadaah left a comment •

edited

Loading

daadaadaah Jun 21, 2022 •

edited

Loading

daadaadaah left a comment •

edited

Loading