TEST

테스트 코드를 작성하며 설계의 문제를 발견한 이야기 - 역할 분리의 중요성

류큐큐 2025. 1. 14. 14:05

문제 상황

초기에는 GitMergeRequestManager 클래스가 모든 책임을 담당하고 있었습니다. 이 클래스는 다음과 같은 다양한 작업을 수행했습니다:

  1. 프로젝트 존재 여부 확인 및 등록.
  2. 브랜치 등록.
  3. 커밋 및 변경된 파일의 저장.
  4. 머지 리포트 초기화.

테스트 코드를 작성하면서, 다음과 같은 문제점이 드러났습니다:

  • 복잡성 증가: 여러 책임을 한 클래스에 모으다 보니 테스트 코드에서 모킹해야 할 객체가 너무 많아졌습니다.
  • 가독성 저하: 테스트 코드가 복잡해지면서 각 로직을 검증하는 데 많은 코드가 필요했습니다.
  • 유지보수성 저하: 클래스가 너무 많은 역할을 맡고 있어, 변경 사항이 생길 경우 전체 코드를 다시 검토해야 했습니다.

 

@Component
public class GitMergeRequestManager {

	private final ProjectFinder projectFinder;
	private final ProjectRegister projectRegister;
	private final BranchRegister branchRegister;
	private final CommitRegister commitRegister;
	private final ChangedFileRegister changedFileRegister;
	private final MergeReportRegister mergeReportRegister;

	public GitMergeRequestManager(ProjectFinder projectFinder, ProjectRegister projectRegister, BranchRegister branchRegister,
								  CommitRegister commitRegister, ChangedFileRegister changedFileRegister,
								  MergeReportRegister mergeReportRegister) {
		this.projectFinder = projectFinder;
		this.projectRegister = projectRegister;
		this.branchRegister = branchRegister;
		this.commitRegister = commitRegister;
		this.changedFileRegister = changedFileRegister;
		this.mergeReportRegister = mergeReportRegister;
	}

	@Transactional
	public long register(GitMergeRequestEvent gitMergeRequestEvent) {
		Optional<Project> projectOpt = projectFinder.findByGitProjectId(
			gitMergeRequestEvent.getGitProjectId());

		long projectId;
		if(projectOpt.isPresent()) {
			Project project = projectOpt.get();
			projectId = project.getId();
		}else{
			projectId = projectRegister.register(gitMergeRequestEvent.project());
		}

		long branchId = branchRegister.register(projectId, gitMergeRequestEvent.branch());

		gitMergeRequestEvent.commits().forEach(commit -> {
			long commitId = commitRegister.save(branchId, commit);
			changedFileRegister.register(commitId, commit.getChangedFiles());
		});

		mergeReportRegister.register(MergeReport.init(branchId, gitMergeRequestEvent.getTotalFilesCount()));

		return projectId;
	}

}

 

 

테스트 코드로 확인된 문제 코드

아래는 GitMergeRequestManager의 초기 테스트 코드입니다:

class GitMergeRequestManagerTest extends BaseUnitTest {

    @InjectMocks
    private GitMergeRequestManager gitMergeRequestManager;

    @Mock
    private ProjectFinder projectFinder;

    @Mock
    private ProjectRegister projectRegister;

    @Mock
    private BranchRegister branchRegister;

    @Mock
    private CommitRegister commitRegister;

    @Mock
    private ChangedFileRegister changedFileRegister;

    @Mock
    private MergeReportRegister mergeReportRegister;

    @DisplayName("기존 프로젝트가 존재할 때 register 메서드를 검증한다.")
    @Test
    void shouldRegisterWhenProjectExists() {
        // Given
        GitMergeRequestEvent event = mock(GitMergeRequestEvent.class);
        Project existingProject = new Project(1L, "existing-project", "test-repo", "test-owner");
        when(event.getGitProjectId()).thenReturn(1L);
        when(projectFinder.findByGitProjectId(1L)).thenReturn(Optional.of(existingProject));
        when(branchRegister.register(eq(1L), any(Branch.class))).thenReturn(2L);

        Commit commit = mock(Commit.class);
        when(event.commits()).thenReturn(List.of(commit));
        when(commitRegister.save(eq(2L), eq(commit))).thenReturn(3L);

        // When
        long result = gitMergeRequestManager.register(event);

        // Then
        assertEquals(1L, result, "기존 프로젝트의 ID가 반환되어야 합니다.");
        verify(projectFinder, times(1)).findByGitProjectId(1L);
        verify(projectRegister, never()).register(any(Project.class));
        verify(branchRegister, times(1)).register(1L, event.branch());
        verify(commitRegister, times(1)).save(eq(2L), eq(commit));
        verify(changedFileRegister, times(1)).register(3L, commit.getChangedFiles());
        verify(mergeReportRegister, times(1)).register(any(MergeReport.class));
    }

    @DisplayName("새로운 프로젝트가 등록될 때 register 메서드를 검증한다.")
    @Test
    void shouldRegisterWhenProjectDoesNotExist() {
        // Given
        GitMergeRequestEvent event = mock(GitMergeRequestEvent.class);
        Project newProject = mock(Project.class);
        when(event.getGitProjectId()).thenReturn(1L);
        when(projectFinder.findByGitProjectId(1L)).thenReturn(Optional.empty());
        when(event.project()).thenReturn(newProject);
        when(projectRegister.register(newProject)).thenReturn(10L);
        when(branchRegister.register(eq(10L), any(Branch.class))).thenReturn(20L);

        Commit commit = mock(Commit.class);
        when(event.commits()).thenReturn(List.of(commit));
        when(commitRegister.save(eq(20L), eq(commit))).thenReturn(30L);

        // When
        long result = gitMergeRequestManager.register(event);

        // Then
        assertEquals(10L, result, "새로 등록된 프로젝트의 ID가 반환되어야 합니다.");
        verify(projectFinder, times(1)).findByGitProjectId(1L);
        verify(projectRegister, times(1)).register(newProject);
        verify(branchRegister, times(1)).register(10L, event.branch());
        verify(commitRegister, times(1)).save(eq(20L), eq(commit));
        verify(changedFileRegister, times(1)).register(30L, commit.getChangedFiles());
        verify(mergeReportRegister, times(1)).register(any(MergeReport.class));
    }
}
 

 

문제점:

  1. 모킹할 객체가 너무 많아져서 테스트 작성이 번거롭다.
  2. GitMergeRequestManager가 지나치게 많은 책임을 가지고 있다.

 

해결 방법: 역할 분리

이 문제를 해결하기 위해, GitMergeRequestManager의 책임을 의미 있는 도메인 단위로 분산시켰습니다:

  1. 프로젝트 관련 로직 → ProjectManager로 이동
  2. 브랜치 및 커밋 관련 로직 → BranchManager로 이동

 

개선된 코드

1. 변경된 GitMergeRequestManager

@Component
public class GitMergeRequestManager {

	private final ProjectManager projectManager;
	private final BranchManager branchManager;
	private final MergeReportRegister mergeReportRegister;

	public GitMergeRequestManager(ProjectManager projectManager, BranchManager branchManager,
								  MergeReportRegister mergeReportRegister) {
		this.projectManager = projectManager;
		this.branchManager = branchManager;
		this.mergeReportRegister = mergeReportRegister;
	}

	@Transactional
	public long register(GitMergeRequestEvent gitMergeRequestEvent) {
		long projectId = projectManager.findOrRegisterProject(gitMergeRequestEvent.project());
		long branchId = branchManager.registerBranchWithCommits(projectId, gitMergeRequestEvent.branch(), gitMergeRequestEvent.commits());
		mergeReportRegister.register(MergeReport.init(branchId, gitMergeRequestEvent.getTotalFilesCount()));

		return projectId;
	}
}

 

2. ProjectManager

@Component
public class ProjectManager {

	private final ProjectFinder projectFinder;
	private final ProjectRegister projectRegister;

	public ProjectManager(ProjectFinder projectFinder, ProjectRegister projectRegister) {
		this.projectFinder = projectFinder;
		this.projectRegister = projectRegister;
	}

	public long findOrRegisterProject(Project project) {
		Optional<Project> projectOpt = projectFinder.findByGitProjectId(project.getGitProjectId());
		return projectOpt.map(Project::getId).orElseGet(() -> projectRegister.register(project));
	}
}

 

3. BranchManager

@Component
public class BranchManager {

	private final BranchRegister branchRegister;
	private final CommitManager commitManager;

	public BranchManager(BranchRegister branchRegister, CommitManager commitManager) {
		this.branchRegister = branchRegister;
		this.commitManager = commitManager;
	}

	public long registerBranchWithCommits(long projectId, Branch branch, List<Commit> commits) {
		long branchId = branchRegister.register(projectId, branch);
		commitManager.processCommits(branchId, commits);
		return branchId;
	}
}

 

 

개선된 테스트 코드

역할 분리 후, 테스트 코드의 모킹이 간소화되고 가독성이 개선되었습니다:

 

	@InjectMocks
	private GitMergeRequestManager gitMergeRequestManager;

	@Mock
	private ProjectManager projectManager;

	@Mock
	private BranchManager branchManager;

	@Mock
	private MergeReportRegister mergeReportRegister;
    
    
	@DisplayName("기존 프로젝트가 존재할 때 register 메서드를 검증한다.")
	@Test
	void shouldRegisterWhenProjectExistsUsingMatchers() {
		// Given
		GitMergeRequestEvent event = mock(GitMergeRequestEvent.class);

		when(event.project()).thenReturn(mock(Project.class));
		when(event.branch()).thenReturn(mock(Branch.class));
		when(event.commits()).thenReturn(List.of(mock(Commit.class)));
		when(event.getTotalFilesCount()).thenReturn(5);

		when(projectManager.findOrRegisterProject(any(Project.class))).thenReturn(1L);
		when(branchManager.registerBranchWithCommits(anyLong(), any(Branch.class), anyList())).thenReturn(2L);

		// When
		long result = gitMergeRequestManager.register(event);

		// Then
		assertEquals(1L, result, "프로젝트 ID가 반환되어야 합니다.");
		verify(projectManager, times(1)).findOrRegisterProject(any(Project.class));
		verify(branchManager, times(1)).registerBranchWithCommits(eq(1L), any(Branch.class), anyList());
		verify(mergeReportRegister, times(1)).register(any(MergeReport.class));
	}

 

 

깨달은 점

  1. 테스트 코드는 설계의 문제를 드러낸다 테스트 코드를 작성하다 보면 설계가 복잡하다는 사실을 자연스럽게 알게 된다.
  2. 책임 분리는 테스트 코드의 간결성과 가독성을 크게 향상시킨다.
  3. 역할을 명확히 나누면 유지보수가 쉬워지고, 각 객체를 독립적으로 테스트할 수 있다.
  4. 테스트 코드를 작성하며, 자연스럽게 코드 설계의 문제점을 발견하고 개선할 수 있었다.

 

결론

테스트 코드 작성은 단순히 코드의 동작을 검증하는 것을 넘어, 설계의 문제점과 개선 방향을 알려주는 중요한 역할을 합니다.

역할 분리를 통해 단일 책임 원칙을 준수하며, 더 나은 코드 품질을 달성할 수 있었습니다.

테스트는 코드 개선의 출발점임을 잊지 말아야 합니다.

'TEST' 카테고리의 다른 글

테스트 코드 작성 시 Mockito로 static 메서드 모킹하기  (0) 2025.02.14
테스트 코드를 작성하며 설계의 문제를 발견한 이야기 - 무심코 작성한 코드의 숨은 위험을 찾아내다  (0) 2025.02.11
테스트 코드를 작성하며 설계의 문제를 발견한 이야기 - 데미터의 법칙  (1) 2025.01.14
테스트 코드 - 더미 데이터 쉽게 만들기 (Easy Random) 사용하기 (2)  (1) 2024.03.19
테스트 코드 - 더미 데이터 쉽게 만들기 (Easy Random) 알아보기  (0) 2023.08.26

'TEST'의 다른글

  • 현재글테스트 코드를 작성하며 설계의 문제를 발견한 이야기 - 역할 분리의 중요성

관련글

티스토리툴바