-
Notifications
You must be signed in to change notification settings - Fork 8
chore: ci 스크립트 작성 및 ci 통과를 위한 코드 수정 #266
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
chore: ci 스크립트 작성 및 ci 통과를 위한 코드 수정 #266
Conversation
- create-drop을 create로 변경 - 테스트 컨테이너를 실행해서 테스트하기 때문에, 테스트가 종료되면 당연히 테이블이 삭제된다. 따라서 create-drop을 하는것과 create를 하는 것이 동일하게 동작한다. 굳이 create로 바꾼 것은 삭제 시 FK 위반으로 WARN 로그가 불필요하게 백몇줄이 찍히는걸 막기 위해서.
- 깨지는 테스트 수정 - 비지니스 로직 변경으로 더 이상 해당되지 않는 테스트 삭제
Walkthrough
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/resources/application.yml (1)
1-74: 테스트용 application.yml 파일 추가 확인 ✅테스트를 위한 별도의 설정 파일을 추가한 것은 좋은 접근입니다. 몇 가지 제안 사항이 있습니다:
- Redis 및 데이터베이스 설정이 잘 구성되어 있습니다.
- OAuth 설정이 테스트용으로 적절히 구성되었습니다.
- AWS 및 기타 외부 서비스 설정도 테스트 환경에 맞게 적절히 설정되었습니다.
- secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA== # base64 encoded 된 임의의 값 + secret-key: ${APPLE_SECRET_KEY:MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA==} # 환경 변수 또는 기본값(base64 encoded 임의 값)테스트 설정에 하드코딩된 값 대신 환경 변수를 사용하는 패턴을 적용하면 더 유연한 구성이 가능합니다. 필요에 따라 환경 변수로 오버라이드할 수 있으면서도 기본값을 제공하는 방식입니다.
🧰 Tools
🪛 Gitleaks (8.21.2)
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
.github/workflows/ci.yml (1)
1-38: Gradle CI 파이프라인 설정 완료
- Gradle 빌드 및 테스트를 자동화하여 PR마다 안전성을 확보했습니다.
- 추가로 Gradle 캐시 설정을 활용하면 빌드 시간을 더 단축할 수 있겠습니다.
- 모든 단계를 명확히 분리해 디버깅과 유지보수가 용이합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/ci.yml(1 hunks)src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java(3 hunks)src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java(1 hunks)src/main/java/com/example/solidconnection/entity/common/BaseEntity.java(2 hunks)src/test/java/com/example/solidconnection/database/DatabaseConnectionTest.java(0 hunks)src/test/java/com/example/solidconnection/database/RedisConnectionTest.java(0 hunks)src/test/java/com/example/solidconnection/e2e/ApplicantsQueryTest.java(4 hunks)src/test/java/com/example/solidconnection/support/DatabaseCleaner.java(0 hunks)src/test/java/com/example/solidconnection/support/RedisTestContainer.java(1 hunks)src/test/java/com/example/solidconnection/support/TestContainerDataJpaTest.java(0 hunks)src/test/java/com/example/solidconnection/support/TestContainerSpringBootTest.java(2 hunks)src/test/resources/application.yml(1 hunks)
💤 Files with no reviewable changes (4)
- src/test/java/com/example/solidconnection/database/RedisConnectionTest.java
- src/test/java/com/example/solidconnection/database/DatabaseConnectionTest.java
- src/test/java/com/example/solidconnection/support/TestContainerDataJpaTest.java
- src/test/java/com/example/solidconnection/support/DatabaseCleaner.java
🧰 Additional context used
🪛 Gitleaks (8.21.2)
src/test/resources/application.yml
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (12)
src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java (1)
13-14: Apple OAuth 속성에 secretKey 필드 추가 확인 👍Apple OAuth 설정에 secretKey 필드를 추가한 변경사항이 적절합니다. 이 변경으로 인해 p8 파일에서 직접 키를 읽어오는 대신 설정 파일을 통해 키를 관리할 수 있게 되었네요.
src/main/java/com/example/solidconnection/entity/common/BaseEntity.java (3)
14-15: 정적 임포트 추가 적절함 👍ZoneOffset.UTC와 ChronoUnit.MICROS의 정적 임포트 추가가 적절합니다. 이를 통해 코드가 더 간결해지고 읽기 쉬워졌습니다.
29-29: ZonedDateTime 생성 시 마이크로초 단위 정밀도 적용 👏ZonedDateTime을 마이크로초 단위로 절삭하는 변경사항은 매우 적절합니다. 이로 인해 애플리케이션에서 초기화된 시간과 데이터베이스에서 검색된 시간 간의 일관성이 유지될 수 있습니다.
주석에 설명된 대로 나노초 6자리까지만 저장함으로써 MySQL과의 호환성 문제를 해결하는 좋은 접근 방법입니다.
35-35: updatedAt 필드도 동일한 정밀도 적용 👍onPreUpdate 메서드에서도 동일하게 마이크로초 단위의 정밀도를 적용하여 일관성을 유지한 점이 좋습니다.
src/test/java/com/example/solidconnection/support/TestContainerSpringBootTest.java (2)
7-7: Redis 테스트 컨테이너 설정 개선 ✅ContextConfiguration 어노테이션을 추가하고 RedisTestContainer를 초기화 클래스로 지정한 변경은 좋은 접근입니다. 이를 통해 테스트 환경에서 Redis 설정을 보다 명확하게 제어할 수 있습니다.
Also applies to: 16-16
20-20: @import 어노테이션 수정 적절함 👍@import 어노테이션에서 RedisTestContainer를 제거하고 MySQLTestContainer만 포함하도록 변경한 것은 적절합니다. 이제 RedisTestContainer는 ContextConfiguration을 통해 초기화되므로 중복 설정을 방지할 수 있습니다.
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java (2)
13-15: 이식성 높은 예외 클래스 사용으로 명확성 향상
NoSuchAlgorithmException과InvalidKeySpecException을 명확히 import하여 필요 예외만 처리하고 있습니다.- 다른 범위의 예외가 발생할 경우를 대비해 추가 로깅 또는 후속 처리를 고려해보면 좋겠습니다.
56-63: 사설키 주입 로직의 간소화 및 접근성 개선
- Base64 디코딩 후
PKCS8EncodedKeySpec에 직접 연결하여 로직이 간결합니다.- 예외 범위가 구체적으로 한정되어 디버깅이 수월해집니다.
- 필요하다면 예외 메시지를 조금 더 구체적으로 전달해 디버그 시간을 줄일 수 있습니다.
src/test/java/com/example/solidconnection/e2e/ApplicantsQueryTest.java (3)
108-139: 경쟁자 조회 엔드포인트 변경에 따른 테스트 보강
/applications에서/applications/competitors로 수정된 내용을 반영해 실제 서비스 의도에 맞게 테스트가 보완되었습니다.- 린츠 대학 정보에 빈 배열을 확인하는 로직이 잘 정의되어, 선택하지 않은 대학의 검증이 명확해졌습니다.
148-167: 지역 필터링 테스트의 엔드포인트 변경
region쿼리 파라미터를 통해 영미권 코드만 필터링하도록 수정되어, 로직이 선명해졌습니다.- 깔끔하게 메서드 체인으로 RestAssured를 활용해 결과를 검증하고 있어 유지보수성도 좋아 보입니다.
175-175: 이전 학기 지원자 미포함 검증
/competitors엔드포인트에서 이전 학기 지원자를 제외함으로써 정확한 결과를 유도합니다.- 필요한 경우 실제 DB에서도 이전 학기 데이터가 유효한지 한번 더 점검해 안전성을 높일 수 있습니다.
src/test/java/com/example/solidconnection/support/RedisTestContainer.java (1)
3-23: 프로그램 방식의 Redis 테스트 컨테이너 초기화
ApplicationContextInitializer활용으로 컨테이너가 자동 기동되어 환경설정이 직관적으로 합쳐집니다.TestPropertyValues.of(...)로 테스트 속성을 바로 주입해, 복잡했던 설정을 깔끔하게 정리했습니다.- 테스트 컨테이너 버전을 주기적으로 갱신해 호환성과 보안성을 유지하면 더욱 좋습니다.
Gyuhyeok99
left a comment
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.
고생 많으셨습니다! 트러블슈팅 정리해주신 거 덕분에 과정이 더 잘 이해됐습니다 😊
| jpa: | ||
| hibernate: | ||
| ddl-auto: create-drop | ||
| ddl-auto: create |
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.
테스트 환경에선 차이가 없는 거 같은데 왜 바뀐걸까 하고 여쭤보려 들어왔는데 설명이 써있었네요 😅
| key-id: key-id | ||
| redirect-url: "https://localhost:8080/auth/apple" | ||
| secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA== | ||
| secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA== # base64 encoded 된 임의의 값 |
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.
실제 키처럼 보이는 거엔 주석 표시하는 거 좋은 거 같습니다!
근데 이것만 실제 키처럼 따로 설정을 해주신 이유가 있을까요?
추가로 이렇게 키처럼 보이면 깃허브에서 따로 경고 안주나요? 이건 그냥 궁금해서 여쭤봅니다. 😅
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.
| @Override | ||
| public void initialize(ConfigurableApplicationContext applicationContext) { |
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.
이렇게 변경하면서 redis와 mysql testcontainers 구현 방식이 달라졌는데 통일하는 건 어떨까요?
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.
굳이 다르게 설정할 필요는 없겠네요.
반영했습니다! 🫡
|
추가로, 바뀐 code rabbit은 어떤 것 같은지도 알려주시면 감사하겠습니다🌻 |
- redis 테스트 컨테이너와 동일하게 설정하여 불필요한 인지 부하를 줄인다.
bfa3b90 to
c364691
Compare
원래는 영어로만 나오고 엄청 길었던 것 같은데 보기 좋게 개선된 것 같아요! |
| team-id: team-id | ||
| key-id: key-id | ||
| redirect-url: "https://localhost:8080/auth/apple" | ||
| secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA== # base64 encoded 된 임의의 값 |
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.
이 값은 디코드해도 의미없는 값이 나오는데 혹시 어떻게 생성하신건가요? 단순 호기심입니다!
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.
사실 지피티한테 만드는 방법 물어봤씁니다 🤭
소스코드는 이러합니다!
try {
KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC");
kpg.initialize(new ECGenParameterSpec("secp256r1"), new SecureRandom());
KeyPair kp = kpg.generateKeyPair();
PrivateKey privKey = kp.getPrivate();
String base64Encoded = Base64.getEncoder().encodeToString(privKey.getEncoded());
System.out.println("Base64 Encoded EC Private Key:");
System.out.println(base64Encoded);
} catch (Exception e) {
e.printStackTrace();
}|
저도 지금 설정이 더 보기 좋은 거 같아요! |

관련 이슈
작업 내용
원래 목적은 ci 스크립트 작성이었는데,…
ci 스크립트를 작성하고, 테스트들을 다 통과시키기 위해서는 세트로 발생하는 문제들이 꽤나 많았습니다🥲
❗️ PR 디스크립션과 커밋 따라가시면 파악하기 쉬우실겁니다! ❗️
테스트 코드가 main/secret/AppleOAuthKey.p8 파일에 접근하지 못해서 테스트가 실패하는 문제가 있었는데,
동일하게 test/secret/ 경로에 AppleOAuthKey.p8 파일을 만들기보다, 다른 변수들처럼 yml 로 관리하는게 좋을 것 같아 바꿔줬습니다.
기존 설정이 잘못되어있더라고요🥲
이걸 왜 지금 알게 되었는지와, 어떻게 다시 설정했는지를 디스커션에 정리했습니다.
스프링의 환경변수인 Environment 에 대한 내용이 있으니 한번 읽어보셔도 도움될 것 같습니다.
이를 MySQL DB에 넣었다가 꺼낸 상태의 ZonedDateTime을 비교하는 과정에서 문제가 발생했습니다.
디스커션에 문제 원인 정리했습니다.
‘지원자는 자신이 지원한 학교의 경쟁자들만 볼 수 있다’는 내용이 e2e 테스트에는 반영이 안되었더라고요.
이로써 e2e 테스트의 또 다른 단점을 발견하게 되었습니다..😮💨
e2e 테스트 코드 지우는 PR 새로 만들겠습니다.
특이 사항