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

기존 테스트 환경 TestContainers로 리팩토링 #434

Merged
merged 11 commits into from
Sep 7, 2023

Conversation

nuyh99
Copy link
Collaborator

@nuyh99 nuyh99 commented Sep 5, 2023

#️⃣ 연관된 이슈

📝 작업 내용

  • Test Containers로 테스트 환경 분리
  • DB 초기화용 DataInitializer 구현
  • 서비스, 리포지토리 레이어 테스트용 BaseTest 구현

@nuyh99 nuyh99 added the BE 개발은 백이징 label Sep 5, 2023
@nuyh99 nuyh99 self-assigned this Sep 5, 2023
@nuyh99 nuyh99 changed the title Refactor/426 testcontainers 기존 테스트 환경 TestContainers로 리팩토링 Sep 5, 2023
@nuyh99 nuyh99 closed this Sep 5, 2023
@nuyh99 nuyh99 reopened this Sep 5, 2023
Copy link
Collaborator

@green-kong green-kong left a comment

Choose a reason for hiding this comment

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

코드 맛있게 읽었습니다!
몇가지 궁금한점 남겨놨는데 확인해주세요~

@@ -82,4 +82,4 @@ create table if not exists `yozm-cafe`.menu_board (
CONSTRAINT menu_board_cafe_id
foreign key (cafe_id) references cafe (id)
ON DELETE CASCADE
);
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

저기.. 선생님 죄송하지만 여기 개행하나만 추가해주시겠어요...? flywayPR merge되면서 개행이 사라진 듯 하네요 ㅠ

Comment on lines +12 to +16
/**
* Application Context들이 필요할 때 상속하면 됩니다.
* Repository, Service 레이어 통합 테스트용으로 쓰면 됩니다.
*/
@SpringBootTest
Copy link
Collaborator

Choose a reason for hiding this comment

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

감사합니다. 잘먹겠습니다.

Comment on lines 32 to 38
@DynamicPropertySource
static void configureProperties(final DynamicPropertyRegistry registry) {
registry.add("spring.datasource.url", container::getJdbcUrl);
registry.add("spring.datasource.username", () -> "root");
registry.add("spring.datasource.password", () -> "test");
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 방식으로도 properties를 동적으로 매핑해줄수가 있네요!
음 근데 궁금한게 있습니다.
datasource.url 같은 경우는 테스트컨테이너가 매번 랜덤한 포트를 사용하기에 이렇게 동적으로 바인딩 해주는 것이 이해가 되는데,
usernamepassword도 동적으로 매핑해줄 필요가 있나요??

Copy link
Collaborator

Choose a reason for hiding this comment

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

application-test.propertiesusernamepassword 추가해주면 되는거 아닌가 싶어서요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

테스트 컨테이너의 루트 비밀번호를 윗 줄에서 test로 관리하고 있어서 여기에서 매핑을 했습니다!

static {
        container = (MySQLContainer) new MySQLContainer("mysql:8.0")
                .withDatabaseName("yozm-cafe")
                .withEnv("MYSQL_ROOT_PASSWORD", ROOT_PASSWORD)
                .withReuse(true);

        container.start();
    }

    @DynamicPropertySource
    static void configureProperties(final DynamicPropertyRegistry registry) {
        registry.add("spring.datasource.url", container::getJdbcUrl);
        registry.add("spring.datasource.username", () -> ROOT);
        registry.add("spring.datasource.password", () -> ROOT_PASSWORD);
    }

위와 같이 하나의 상수로 관리할 수 있도록 적용했습니다!
properties에 적용해도 되지만 루트 비밀번호를 테스트 컨테이너 생성할 때 설정해줘야 하기 때문에 위와 같이 했습니다.

Comment on lines 39 to 45
@Autowired
private DataInitializer dataInitializer;

@BeforeEach
void delete() {
dataInitializer.deleteAll();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

필드 변수가 왜 여기..?
서로 연관있는 애들끼리 묶어 놓으려했던 의도 같아요. 맞을까요?
근데 delete() method 에서 dataInitializer 가 사용되는 걸보고
엥 이런게 있었나 하면서 파일 최상단으로 스크롤을 올리다가 위 필드변수를 보게되었어요.
혹시 이게 제가 모르는 컨벤션일까요??

아니라면, 관습적으로 사용하는 컨벤션은 따르는게 가독성이 좋지 않을까 싶어요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

헉 프리코스 때보다 못해진 제 자신이네요ㅠㅠ
수정했습니다!

@DataJpaTest
@AutoConfigureTestDatabase(replace = Replace.NONE)
class CafeRepositoryTest {
class CafeRepositoryTest extends BaseTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 되면 RepositoryTest 도 @SpringBootTest로 돌아가겠네요..
delete()를 통해서 테스트 격리는 되겠지만, 테스트 속도에 유의미한 차이는 없던가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

단일 클래스 전체 테스트할 때는 모든 ApplicationContext를 띄우기 때문에 더 느려집니다.
하지만 빌드를 하거나 전체 테스트들을 모두 돌릴 때는 ApplicationContext를 재사용하기 때문에 속도 차이가 없습니다!

단일 클래스 테스트 속도 차이가 크다면, @SpringBootTest 대신 다른 어노테이션을 붙인 새로운 BaseTest를 만들어 쓰면 좋을 것 같습니다.
레이어별로 BaseTest가 생겨서 헷갈릴 수 있을 것 같아서 지금의 구조로 했습니다.
바꾸는 것이 좋을까요??!

Comment on lines +56 to 60
assertSoftly(softAssertions -> {
assertThat(cafes).hasSize(5);
assertThat(cafes).containsExactlyInAnyOrder(cafe1, cafe2, cafe3, cafe4, cafe5);
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

틈새 리팩토링 ㄷㄷ;; 감사합니다.

Comment on lines +56 to +71
private void init() {
try (final Statement statement = dataSource.getConnection().createStatement()) {
final ResultSet resultSet = statement.executeQuery("SHOW TABLES ");

while (resultSet.next()) {
final String tableName = resultSet.getString(FIRST_COLUMN);
if (tableName.contains(FLYWAY)) {
continue;
}

deleteDMLs.add("TRUNCATE " + tableName);
}
} catch (Exception e) {
e.printStackTrace();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분 이해가 잘 안되는데... 내일 직접 대면으로 물어볼게요..!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. DB와 연결된 DataSource를 주입 받아서 사용합니다.
  2. 거기로 SHOW TABLES; SQL을 날려서 ResultSet을 받습니다.
  3. 결과는 테이블 이름들이고, 그 앞에 각각 Truncate 를 붙여서 전체 테이블에 대한 Truncate 문을 만듭니다. 이때 flyway 히스토리 테이블까지 삭제하면 매번 스키마가 적용되므로 warning이 로그에 뜨길래 제외해줬습니다.

위와 같이 한 이유는 지금까지 truncate.sql을 직접 작성해서 사용했는데 테이블을 추가하고 빠트리는 경우가 있어서 그렇습니다.

Copy link
Collaborator

@hum02 hum02 left a comment

Choose a reason for hiding this comment

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

테스트 컨테이너 맛있네요

continue;
}

deleteDMLs.add("TRUNCATE " + tableName);
Copy link
Collaborator

@hum02 hum02 Sep 6, 2023

Choose a reason for hiding this comment

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

오 flyway 테이블을 제외한 테이블들을 조회해와서 truncate를 이렇게 하는군요. 짱이네요

@ActiveProfiles("test")
public abstract class BaseTest {

@Container
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래에서 직접 container.start() 하기에 @Container은 붙이지 않아도 될 것 같아요!
https://java.testcontainers.org/test_framework_integration/junit_5/

container = (MySQLContainer) new MySQLContainer("mysql:8.0")
.withDatabaseName("yozm-cafe")
.withEnv("MYSQL_ROOT_PASSWORD", "test")
.withReuse(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 reuse옵션 ~/.testcontainers.properties파일이 있어야 정상작동할텐데?
하고 로컬에서 실행해보니 신기하게도 아래와 같은 로그를 띄우기는 하는데 컨테이너를 매번 띄우지는 않네요;; 재사용은 정상적으로 되는 듯합니다.

image

https://java.testcontainers.org/features/reuse/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

private final 에서도 재사용하는 옵션인가보군요!
적용되지 않는 거 보고 제거했습니다.

재사용되는 이유는 static 필드로 써서 다시 띄우지 않습니다.

@nuyh99 nuyh99 merged commit 95715a3 into dev Sep 7, 2023
1 check passed
@nuyh99 nuyh99 deleted the refactor/426-testcontainers branch September 7, 2023 04:14
@nuyh99 nuyh99 restored the refactor/426-testcontainers branch September 7, 2023 04:14
@nuyh99 nuyh99 deleted the refactor/426-testcontainers branch September 7, 2023 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 개발은 백이징
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants