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

test: TestContainer 적용 #27

Merged
merged 3 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,14 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'com.mysql:mysql-connector-j'
compileOnly 'org.projectlombok:lombok'
runtimeOnly 'com.h2database:h2'
runtimeOnly 'com.mysql:mysql-connector-j'
annotationProcessor 'org.projectlombok:lombok'
testImplementation 'org.testcontainers:testcontainers:1.19.3'
testImplementation 'org.testcontainers:junit-jupiter:1.19.3'
testImplementation 'org.testcontainers:mysql'
testImplementation 'io.rest-assured:rest-assured:5.3.2'
testImplementation 'org.springframework.boot:spring-boot-starter-test'
}
Expand Down
4 changes: 4 additions & 0 deletions src/main/resources/application-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
spring:
jpa:
hibernate:
ddl-auto: create
55 changes: 55 additions & 0 deletions src/test/java/in/koreatech/koin/AcceptanceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package in.koreatech.koin;

import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;

import in.koreatech.koin.support.DBInitializer;
import io.restassured.RestAssured;
import org.junit.jupiter.api.BeforeEach;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.context.annotation.Import;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.testcontainers.containers.MySQLContainer;
import org.testcontainers.junit.jupiter.Container;

@SpringBootTest(webEnvironment = RANDOM_PORT)
@Import(DBInitializer.class)
@ActiveProfiles("test")
public abstract class AcceptanceTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

설정을 클래스로 따로 분리해서, 인수 테스트를 생성할 때는 해당 클래스를 상속만 받으면 되니까 편리하고 보기 좋은 것 같습니다!

이름은 왜 Acceptance인가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

인수테스트에 해당 클래스를 상속받게 만들고자했습니다~!
용어는 인수 검사(acceptance testing)에서 따왔습니다

참고: https://ko.wikipedia.org/wiki/%EC%9D%B8%EC%88%98_%EA%B2%80%EC%82%AC


private static final String ROOT = "test";
private static final String ROOT_PASSWORD = "1234";
Comment on lines +23 to +24
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.yml과 같은 설정 파일에 분리할 수 있을 것 같은데, 따로 분리하지 않은 이유가 있을까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

위에서 한수에게 답변한 내용과 동일합니다~

charset 수정을 해당 부분에서 하는 게 맞는지 (schema.sql 에서도 될 것 같고 application.yml에서도 가능할 것 같아요)

해당 MySQL 컨테이너를 띄우는 부분은 오로지 testContainer에서 static하게 단 한번만 사용하므로 전역적으로 yml 파일로 선언하기보다는 작성해주신 대로 해당 코드에서만 사용하는게 어떨까 생각이 듭니다.

별도의 config 파일로 분리할 정도로 중요한 내용이 아니라고 생각되고, 반복되지 않는 작업을 불필요하게 분리하여 관리 요소가 늘어나는 것 같다 라는 의견입니다.

Comment on lines +23 to +24
Copy link
Collaborator

Choose a reason for hiding this comment

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

설정이 test, 1234로 되어 있는데 아무거나 넣어도 되는 건가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

넹 아무거나 넣어도 됩니다~!
테스트용이라 아무거나 넣었어요


@LocalServerPort
protected int port;

@Autowired
private DBInitializer dataInitializer;

@Container
protected static MySQLContainer container;

@DynamicPropertySource
private 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);
}

static {
container = new MySQLContainer("mysql:8")
Copy link
Collaborator

@Invidam Invidam Dec 13, 2023

Choose a reason for hiding this comment

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

Suggested change
container = new MySQLContainer("mysql:8")
container = (MySQLContainer) new MySQLContainer("mysql:5.7.34")
.withDatabaseName("test")
.withUsername(ROOT)
.withPassword(ROOT_PASSWORD)
.withCommand("--character-set-server=utf8mb4", "--collation-server=utf8mb4_unicode_ci")
;
container.start();
  • Production에서는 5.7.33 버전을 사용중이어서 비슷한 버전으로 다운그레이드 하였구 5.7 버전대는 8.0과 Character Set이 달라, 한글 입력 관련 에러가 터져 수정하는 코드까지 추가해봤습니당
    • (SQLException: Incorrect string value: '\xED\x95\x9C\xEC\x9A\xB0...' for column 'name' at row 1에러가 떴었음.)
  • .withCommand() 이용해서 char set 수정했고, 해당 빌더 메서드 이용시 GenericContainer로 반환타입이 변환되어 다시 (MysqlContainer)로 변환했습니다

추가로

  • charset 수정을 해당 부분에서 하는 게 맞는지 (schema.sql 에서도 될 것 같고 application.yml에서도 가능할 것 같아요)
  • mysql 5.7은 arm(맥기반)이 제공되지 않아 amd 환경의 도커 컨테이너를 이용해야 했는데 괜찮은지 (속도 측면)
  • 윈도우는 도커 사용하기 힘든 환경이라고 알고 있는데 사용할 수 있을지 (도커 사용시 메모리 누수가 되었던 경험이 있음)

고민해봐야할 것 같아요 (윈도우인 분들의 테스트가 필요할 것 같네요!!)

Copy link
Member Author

Choose a reason for hiding this comment

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

Production MySQL 버전을 미처 신경쓰지 못했네요!!
좋은 지적 감사합니다 👍

추가로 말씀해주신 내용에 대한 저의 의견을 말씀드려봅니다.

charset 수정을 해당 부분에서 하는 게 맞는지 (schema.sql 에서도 될 것 같고 application.yml에서도 가능할 것 같아요)

해당 MySQL 컨테이너를 띄우는 부분은 오로지 testContainer에서 static하게 단 한번만 사용하므로 전역적으로 yml 파일로 선언하기보다는 작성해주신 대로 해당 코드에서만 사용하는게 어떨까 생각이 듭니다.

별도의 config 파일로 분리할 정도로 중요한 내용이 아니라고 생각되고, 반복되지 않는 작업을 불필요하게 분리하여 관리 요소가 늘어나는 것 같다 라는 의견입니다.

mysql 5.7은 arm(맥기반)이 제공되지 않아 amd 환경의 도커 컨테이너를 이용해야 했는데 괜찮은지 (속도 측면)

속도 측면에서 어떤 부분이 걱정인것인지 정확이 알기 어렵습니다.

5.7을 사용함에 따라 테스트 속도가 느려지는것을 걱정하시는 걸까요? 아니면 컨테이너 로드하는 시간에 대한 걱정일까요?
정확히 얼마나 유의미한 속도차이가 있을지 잘 모르겠네요.

정말 크리티컬하게 속도차이가 나는것이 아니라면 production 환경과 맞춰야하므로 5.7 버전을 사용해야한다 라는 이유가 더 중요하다고 판단되어 트레이드오프 해야하는 부분이라고 생각됩니다.

윈도우는 도커 사용하기 힘든 환경이라고 알고 있는데 사용할 수 있을지 (도커 사용시 메모리 누수가 되었던 경험이 있음)

window docker 환경에서의 메모리 누수에 대한 이슈가 있기도 하군요

실제 운영 환경은 Linux 환경이라 해당 부분에 대한 이슈는 없을 것이라고 예상됩니다.

개발자의 로컬 환경에 대한 별도의 처리를 코드레벨에서 고려해야할지는 고민인 부분입니다.
Mac 환경으로 개발하고 있는 상황에서 실제로 경험해보지 못한 부분이기도 하고 정확한 원인을 파악하지 못해 윈도우 사용자의 코멘트가 필요할 것 같네요

해당 의견에 대해서는 직접 사용해본 후 문제가 발생하면 그때 다시 논의해보면 좋을 것 같습니다.
cc. @daheeParkk

Copy link
Collaborator

@Invidam Invidam Dec 17, 2023

Choose a reason for hiding this comment

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

  1. charset은 말씀했던대로 TestContainer 레벨에서 처리하는 게 맞을 것 같아요!
    • 실제 프로덕션 환경에 이미 적용이 되어있고, 테스트 환경에서만 적용하면 되기 때문에
  2. 5.7에 맞추는 게 더 중요할 것 같고, "칩셋(arm, amd) 차이로 인해 맥 이용시 실행 속도가 느려지거나 동작이 미세하기 다를 수 있다"는 점만 다같이 인지하고 있음 될 것 같아요
  3. 테스트 컨테이너가 짧은 시간동안만 실행되고 종료된다는 점을 생각하면 메모리 문제도 그리 심각하지 않을 수 있을 것 같아요!

직접 사용해본 후 문제가 발생하면 그때 다시 논의해보면 좋을 것 같습니다.

👍👍

.withDatabaseName("test")
.withUsername(ROOT)
.withPassword(ROOT_PASSWORD);
container.start();
}

@BeforeEach
void delete() {
dataInitializer.clear();
RestAssured.port = port;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
RestAssured.port = port;
if (RestAssured.port == RestAssured.UNDEFINED_PORT) {
RestAssured.port = port;
}

로 UNDEFINED인 경우에만 넣어주는 건 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

오! 좋습니다~!

}
}
23 changes: 4 additions & 19 deletions src/test/java/in/koreatech/koin/acceptance/TrackApiTest.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package in.koreatech.koin.acceptance;

import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
import static org.springframework.test.annotation.DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD;

import in.koreatech.koin.AcceptanceTest;
import in.koreatech.koin.domain.Member;
import in.koreatech.koin.domain.TechStack;
import in.koreatech.koin.domain.Track;
Expand All @@ -14,21 +12,12 @@
import io.restassured.response.Response;
import java.time.format.DateTimeFormatter;
import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.http.HttpStatus;
import org.springframework.test.annotation.DirtiesContext;

@SpringBootTest(webEnvironment = RANDOM_PORT)
@DirtiesContext(classMode = BEFORE_EACH_TEST_METHOD)
class TrackApiTest {

@LocalServerPort
int port;
class TrackApiTest extends AcceptanceTest {

@Autowired
private TrackRepository trackRepository;
Expand All @@ -39,11 +28,6 @@ class TrackApiTest {
@Autowired
private MemberRepository memberRepository;

@BeforeEach
void setUp() {
RestAssured.port = port;
}

@Test
@DisplayName("BCSDLab 트랙 정보를 조회한다")
void findTracks() {
Expand Down Expand Up @@ -119,7 +103,8 @@ void findTrack() {
softly.assertThat(response.body().jsonPath().getString("TrackName")).isEqualTo(track.getName());

softly.assertThat(response.body().jsonPath().getList("Members")).hasSize(1);
softly.assertThat(response.body().jsonPath().getInt("Members[0].id")).isEqualTo(member.getId());
softly.assertThat(response.body().jsonPath().getInt("Members[0].id"))
.isEqualTo(member.getId().longValue());
softly.assertThat(response.body().jsonPath().getString("Members[0].name")).isEqualTo(member.getName());
softly.assertThat(response.body().jsonPath().getString("Members[0].student_number"))
.isEqualTo(member.getStudentNumber());
Expand Down
61 changes: 61 additions & 0 deletions src/test/java/in/koreatech/koin/support/DBInitializer.java
Copy link
Collaborator

@Invidam Invidam Dec 14, 2023

Choose a reason for hiding this comment

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

https://github.com/ejoongseok/product-order-service/blob/master/src/test/java/com/example/productorderservice/DatabaseCleanup.java
(케이포타운 개발자분의 강의에서 가져온 코드)
비슷하게 DB를 초기화(클린업)하는 코드인데

  1. JPA Entity로 사용중인 테이블들만 Truncate
  2. 참조 무결성 옵션을 껐다 켜줘서 FK도 제거 가능하도록 설정

의 차이가 있더라구요. (1) 같은 경우는 모든 테이블을 제거하는 것보다 JPA에서 사용한 테이블만 제거하는 것이 성능에 있어 뛰어날 것 같은데 어떻게 생각하세요? (테스트가 필요할까요?)

참고로 강의는 링크

Copy link
Member Author

@Choi-JJunho Choi-JJunho Dec 16, 2023

Choose a reason for hiding this comment

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

좋은 의견이네요!
몇가지 의문점이 있어 코멘트 남겨봅니다.

1. 현재 설계상으로 JPA Entity로 사용중이지 않은 테이블이 있을까요?

사례가 잘 와닿지 않아 해당 방식으로 코드를 구성하는 방향에 대한 의문이 듭니다.

2. 엔티티 이름과 실제 테이블명이 다른 경우

final List<String> entityNames = entities.stream()
        .filter(e -> isEntity(e) && !hasTableAnnotation(e))
        .map(e -> CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_UNDERSCORE, e.getName()))
        .toList();

위 코드를 보면 특정 @Entity, @Table 어노테이션이 붙어있는 객체를 가져와 그 객체의 이름을 snake_case로 변환하는 것을 확인 할 수 있습니다.

하지만 해당 방식의 경우 객체 명과 실제 테이블명이 다른 상황에 대한 대처가 어렵다고 생각합니다.

// 예시
@Entity
@Table(name = "students")
public class Student {
// ...
}

3. 성능 개선이 필요할만큼 느린가?

현재 얼마만큼의 속도가 책정되고 저 방식을 적용한다면 얼마만큼의 성능이 개선될지 의문인 상황입니다.

현재로서는 성능 개선이 필요할만큼 속도가 느리다고 생각되지 않는 시점이라 다소 고려를 많이 해보고있다는 부분 양해부탁드립니다. 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 지금이나 앞으로나 설계상 JPA Entity로 사용하지 않는 테이블이 떠오르진 않네요
    • 테스트 환경에 대한 이해가 부족해서 실제 서버 환경과 헷갈렸던 것 같아요
    • 따라서 크게 변경할 필요는 없어보이네요!
      • 적용하기 위해선 2번 문제[테이블 명이 다른 경우]도 해결해주어야 하고..
  2. 불필요하게 Truncate되었던 테이블의 수만큼 성능이 향상될 것 같은데, 그런 테이블이 없다면 성능 향상은 미미할 것 같네요

Copy link
Collaborator

Choose a reason for hiding this comment

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

추가로 링크에서 외래키 참조 무결성을 해제했던 쿼리 SET REFERENTIAL_INTEGRITY FALSE은 h2, oracle에서만 가능한 것 같네용..

참고 링크

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package in.koreatech.koin.support;

import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.List;
import javax.sql.DataSource;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.TestComponent;
import org.springframework.transaction.annotation.Transactional;

@TestComponent
public class DBInitializer {

private static final int OFF = 0;
private static final int ON = 1;
private static final int COLUMN_INDEX = 1;

private final List<String> tableNames = new ArrayList<>();

@Autowired
private DataSource dataSource;

@PersistenceContext
private EntityManager entityManager;

private void findDatabaseTableNames() {
try (final Statement statement = dataSource.getConnection().createStatement()) {
ResultSet resultSet = statement.executeQuery("SHOW TABLES");
while (resultSet.next()) {
final String tableName = resultSet.getString(COLUMN_INDEX);
tableNames.add(tableName);
}
} catch (Exception e) {
e.printStackTrace();
}
}

private void truncate() {
setForeignKeyCheck(OFF);
for (String tableName : tableNames) {
entityManager.createNativeQuery(String.format("TRUNCATE TABLE %s", tableName)).executeUpdate();
}
setForeignKeyCheck(ON);
}

private void setForeignKeyCheck(int mode) {
entityManager.createNativeQuery(String.format("SET FOREIGN_KEY_CHECKS = %d", mode)).executeUpdate();
}

@Transactional
public void clear() {
if (tableNames.isEmpty()) {
findDatabaseTableNames();
}
entityManager.clear();
truncate();
}
}
Loading