Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BG-200]: 테스트 fixture 리팩토링 (5h / 3h) #14

Merged
merged 8 commits into from
Jul 2, 2024

Conversation

Dltmd202
Copy link
Member

@Dltmd202 Dltmd202 commented Jul 1, 2024

Why

간단하게 테스트 픽스처만 리팩토링하려고 했는데,
user의 식별자를 UUID에서 String으로 수정하면서 작업양이 많아짐

Result

image

Prize

  • 픽스처 자체를 테스트용 스프링 빈으로 등록했다.
@Component
class UserFixture(
    private val userRepository: UserRepository,
    private val stubIdPublisher: StubIdPublisher,
) {
    companion object {
        fun createUser(
            id: String = UUID.randomUUID().toString(),
            name: String? = null,
            email: String? = null,
            picture: String? = null,
        ): User =
            User(
                id = id,
                name = name ?: nameBuilder(id),
                email = email ?: nameBuilder(id),
                picture = picture ?: nameBuilder(id),
            )

        private fun nameBuilder(id: Any): String = "name-$id"

        private fun emailBuilder(id: Any): String = "$id@gmail.com"

        private fun pictureBuilder(id: Any): String = "http://server/picture-$id"
    }

    fun createPersistedUser(
        id: Any = stubIdPublisher.publishId(),
        name: String? = null,
        email: String? = null,
        picture: String? = null,
    ) = userRepository
        .save(
            UserEntity.of(createUser(id.toString(), name, email, picture)),
        ).toDomain()

    fun createPersistedUsers(count: Long): List<User> = (1..count).map { createPersistedUser(id = it) }

    fun createPersistedUsers(vararg ids: Any): List<User> = ids.map { createPersistedUser(it.toString()) }
}
  • 어그리게이션에 대한 많은 도메인들이 모두 각각의 fixture로 등록되면 코드가 복잡해질 것 같아서 이를 하나로 묶어주었다.
    • 또 여기서 이 어그리케이션에 대해 하나의 데이터셋이 초기화 되어있어야, 전체 데이터가 조회되지는 않는지 테스트 해볼 수 있을 것 같아서 하나의 데이터셋을 만들어줄 수 있는 setup 메서드를 따로 만들어 주었다.
@Component
class WorkspaceFixtureFacade(
    val chatRoom: ChatRoomFixture,
    val chatRoomUser: ChatRoomUserFixture,
    val workspace: WorkspaceFixture,
    val workspaceUser: WorkspaceUserFixture,
    val user: UserFixture,
) {
    fun setUp() {
        val leader: User = user.createPersistedUser(name = "김리더", email = "[email protected]")
        val workspace: Workspace = workspace.createPersistedWorkspace(name = "테스트 워크스페이스")
        val members: List<User> = user.createPersistedUsers(10)

        val workspaceUsers: List<WorkspaceUser> =
            workspaceUser.createPersistedWorkspaceUser(
                workspaceId = workspace.id,
                leaderId = leader.id,
                memberIds = members.map { it.id },
            )

        val chatRooms: List<ChatRoom> =
            chatRoom.testChatRoomSetUp(count = 10, workspace = workspace)

        chatRooms.forEach { chatRoom ->
            chatRoomUser.createPersistedChatRoomUser(
                chatRoomId = chatRoom.id,
                userIds = workspaceUsers.map { it.userId },
            )
        }
    }
}
  • 실제 사용하는 부분에서는 아직 고민이 더 필요할 것 같다.
    • 현재는 수동적으로 각각의 도메인에 대해 수동적으로 세팅해주고 있다.
      • 테스트는 명시적으로 given이 보여지는 것이 좋다고 생각하여 이렇게 구성하였다.
      • 하지만 테스트 규모가 커지면 이렇게 처리하기 힘들 것이라 생각하여 추후 방향성은 아직 고민중이다.
@DisplayName("WorkspaceFacadeService 테스트")
@Transactional
@SpringBootTest
class WorkspaceFacadeServiceTest {
    @Autowired
    lateinit var workspaceFacadeService: WorkspaceFacadeService

    @Autowired
    lateinit var fixtures: WorkspaceFixtureFacade

    @Test
    @DisplayName("유저의 워크스페이스들 조회")
    fun findWorkspaces() {
        // given
        val userId = "tester"
        val user: User = fixtures.user.createPersistedUser(userId)
        val workspace1 = fixtures.workspace.createPersistedWorkspace(name = "워크스페이스1")
        fixtures.workspaceUser.createPersistedWorkspaceUser(workspaceId = workspace1.id, leaderId = userId)
        val workspace2 = fixtures.workspace.createPersistedWorkspace(name = "워크스페이스2")
        fixtures.workspaceUser.createPersistedWorkspaceUser(workspaceId = workspace2.id, leaderId = userId)
        val workspace3 = fixtures.workspace.createPersistedWorkspace(name = "워크스페이스3")
        fixtures.workspaceUser.createPersistedWorkspaceUser(workspaceId = workspace3.id, leaderId = userId)
        val workspace4 = fixtures.workspace.createPersistedWorkspace(name = "워크스페이스4")
        fixtures.workspaceUser.createPersistedWorkspaceUser(workspaceId = workspace4.id, leaderId = userId)

        // when
        val result = workspaceFacadeService.findWorkspaces(userId)

        // then
        assertThat(result.userId).isEqualTo(userId)
        assertThat(result.workspaces.size).isEqualTo(4)
    }
    //...
}

Link

BG-200

@@ -13,8 +14,9 @@ data class GoogleUserInfoDto(
val givenName: String,
val picture: String,
) {
fun toDomain(): User =
fun toDomain(idPublisher: IdPublisher): User =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우리 유저쪽 ID어떻게 될지 모르니까 일단 추상화 시켜놨는데 어떨까??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 좋아!!! 👍

Comment on lines +8 to +10
class UUIDIdPublisher : IdPublisher {
override fun publishId(): String = UUID.randomUUID().toString()
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금은 그 유저 식별자 퍼블리싱하는 거 기본 구현체로 UUID로 발행하는데 나중에 정해지면 여기서 발행하면 될 것 같아서

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋아좋아 다음에 쉽게 바꿀수 있을듯
근데 우리가 사용하는 IdPublisher가 유저 하나만 있을까 싶기도 한데
그럼 팩토리 패턴? 을 사용해야 될까싶네

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

으음... 일단 오케 일단은 우리가 직접 설계한 키를 사용하는 도메인이 user 밖에 없으니까 간단하게 이렇게 처리하긴 했는데

지금 당장하기보단 키 설계를 해야하는 다른 도메인 개발이 들어갈 때 처리하면 되지 않을까??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋아좋아!! 👍

@Dltmd202 Dltmd202 requested a review from GGHDMS July 1, 2024 14:42
Copy link
Member Author

@Dltmd202 Dltmd202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

첫 번째 커밋 e368e6e890f519ea5060066f4bc8af8283012138 이건 유저 식별자 String으로 변환하는 부분인데 이게 대부분 fileChanged 차지하는거라 그거 제외하고 봐주는게 좋을 것 같아!

Copy link
Member

@GGHDMS GGHDMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@GGHDMS GGHDMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@GGHDMS GGHDMS merged commit 03e8b08 into develop Jul 2, 2024
1 check passed
@GGHDMS GGHDMS deleted the feature/BG-200-test-fixutre-refactor branch July 2, 2024 06:10
@Dltmd202 Dltmd202 self-assigned this Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants