TDD 실습#1
Conversation
bd501d7 to
2237c4e
Compare
- Add POST /seller/signUp full HTTP integration specs (35 cases) - Implement controller → service → repository flow (TDD RED→GREEN) - Separate Request(web) and Command(application) DTOs - Split exception handlers per type via @ControllerAdvice (SRP) - Organize packages on a single layer axis(domain / application / infrastructure / api)
2237c4e to
75d6a68
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| assertThat(response.getBody()).isNotNull(); | ||
| assertThat(response.getBody().accessToken()).isNotNull(); |
There was a problem hiding this comment.
💡 Suggestion — 시나리오보다 약한 단언 (관점 2)
이 테스트의 이름은 "액세스 토큰을 반환한다"인데 단언은 isNotNull()뿐이라, 서버가 빈 문자열 ""을 돌려줘도 통과합니다. 시나리오가 보장하려는 "실제 토큰 값이 있다"는 계약보다 단언이 약합니다.
| assertThat(response.getBody()).isNotNull(); | |
| assertThat(response.getBody().accessToken()).isNotNull(); | |
| assertThat(response.getBody()).isNotNull(); | |
| assertThat(response.getBody().accessToken()).isNotBlank(); |
JWT 형식은 바로 다음 테스트(액세스_토큰은_JWT_형식을_따른다)가 지키므로, 여기서는 최소 isNotBlank()만으로도 "빈 토큰"이라는 실패를 잡아낼 수 있습니다.
| @ValueSource(strings = { | ||
| "invalid-email", | ||
| "invalid-email@", | ||
| "invalid-email@test", | ||
| "invalid-email@test.", | ||
| "invalid-email@.com" | ||
| }) |
There was a problem hiding this comment.
💡 Suggestion — 반복되는 반례 목록은 공유 소스로 (관점 8)
이 무효 이메일 형식 목록이 세 곳에 중복되어 있습니다.
- 여기(
seller/signup) 인라인@ValueSource shopper/signup/POST_specs.java:88에 바이트 단위로 동일한 인라인@ValueSourceTestDataSource.invalidEmails()(여기엔null까지 포함)
이미 @InvalidEmailSource 라는 공유 소스 애노테이션을 만들어 두고 contactEmail 테스트에는 쓰고 있는데, 정작 email 형식 테스트에서는 두 스펙이 같은 목록을 다시 인라인으로 나열합니다. 이메일 정책이 바뀌면 세 곳을 모두 고쳐야 합니다.
TestDataSource에 null을 뺀 invalidEmailFormats()를 추가하고(미지정 케이스는 별도 테스트가 담당하므로), 이를 감싼 @InvalidEmailFormatSource로 두 signup 스펙이 한 줄로 공유하도록 끌어올리는 방향을 권합니다. 반례가 한곳에 모여 정책 변경 시 한 곳만 고치면 됩니다.
참고: username 목록은
seller/shopper접두사가 달라 동일하지 않으니 그대로 두는 게 맞습니다 — 인라인@ValueSource가 더 읽기 좋습니다.
| LocalDateTime referenceTime = LocalDateTime.now(UTC); | ||
| fixture.registerProduct(); | ||
|
|
||
| // Act | ||
| ResponseEntity<ArrayCarrier<SellerProductView>> response = | ||
| fixture.client().exchange( | ||
| get("/seller/products").build(), | ||
| new ParameterizedTypeReference<>() { } | ||
| ); | ||
|
|
||
| // Assert | ||
| SellerProductView actual = requireNonNull(response.getBody()).items()[0]; | ||
| assertThat(actual.registeredTimeUtc()) | ||
| .isCloseTo(referenceTime, within(1, SECONDS)); |
There was a problem hiding this comment.
📝 Tech Debt — 실제 현재시각 의존 (관점 7)
LocalDateTime.now(UTC)를 기준값으로 잡고 within(1, SECONDS)로 비교합니다. 기준값 캡처 → HTTP 왕복 → 서버가 등록 시각 부여 사이의 지연이 1초를 넘으면(콜드 스타트/느린 CI/컨텍스트 워밍업) 플래키하게 실패할 수 있습니다. products/id/GET_specs.java:164 도 동일 패턴입니다.
당장 막을 이슈는 아니지만, 다음 중 하나를 권합니다.
- 프로덕션에
Clock을 주입 가능하게 하고 테스트에서 고정 시계 사용(가장 견고, 프로덕션 변경 필요) - 최소한 허용 오차를
within(10, SECONDS)수준으로 넓혀 왕복 지연 스파이크를 흡수
지금은 값 자체(등록 시각이 대략 "지금"인지)를 검증하려는 의도이므로, 정밀한 1초 창은 검증 의도에 비해 과하게 타이트합니다.
| assertThat(actual.getPath()) | ||
| .startsWith("/seller/products/") | ||
| .matches(endsWithUUID()); |
There was a problem hiding this comment.
💡 Suggestion — 커스텀 Predicate 단언의 빈약한 실패 메시지 (관점 3)
matches(endsWithUUID())는 실패 시 "does not match given predicate" 수준의 메시지만 남겨, 실제 어떤 path가 왔는지·왜 틀렸는지를 말해주지 못합니다. 검증 방법(split → 마지막 세그먼트 → UUID.fromString try/catch)이 본문에서 빠져 의도는 드러나지만, 실패 진단력은 약합니다.
정규식 단언으로 바꾸면 AssertJ가 기대 패턴과 실제 값을 함께 출력해 진단이 쉬워집니다.
| assertThat(actual.getPath()) | |
| .startsWith("/seller/products/") | |
| .matches(endsWithUUID()); | |
| assertThat(actual.getPath()) | |
| .startsWith("/seller/products/") | |
| .matches("/seller/products/[0-9a-fA-F-]{36}"); |
또는 UUID 검증을 도메인 언어의 커스텀 단언(satisfies(pathEndsWithUuid()))으로 끌어올려 ProductAssertions에 모으는 방향도 좋습니다.
🧪 테스트 코드 품질 리뷰전반적으로 모범적인 outside-in 통합 테스트입니다. 아래는 신뢰성·중복 측면의 개선 제안입니다.
|
| assertThat(response.getBody()).isNotNull(); | ||
| assertThat(response.getBody().accessToken()).isNotNull(); |
There was a problem hiding this comment.
💡 Suggestion — 시나리오보다 약한 단언 (관점 2)
이 테스트의 이름은 "액세스 토큰을 반환한다"인데 단언은 isNotNull()뿐이라, 서버가 빈 문자열 ""을 돌려줘도 통과합니다. 시나리오가 보장하려는 "실제 토큰 값이 있다"는 계약보다 단언이 약합니다.
| assertThat(response.getBody()).isNotNull(); | |
| assertThat(response.getBody().accessToken()).isNotNull(); | |
| assertThat(response.getBody()).isNotNull(); | |
| assertThat(response.getBody().accessToken()).isNotBlank(); |
💡 JWT 형식은 바로 다음 테스트(액세스_토큰은_JWT_형식을_따른다)가 지키므로, 여기서는 최소 isNotBlank()만으로도 "빈 토큰"이라는 실패를 잡아낼 수 있습니다.
| @ValueSource(strings = { | ||
| "invalid-email", | ||
| "invalid-email@", | ||
| "invalid-email@test", | ||
| "invalid-email@test.", | ||
| "invalid-email@.com" | ||
| }) |
There was a problem hiding this comment.
💡 Suggestion — 반복되는 반례 목록은 공유 소스로 (관점 8)
이 무효 이메일 형식 목록이 세 곳에 중복되어 있습니다.
- 여기(
seller/signup) 인라인@ValueSource shopper/signup/POST_specs.java:88에 바이트 단위로 동일한 인라인@ValueSourceTestDataSource.invalidEmails()(여기에null까지 포함)
이미 @InvalidEmailSource 라는 훌륭한 공유 소스 애노테이션을 만들어 두고 contactEmail 테스트에는 쓰고 있는데, 정작 email 형식 테스트에서는 두 스펙이 같은 목록을 다시 인라인으로 나열합니다. 이메일 정책이 바뀌면 세 곳을 모두 고쳐야 합니다.
TestDataSource에 null을 뺀 invalidEmailFormats()를 추가하고(미지정 케이스는 별도 테스트가 담당하므로), 이를 감싼 @InvalidEmailFormatSource로 두 signup 스펙이 한 줄로 공유하도록 끌어올리는 방향을 권합니다. 그러면 반례가 한곳에 모여 정책 변경 시 한 곳만 고치면 됩니다.
참고: username 목록은
seller/shopper접두사가 달라 동일하지 않으니 그대로 두는 게 맞습니다 — 인라인@ValueSource가 더 읽기 좋습니다.
- 존재하지 않는 이메일로 토큰 발행 시도 → 400 Bad Request - 잘못된 비밀번호로 토큰 발행 시도 → 400 Bad Request - 올바른 자격증명 → 200 OK with JWT token - JWT 형식 검증 Seller issueToken 명세를 기준으로 작성했으며 커버리지 공백 해소
1 participant
No description provided.