-
Notifications
You must be signed in to change notification settings - Fork 1
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
spring: | ||
jpa: | ||
hibernate: | ||
ddl-auto: create |
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 { | ||||||||||||||||||
|
||||||||||||||||||
private static final String ROOT = "test"; | ||||||||||||||||||
private static final String ROOT_PASSWORD = "1234"; | ||||||||||||||||||
Comment on lines
+23
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분은 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 위에서 한수에게 답변한 내용과 동일합니다~
해당 MySQL 컨테이너를 띄우는 부분은 오로지 testContainer에서 static하게 단 한번만 사용하므로 전역적으로 yml 파일로 선언하기보다는 작성해주신 대로 해당 코드에서만 사용하는게 어떨까 생각이 듭니다. 별도의 config 파일로 분리할 정도로 중요한 내용이 아니라고 생각되고, 반복되지 않는 작업을 불필요하게 분리하여 관리 요소가 늘어나는 것 같다 라는 의견입니다.
Comment on lines
+23
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 설정이 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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") | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
추가로
고민해봐야할 것 같아요 (윈도우인 분들의 테스트가 필요할 것 같네요!!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Production MySQL 버전을 미처 신경쓰지 못했네요!! 추가로 말씀해주신 내용에 대한 저의 의견을 말씀드려봅니다.
해당 MySQL 컨테이너를 띄우는 부분은 오로지 testContainer에서 static하게 단 한번만 사용하므로 전역적으로 yml 파일로 선언하기보다는 작성해주신 대로 해당 코드에서만 사용하는게 어떨까 생각이 듭니다. 별도의 config 파일로 분리할 정도로 중요한 내용이 아니라고 생각되고, 반복되지 않는 작업을 불필요하게 분리하여 관리 요소가 늘어나는 것 같다 라는 의견입니다.
속도 측면에서 어떤 부분이 걱정인것인지 정확이 알기 어렵습니다. 5.7을 사용함에 따라 테스트 속도가 느려지는것을 걱정하시는 걸까요? 아니면 컨테이너 로드하는 시간에 대한 걱정일까요? 정말 크리티컬하게 속도차이가 나는것이 아니라면
window docker 환경에서의 메모리 누수에 대한 이슈가 있기도 하군요 실제 운영 환경은 Linux 환경이라 해당 부분에 대한 이슈는 없을 것이라고 예상됩니다. 개발자의 로컬 환경에 대한 별도의 처리를 코드레벨에서 고려해야할지는 고민인 부분입니다. 해당 의견에 대해서는 직접 사용해본 후 문제가 발생하면 그때 다시 논의해보면 좋을 것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
👍👍 |
||||||||||||||||||
.withDatabaseName("test") | ||||||||||||||||||
.withUsername(ROOT) | ||||||||||||||||||
.withPassword(ROOT_PASSWORD); | ||||||||||||||||||
container.start(); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
@BeforeEach | ||||||||||||||||||
void delete() { | ||||||||||||||||||
dataInitializer.clear(); | ||||||||||||||||||
RestAssured.port = port; | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
로 UNDEFINED인 경우에만 넣어주는 건 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오! 좋습니다~! |
||||||||||||||||||
} | ||||||||||||||||||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
의 차이가 있더라구요. (1) 같은 경우는 모든 테이블을 제거하는 것보다 JPA에서 사용한 테이블만 제거하는 것이 성능에 있어 뛰어날 것 같은데 어떻게 생각하세요? (테스트가 필요할까요?) 참고로 강의는 링크 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(name = "students")
public class Student {
// ...
} 3. 성능 개선이 필요할만큼 느린가?현재 얼마만큼의 속도가 책정되고 저 방식을 적용한다면 얼마만큼의 성능이 개선될지 의문인 상황입니다. 현재로서는 성능 개선이 필요할만큼 속도가 느리다고 생각되지 않는 시점이라 다소 고려를 많이 해보고있다는 부분 양해부탁드립니다. 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가로 링크에서 외래키 참조 무결성을 해제했던 쿼리 |
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(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
설정을 클래스로 따로 분리해서, 인수 테스트를 생성할 때는 해당 클래스를 상속만 받으면 되니까 편리하고 보기 좋은 것 같습니다!
이름은 왜
Acceptance
인가요?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인수테스트에 해당 클래스를 상속받게 만들고자했습니다~!
용어는
인수 검사(acceptance testing)
에서 따왔습니다