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

#22_4 [ParameterizedTest 진행하기] #24

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rlawltjd8547
Copy link
Collaborator

@FieldSource는 존재하지 않다고 나와서 진행을 하지못하였습니다..

Copy link
Collaborator

@f-lab-maverick f-lab-maverick left a comment

Choose a reason for hiding this comment

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

고생하셨습니다. JUnit 주요 사용법을 대부분 작성해보셨네요!
리뷰 반영한 후 다시 요청 주시면 되겠습니다.

추가적으로,
image
PR 생성을 진행하실 때에는 리뷰어로 @f-lab-maverick 을 지정해주세요~

Comment on lines +9 to +14
public static LocalDate findFirstSunday(LocalDate date) {
if (date.getDayOfWeek().getValue() == 7) { // 날짜가 일요일인지 확인
return date; // 만약 해당 날짜가 일요일이면 그 날짜를 반환
}
return findFirstSunday(date.plusDays(1)); // 일요일이 아니면 하루를 더하고 다시 함수 호출
}
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.

사실 재귀함수를 사용할수있으면 계속사용해보곳싶었는데
재귀함수가 참 잘 사용하기 어려운 것 같아서 지금까지 사용할 상황을 잘 캐치하지못했었습니다
요번에 반복적인 행위테스트를 진행하다 보니 재귀를 사용할수있을거란 생각을 하고 시도를 하다가 실패하여서..
슬프게도..코드자체는 GPT도움이 있었습니다..
GPT의존도는 많이 줄이긴했는데..완전히 떼지는 못했네요..

Copy link
Collaborator

Choose a reason for hiding this comment

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

가능하면 직접 구현해보려고 노력해보세요~ 로직을 짜고 직접 구현하는 과정을 통해서 시간이 걸릴 수 있겠지만, 구현능력을 키우려면 직접 짜보셔야 하기 때문에 연습이 필요합니다 :)



