앱을 만들 때 레이어를 구분하고 낮은 의존성 관계를 추구한다.
레이어는 비즈니스 로직과 UI의 구분을 의미한다.
기존 코드
처참한 코드라고 한다. 잘 짠 것 같아 보이는데 테스트를 위한 예제 코드기 때문에 테스트하기 힘들게 되어 있다는 것으로 추측할 수 있다.
위에서 말했듯이 비즈니스 로직과 UI를 구분해야 한다고 하는데 컴포넌트 내에 비즈니스 로직(함수)이 합쳐져 있는 것이 문제인 것 같다.
과거 클래스형이 주류일 때 container 컴포넌트에서 props로 내려주던 함수들을 hooks가 도입되면서 로직을 필요한 컴포넌트 내에서 바로 호출해서 사용하게 되었고, 비즈니스 로직들을 함수 컴포넌트 내부에 위치시키는 것을 허용하게 되었던 것으로 생각했다.
하지만 아래의 함수들은 hooks가 아니라서 굳이 저기에 둘 필요가 없다. 외부에 파일을 분리해두고 import해서 사용하는 게 훨씬 간결하다는게 내 추측이다.
import { useEffect, useState } from "react";
import axios from "axios";
export enum State {
ACTIVE = 1,
COMPLETED = 2,
CANCELLED = 3,
}
export interface Todo {
id: number;
text: string;
isActive: State;
}
const TodoApp = (): JSX.Element => {
const [text, setText] = useState("");
const [todos, setTodos] = useState<Todo[]>([]);
useEffect(() => {
fetchTodo();
}, []);
async function fetchTodo() {
const { data }: { data: { todos: Todo[] } } = await axios.get(
"http://127.0.0.1:3001/todo"
);
setTodos(data.todos);
}
const addTodo = (text: string) => {
setTodos([...todos, { id: todos.length, text, isActive: State.ACTIVE }]);
};
const cancelTodoByIdx = (idx: number) => {
setTodos(todos.filter((todo: Todo, todoIdx: number) => todoIdx !== idx));
};
const onChangeHandler = (e: React.ChangeEvent<HTMLInputElement>) => {
setText(e.target.value);
};
return (
<div>
<input
type="text"
value={text}
onChange={onChangeHandler}
onKeyUp={(e: React.KeyboardEvent<HTMLInputElement>) => {
if (e.keyCode === 13) {
addTodo(text);
setText("");
}
}}
style={{ width: 300 }}
placeholder="할 일을 입력하세요"
/>
<div>
{todos.map((todo: Todo, idx: number) => (
<div key={idx}>
<input
data-test="todo-item"
type="checkbox"
onChange={() => cancelTodoByIdx(idx)}
/>
<label>{todo.text}</label>
<span data-test="todo-cancel" onClick={() => cancelTodoByIdx(idx)}>
X
</span>
</div>
))}
</div>
</div>
);
};
export default TodoApp;
TypeScript
복사
테스트 코드를 잘 작성하기 위해서 코드의 레이어를 구분해 의존성을 줄여야 한다.
같은 기능을 하는 것을 모아 둔다.
서버 통신 - http 통신
useState 데이터 관리 - 상태 관리 - 비즈니스 로직
jsx - UI
여기서 설명하는 데이터 관리, 상태 관리가 비즈니스 로직에 들어갔다. hooks 자체를 선언하는 것은 문제가 안되지만 hooks의 반환값은 다른 함수에 전달해서 사용할 수 있도록 하는 게 아닐까 싶다.
E2E 테스트는 섞여 있더라도 가능하나 별도의 테스트는 까다롭다.
프로젝트 구조 잡기
src/api/todo.ts
api는 서버와 통신하는 레이어다.
import axios from "axios";
export enum State {
ACTIVE = 1,
COMPLETED = 2,
CANCELLED = 3,
}
export interface Todo {
id: number;
text: string;
isActive: State;
}
async function fetchTodo() {
const { data }: { data: { todos: Todo[] } } = await axios.get("/todos");
return data.todos;
}
TypeScript
복사
src/hooks/useTodo/index.ts
hooks는 비즈니스 로직을 담당한다.
redux, context 등으로 관리할 수 있어서 hooks로만 관리할 필요 없다. 하지만 hook은 자체적으로 제공하는 상태관리 라이브러리이므로 여기서는 hook을 쓴다.
import { useEffect, useState } from "react";
import { getTodo, State, Todo } from "../api/todo";
const useTodo = () => {
const [todos, setTodos] = useState<Todo[]>([]);
useEffect(() => {
fetchTodo();
}, []);
async function fetchTodo() {
const todos = await getTodo();
setTodos(todos);
}
const addTodo = (text: string) => {
setTodos([...todos, { id: todos.length, text, isActive: State.ACTIVE }]);
};
const cancelTodoByIdx = (idx: number) => {
setTodos(todos.filter((todo: Todo, todoIdx: number) => todoIdx !== idx));
};
return { todos, addTodo, cancelTodoByIdx };
};
export default useTodo;
TypeScript
복사
그러고보니 회사에서도 리팩터링 작업을 할 때 이런식으로 fetch를 useEffect로 첫 렌더링에 받아주고 그외 수정 및 삭제 등의 로직을 담당하는 함수들을 배치시켜서 반환해 던져주는 방식을 사용했었다. 이름도 useXXX()으로 줬다. 고작 2달 전인데 custom hooks을 잠깐 잊어버렸었다. 이 방식을 프로젝트에 바로 적용해볼 수 있겠다.
hooks에선 todo 데이터를 다루는 기능만 담당한다. api 기능을 가져와 써야 한다.
components/todo/index.tsx
UI를 담당하는 레이어다. UI도 개념적 구분이 가능하지만 여기서는 하나로 한다.
원래 TodoApp에 있던 코드 그대로 컴포넌트에 만든다. 조금 다른 점은 useTodo()를 통해서 로직을 건네받아 쓰는 것이다.
import { useState } from "react";
import useTodo from "../../hooks/useTodo";
const Todo = (): JSX.Element => {
const [text, setText] = useState("");
const { todos, addTodo, cancelTodoByIdx } = useTodo();
TypeScript
복사
테스트 코드 작성
모든 레이어를 테스트하지 않고 E2E 테스트와 비즈니스 로직인 hooks만 테스트한다.
이미 검증된 라이브러리의 기능을 테스트하지 않는다. 따라서 api 레이어를 굳이 하지 않는다.
서버에서 orm이나 DB 관련 라이브러리를 사용할 때 쿼리가 정상적으로 들어가고 응답하는지 테스트하지 않는다.
DB로 쿼리를 날리기 전에 쿼리가 잘 만들어졌는지와 응답이 원하는 데이터인지를 테스트한다.
의심되는 부분만 테스트한다.
단위 테스트와 E2E 테스트 - jest, cypress
비즈니스 로직 테스트
--save-dev @testing-library/react-hooks
// 훅스 테스트용
--save-dev axios-mock-adapter
// axios를 목킹하여 실제로 서버에 요청하지 않고 받아온 척해준다.
hook/useTodo/index.test.ts
import { renderHook, act } from '@testing-library/react-hooks';
import axios from 'axios';
import MockAdapter from 'axios-mock-adapter';
import { Todo, State } from '../../api/todo'
import useTodo from '.';
const TEMP_TODOS: Todo[] = [
{
id: 0,
text: 'todo1',
isActive: State.ACTIVE,
},
{
id: 1,
text: 'todo2',
isActive: State.ACTIVE,
},
{
id: 2,
text: 'todo3',
isActive: State.ACTIVE,
},
];
const mock = new MockAdapter(axios);
TypeScript
복사
만든 더미데이터와 axios를 목킹한 객체를 만들어서 axios가 서버로 요청하지 않게 한다.
mock.onGet('/todos').reply(200, { todos: TEMP_TODOS });
TypeScript
복사
전달한 path에 따라 응답코드와 응답데이터를 지정할 수 있다.(서버와 격리된 환경에서 테스트 가능)
Todo 항목 추가 테스트
describe("Todo App", () => {
test("can add item", async () => {
mock.onGet("/todos").reply(200, { todos: TEMP_TODOS });
// mock axios가 /todos에 대한 api를 만들어준다. 응답까지 작성
const { result, rerender, waitForNextUpdate } = renderHook(() => useTodo());
await waitForNextUpdate();
// 실제 useTodo를 호출한 상태를 renderHook으로 던져서 만들어준다.
// 렌더링 업데이트를 기다린다.
act(() => result.current.addTodo(TEMP_TODOS[0].text));
expect(result.current.todos.length).toBe(4);
// 추가하고 잘 됐는지 확인
act(() => result.current.addTodo(TEMP_TODOS[1].text));
expect(result.current.todos.length).toBe(5);
});
});
TypeScript
복사
rerender는 안 쓰는데 왜 가져온지 모르겠다.
주석은 추측해서 작성해본 것
act()는 처음 봤는데 이게 setHooks를 호출할 때 사용하는 것이라고 한다. 내부에 콜백함수로 훅을 실행시켜준다.
waitForNextUpdate()는 뭔지 궁금해서 찾아봤는데 렌더링 업데이트를 기다리는 역할이다.
여기서 사용한 이유는 useEffect를 통해 useTodo가 데이터를 가져오기 때문에 useEffect()가 실행되기까지 기다리는 것이라고 한다.
addTodo를 호출해서 길이가 늘어났는지를 테스트해본다.
Todo 항목 삭제 테스트
에러가 난다.
describe("can remove todo item", () => {
let hooks: {
current: {
todos: Todo[];
addTodo: (arg0: string) => Promise<void | undefined>;
cancelTodoByIdx: (arg0: number) => Promise<void | undefined>;
};
};
beforeEach(async () => {
mock.onGet("/todos").reply(200, { todos: TEMP_TODOS });
const { result, rerender, waitForNextUpdate }: any = renderHook(() =>
useTodo()
);
hooks = result;
await waitForNextUpdate();
act(() => result.current.addTodo(TEMP_TODOS[0].text));
act(() => result.current.addTodo(TEMP_TODOS[1].text));
act(() => result.current.addTodo(TEMP_TODOS[2].text));
});
test("first item cancel", () => {
expect(hooks.current.todos.length).toBe(6);
act(() => hooks.current.cancelTodoByIdx(0));
expect(hooks.current.todos.length).toBe(5);
});
test("last item cancel", () => {
expect(hooks.current.todos.length).toBe(6);
act(() => hooks.current.cancelTodoByIdx(5));
expect(hooks.current.todos.length).toBe(5);
});
});
TypeScript
복사
beforeEach는 리팩터링 2판에서 배웠던 함수다.
test case가 작성된 test()를 수행하기 전에 매번 실행되는 함수다.
매번 새로운 훅스를 만들어 클린한 테스트 환경을 보장한다.
beforeEach는 서버 데이터를 목킹하고 초기 데이터를 3개 추가하여 서버데이터 3 + 추가 데이터 3인 환경에서 테스트를 수행한다.
cra에 내장된 jest가 .test. 파일을 감시해서 테스트를 수행할 수 있게 코드 변경을 감지해 자동으로 테스트를 수행할 수 있다.
Watch Usage 는 그런 의미다. 감시 모드로 코드 변경(저장)시 자동 테스트를 수행해준다.
코드 최적화
cancelTodoByIdx는 filter메서드로 인해 시간복잡도가 n이다.
이를 1로 줄일 수 있다.
로직을 변경해보면 테스트가 실패한다.
기존 todos를 idx기준으로 잘라가지고 새로 만든 배열에다가 이어붙이는 방식이다.
이렇게하면 기존 todos에서 idx에 해당하는 요소만 삭제된 배열이 이어붙여질 것이다.
두번째 slice는 시작지점을 idx+1로 주는 것이 맞다.(테스트용으로 idx로 주기)
또한 const로 준다음 거기에 concat시키는 방식이 되면 애초에 newTodos가 배열 상태가 아니므로 문제가 생긴다. 일부러 이렇게 준 것.
Array(5) 이런식으로 배열을 반환시켜주면 정상 작동한다.
const cancelTodoByIdx = (idx: number) => {
// 1 : use a filter to find the todo
// setTodos(todos.filter((todo: Todo, todoIdx: number) => todoIdx !== idx));
// 2 : slice the array
let newTodos = Array<Todo>();
newTodos = newTodos.concat(todos.slice(0, idx), todos.slice(idx + 1, todos.length));
setTodos(newTodos);
};
TypeScript
복사
위 방식은 좀 특이한데 새로운 배열을 만들어 놓고 let으로 선언해서 실제로 값 할당은 newTodos.concat으로 한다. concat할 때 아무 배열도 안들어올까봐 그런건가?
왜 const로 둔다음 연결하지 않고 let으로 빈 배열을 만들어서 대체해주는 방식으로 했나?
테스트 케이스 추가 - 보다시피 잘 삭제되나 확인하는 것
test("first item middle", () => {
expect(hooks.current.todos.length).toBe(6);
act(() => hooks.current.cancelTodoByIdx(3));
expect(hooks.current.todos.length).toBe(5);
});
TypeScript
복사
axios를 목킹하지 않고 테스트하는 방법. 요청 객체를 주입
여기서도 역시나 콜백함수 방식으로 배열을 전달해준다. jest.fn().mockImp~는 뭐하는 함수인가?
변수명이 fetchTodos인 걸 보니 axios를 통해 Todos 데이터를 fetch해주는 함수로 추측이 가능하다.
describe("Todo App", () => {
test("can add item", async () => {
// mock.onGet("/todos").reply(200, { todos: TEMP_TODOS });
const fetchTodos: any = jest.fn().mockImplementation(() => TEMP_TODOS);
const { result, rerender, waitForNextUpdate } = renderHook(() => useTodo(
{getTodo: fetchTodo}
));
await waitForNextUpdate();
TypeScript
복사
사전조건을 만들어주는 beforeEach 함수의 axios mocking도 변경해준다.
갑자기 useTodo에 객체를 전달해주는데 이건 없는 파라미터였다. 왜 추가한지 모르겠다.
useTodo안으로 전달하는 건 확실히 아닌 것 같은데, mock.onGet 방식으로 api를 만들어뒀듯이 useTodo 내부의 getTodo가 새로 전달한 getTodos로 변경되길 바란 것 같다.
매개변수로 덮어씌우는 방식으로 useTodo를 변경시켜버리는 것 같다.
뭐가 맞는지 모르겠지만 옵셔널 파라미터를 사용해서 바꾸었다.
기존에는 이렇게 작성했었다.
beforeEach(async () => {
mock.onGet("/todos").reply(200, { todos: TEMP_TODOS });
const { result, rerender, waitForNextUpdate }: any = renderHook(() =>
useTodo()
);
TypeScript
복사
useTodo 수정작업을 했다. 굉장히 가독성이 떨어진다. binding pattern parameter 방식으로 전달하는 코드에 맞춰주기 위해서 getTodos에 기본값 매개변수로 값을 할당해주었고, 아예 값을 전달하지 않는 axios 목킹 방식을 사용하는 경우엔 파라미터가 없어서 에러가 발생하기 때문에 {} : {} = {}와 같이 빈객체를 옵셔널 파라미터로 줬다.
의존성 분리
테스트코드는 목킹을 최대한 하지 않는 것이 좋다.
axios를 호출할 getTodos는 Todo[]를 반환하는 하나의 함수일 뿐이다. axios를 props로 전달받아 사용한다면 axios 전체를 목킹하는 것이 아닌 useTodo는 Todo[]를 반환하는 함수를 전달하여 테스트 가능하다. - ?
function getExpired(day: number) {
return new Date() + day;
}
TypeScript
복사
전달된 숫자만큼의 만료기간을 지정해주는 모듈이다. 이는 테스트하려면 목킹이 필요하다.
의존성을 분리하면 아래와 같아진다.
function getExpired(dt: Date, day: number) {
return dt + day;
}
TypeScript
복사
dt를 외부에서 전달하면 특정 시간으로 만들어진 Date 객체를 전달해서 더 수월한 테스팅이 가능하다.