public static LocalDate findFirstSunday(LocalDate date) {
if (date.getDayOfWeek().getValue() == 7) { // 날짜가 일요일인지 확인
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
if (date.getDayOfWeek().getValue() == 7) { // 날짜가 일요일인지 확인
if (date.getDayOfWeek() == SUNDAY) { // 날짜가 일요일인지 확인


@Order(1)
@Test
@DisplayName("정상적인 LocalDate를 입력했을때 일요일을 찾는다. - success")
Copy link
Collaborator

Choose a reason for hiding this comment

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

테스트명을 더 자세하게 서술하고 있는 부분이 좋습니다 👍
다만, 헷갈릴 여지를 없애는게 좋습니다. 예를 들면 "정상적인 LocalDate"의 기준이 모호할 수 있겠네요. 테스트명은 조건, 행위, 결과가 포함되게 서술하면서 제목만 보고 내용을 유추할 수 있어야 합니다.

"일요일인 날짜를 입력하면 같은 날짜가 리턴된다" 와 같이 비즈니스로직을 표현할 수도 있겠네요.

class FindSundayTest {


@Order(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Order 어노테이션은 테스트에 순서를 주는 역할을 합니다.
단위테스트의 경우 순서와 관계없이 작동할 수 있어야 하기 때문에 해당 어노테이션은 지양해서 사용해주세요. 테스트에 순서가 필수가 되면 각 테스트끼리의 의존성이 생기며, 비즈니스로직 하나가 깨지면 연관된 모든 테스트가 깨질 수 있습니다.

따라서 가능한 단위테스트는 독립적으로 실행되어야하며 실행순서에 관련 없이 성공/실패되어야 합니다.

이번 PR은 학습용이기 때문에 우선 이것저것 사용해보시고, 실제 테스트코드 작성시에는 이 어노테이션은 지양해야 한다고 생각해주시면 되겠습니다.

Comment on lines 30 to 41
@Order(2)
@Test
@DisplayName("날짜 포맷이 맞지않게 12345 입력했을때 실패한다.")
public void givenWrongLocalDateForMat_whenFindFirstSunday_thenFirstSunday() {
//given
LocalDate date = LocalDate.of(12345, 12345, 12345);
//when
LocalDate result = FindSunday.findFirstSunday(date);
//then
assertEquals(LocalDate.of(2024, 9, 29), result);

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 경우 테스트 자체가 실패할 수 있겠죠.
그렇다면 "실패"를 테스트하시면 됩니다. assertThrows 를 사용하시면 예외가 발생하는 상황을 테스트할 수 있으며, 예외가 발생하지 않으면 테스트를 실패하도록 만들 수 있습니다.

모든 테스트는 "성공" / "예외발생" 을 테스트할 수 있지만 결과적으로 테스트 결과는 모두 "성공" 으로 찍혀야 합니다~ 예외가 발생한다면 예외가 발생해야 성공하는 테스트를 작성하여 모든 테스트가 성공하도록 만들어주세요.


@Order(3)
@Test
@DisplayName("경계테스트 - 최소값 테스트 - success!")
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 경우에도 경계테스트라고 작성하기보다는, 명확하게 조건을 작성해주시는게 좋습니다.
"-999999999년을 입력해도 정상적으로 일요일을 찾아 반환한다" 정도로 작성해도 됩니다. 모호한 테스트명만 피해주세요~

Copy link
Collaborator Author

@rlawltjd8547 rlawltjd8547 left a comment

Choose a reason for hiding this comment

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

작성하다보니 시나리오가 맞나? 이런생각이 듭니다 localDate타입을 이미 지정해 버리니 yyyy-mm-dd형식이 아니면 이미
형태가 잘못되었다는걸 인지가 되니 테스트 케이스 작성할때도 아 뭐 어떤 케이스가있지? 이런생각이 오래 하게됩니다

오히려 잘못된 경험일수도있지만 String으로 받아서 컨버팅 해주는 경우가 일반적인것 같아서
시나리오를 애초에 String으로 받고 형변환작업을 추가해서 진행했으면 다양한 예외케이스가 나올것같긴한데
아 사실 이러면 테스트코드 양도 엄청 많을것같네요...

궁금한점이 있습니다!
테스트 메소드명은
givenWrongLocalDateForMat_whenFindFirstSunday_thenFirstSunday
이런식으로 작성을 진행했었는데 개인적으로는 그렇게 좋다곤 생각을 안하지만
이런 형식으로 작성을 하는것인지 궁금합니다!

또한 given-when-then 은 실제로도 지키는 규칙이 맞나요?

Comment on lines 150 to 165
Csv형태는 String으로 반환하지만
Junit5에서는 자동으로 타입변환을 해준다. 즉 LocalDate로 받아도 된다.
*/
@ParameterizedTest
@DisplayName("CsvSource 이용한 테스트")
@CsvSource({
"20241004",
"2024/10/05"
})
public void csvSourceTest(LocalDate date) {
//given
//when
LocalDate result = FindSunday.findFirstSunday(date);
//then
assertThrows(ParameterResolutionException.class, () -> FindSunday.findFirstSunday(date));
}
Copy link
Collaborator Author

@rlawltjd8547 rlawltjd8547 Oct 1, 2024

Choose a reason for hiding this comment

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

csv메소드와 ArgumentsSource 메소드 작성시에
생각을 해보면 두 상황은 성공하는 케이스와 실패케이스가 같이있습니다
근데 생각해보면 성공케이스와 실패케이스는 무조건적인 분리가 필요할것같아서
애초에 시나리오가 잘못생각하고 만든거 아닌가라는 생각이듭니다
즉 테스트 given 에서 성공과 실패를할 데이터는 공존하면 안되는건가요? 각각 나누어야하나요?

또한 Csv쪽에서 작성한내용이 테스트코드의 데이터 타입에 맞추어서 형변환을 지원해주는데
여기서 에러가 나버리면 이건 어떻게 처리를 해야할지 잘모르겠습니다..

그리고
LocalDate형태로 받게 되어있는데 만약 상황이 String으로 받고 localDate로 파싱해서 메소드를 진행하게된다면
이것은 별개의 시나리오가 되는것이고 동작도 파싱이 추가되니 해당 메소드의 테스트코드를 다시 만들어줘야하는게 맞는건가요?...
개인적으로는 그렇다곤 생각하는데 아 이거 짜보니깐 실패케이스도 Exception종류별로생각하면
진짜 하나의 메소드에 테스트코드만 10개도 나올수있을것같아요 지금 간단한 메소드에서도요

Copy link
Collaborator

Choose a reason for hiding this comment

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

Q. 즉 테스트 given 에서 성공과 실패를할 데이터는 공존하면 안되는건가요? 각각 나누어야하나요?
A. 네 분리해야 합니다. 테스트 데이터는 성공하는 조건, 실패하는 조건 등으로 모두 분리해주셔야 합니다. 같은 성질의 데이터끼리 모아두어야 동일한 테스트가 가능하겠죠? 성공/실패 조건의 데이터를 모두 모아서 테스트한다면 결국은 파라미터를 받은 후 또 조건문을 작성해야 하는 경우가 생기기 때문에, 비슷한 조건의 테스트 파라미터를 모아 작성해주시면 되겠습니다.

Q. 또한 Csv쪽에서 작성한내용이 테스트코드의 데이터 타입에 맞추어서 형변환을 지원해주는데
여기서 에러가 나버리면 이건 어떻게 처리를 해야할지 잘모르겠습니다..
A. CSV타입의 데이터를 변환할 때에는 String으로 받아서, 테스트코드 내부에서 형변환을 해주세요. 타입 변환을 테스트 프레임워크에 의존하지 말고 파라미터 테스트는 말 그대로 파라미터 여러개를 제공만 해주는 역할이라고 생각해주세면 됩니다.
특정 타입의 주입이 필요한 경우 CSV가 아닌 다른 파라미터 테스트를 진행해주세요.

Q. LocalDate형태로 받게 되어있는데 만약 상황이 String으로 받고 localDate로 파싱해서 메소드를 진행하게된다면
이것은 별개의 시나리오가 되는것이고 동작도 파싱이 추가되니 해당 메소드의 테스트코드를 다시 만들어줘야하는게 맞는건가요?...
A. 이건 테스트 하고자 하는 로직이 무엇인지에 따라 달라집니다. 위 테스트코드는 findFirstSunday 메서드의 테스트로 첫번째 일요일을 찾을 수 있는지가 주요 관심사이며, LocalDate가 파싱이 잘 되는지를 테스트하는게 아닙니다. 따라서 해당 메서드 테스트는 파싱이 잘되는지를 테스트하지 말고 정확하게 해당 로직에 대해서만 테스트하는게 맞습니다. 만약 파싱에 대한 테스트를 하고싶다면 LocalDate와 관련된 별도 테스트를 구축하시면 됩니다.

Q. 개인적으로는 그렇다곤 생각하는데 아 이거 짜보니깐 실패케이스도 Exception종류별로생각하면
진짜 하나의 메소드에 테스트코드만 10개도 나올수있을것같아요 지금 간단한 메소드에서도요
A. 말씀하신대로 메서드 파라미터를 형변환하는것에 대한 테스트까지 모두 작성하려고하면 하나의 메서드에 수십 수백개의 테스트가 나올 수 있습니다. 따라서 가성비있게 테스트를 작성하려면 내가 원하는 부분을 정확하게 타겟하여 테스트코드를 작성하고, 그 이외에 대한 테스트는 하지 않는 것이 중요합니다.
예를 들어, "2024-10-01" 문자열을 LocalDate로 파싱했을 때 정상적으로 파싱되는지 여부는 이 테스트의 관심사가 아닙니다. 테스트는 findFirstSunday 메서드의 정상 동작이 최대 관심사라서 이 부분을 타겟하여 작성해주시면 됩니다. 파싱이 잘 되는지도 확인하고싶다면 위에 작성한것처럼 LocalDate 관련 학습용 테스트를 따로 작성하면 이후에는 다른 테스트에서 파싱 관련된 테스트를 하지 않아도 되겠죠?

Comment on lines 46 to 57
에초에 테스트작성시나리오가 잘못되었다 given에서 이미 예외가 발생하기 때문
현재 상황에선 아래와 같이 테스트를 작성해야한다.
혹은 테스트할 메소드에 LocalDate로 변환작업이 있어야지 기존 시나리오 검증이 된다
*/

//then
assertThrows(DateTimeException.class, () -> {
//given
LocalDate date = LocalDate.of(12345, 12345, 12345);
//when
FindSunday.findFirstSunday(date);
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분도 많이 어색합니다
시나리오가 잘못된건가 싶기도하고...뭔가 이부분은 아직 모르는것겉아서
멘토님이 뭔가 이부분에서 잘못된곳이있는지 어디가 이상한지 확인해주시면 감사드리겠습니다!!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 경우 given when then을 아예 작성하지 않고, 아래 코드만 작성해도 됩니다.

assertThrows(DateTimeException.class, () -> {
            LocalDate date = LocalDate.of(12345, 12345, 12345);
        });

Comment on lines 66 to 84
assertEquals(LocalDate.of(999999999, 12, 31), result);
//assertThrows는 람다식으로 표현을 하게되면 give-when-then형식이 깨지게된다?
assertThrows(DateTimeException.class, () -> FindSunday.findFirstSunday(date));

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

assertThrows를 처음 써보게되는데요
많이 어색하네요 Exception별로 정의해야할것같기도하고
저런 람다형태로 진행하게되는게 맞는지 이렇게 되면 given-when-then이 깨지는 것 같더라구요
항상 when시점에서 Exception을 잡아야하니깐....
맞는건지 잘 모르겠습니다....

그리고 만약 2가지 혹은 3가지 1가지 이상의 예외가 나는케이스면 예외별로 테스트 케이스를 짜야하는건가요?
아니면 해당 메소드에서 예외 확인을 assertThrows를 통해 3가지를 정의하면 되는건가요?..

Copy link
Collaborator

Choose a reason for hiding this comment

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

이럴때는 보통 when & then 형태로 묶어서 주석을 달기는 합니다. 너무 형식에 구애받아 작성하려 하지 않아도 되니 부담가지지 마세요. 테스트코드는 말 그대로 테스트를 하기 위한 코드이지 given/when/then 형식을 무조건 지켜야 하는게 아닙니다. 해당 패턴은 좀 더 깔끔한 테스트코드를 위한 보조 수단일 뿐이므로 "이게 맞는건가?" 하는 고민이 아니라, "테스트가 잘 되는가?"에 초점을 맞추어 작성해주세요~

Q. 그리고 만약 2가지 혹은 3가지 1가지 이상의 예외가 나는케이스면 예외별로 테스트 케이스를 짜야하는건가요?
아니면 해당 메소드에서 예외 확인을 assertThrows를 통해 3가지를 정의하면 되는건가요?..
A. 각 예외가 발생하는 상황이 있겠죠? 특정 상황에서는 NullPointerException / 다른 상황에서는 IllegalArgumentException 등등 조건에 따라 발생하는 예외가 달라지므로, 각 예외케이스별로 별도의 테스트코드를 작성해야 합니다.

@f-lab-maverick
Copy link
Collaborator

질문에 대한 답변을 남깁니다.

Q1. 테스트 메소드명은
givenWrongLocalDateForMat_whenFindFirstSunday_thenFirstSunday
이런식으로 작성을 진행했었는데 개인적으로는 그렇게 좋다곤 생각을 안하지만
이런 형식으로 작성을 하는것인지 궁금합니다!

A. JUnit4 시절에는 @DisplayName 어노테이션이 존재하지 않아 테스트 메소드명이 곧 테스트의 이름이 되었습니다. 그래서 메소드명을 말씀하신대로 길게 작성하거나, 아예 한글로 메소드명을 짓는 경우가 많았죠. 현재는 JUnit5를 사용하고있고, 테스트 메소드명과 테스트의 이름을 @DisplayName을 통해 분리할 수 있게 되었습니다. 그래서 메소드명은 크게 중요하지 않게 생각해주셔도 됩니다. 간단하게 작성해주셔도 되며 대신 테스트 내용을 자세하게 작성해주세요.

Q2. 또한 given-when-then 은 실제로도 지키는 규칙이 맞나요?

A. 테스트 방식에 따라 달라집니다. 말씀해주신 given/when/then방식은 Behavior-Driven Development (BDD) 기법의 테스트코드 작성 방식입니다. 조건과 동작, 결과를 예측할 수 있는 방식이라 자주 사용되기는 하지만 필수는 아닙니다.

Copy link
Collaborator

@f-lab-maverick f-lab-maverick left a comment

Choose a reason for hiding this comment

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

불필요하게 BDD방식을 사용할 필요는 없습니다.
형식을 지키지 않아도 되니 좀 더 테스트에 집중해서 코드를 수정해보시겠어요?
given / when / then 패턴이 사용되기 어려운 곳에서 해당 패턴을 사용하지 않도록 걷어내는게 이번 리뷰의 미션입니다. 파이팅입니다!

Comment on lines 46 to 57
에초에 테스트작성시나리오가 잘못되었다 given에서 이미 예외가 발생하기 때문
현재 상황에선 아래와 같이 테스트를 작성해야한다.
혹은 테스트할 메소드에 LocalDate로 변환작업이 있어야지 기존 시나리오 검증이 된다
*/

//then
assertThrows(DateTimeException.class, () -> {
//given
LocalDate date = LocalDate.of(12345, 12345, 12345);
//when
FindSunday.findFirstSunday(date);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 경우 given when then을 아예 작성하지 않고, 아래 코드만 작성해도 됩니다.

assertThrows(DateTimeException.class, () -> {
            LocalDate date = LocalDate.of(12345, 12345, 12345);
        });

Comment on lines +92 to +94
@ParameterizedTest //@Test 대신 @ParameterizedTest를 사용한다.
@DisplayName("@ValueSource 어노테이션을 사용하여 여러개의 값을 테스트에 주입할 수 있다. - success!")
@ValueSource(strings = {"2024-09-30", "2024-10-01", "2024-10-02", "2024-10-03", "2024-10-04", "2024-10-05"})
Copy link
Collaborator

Choose a reason for hiding this comment

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

명확하게 학습을 위한 테스트로 이름을 지어주셨네요~ 훨씬 테스트의 목적이 명확해보입니다 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants